Skip to content

Commit c645eda

Browse files
ahmedhamidawanahmedhamidawan
authored
Merge pull request #20401 from davelopez/25.0_fix_form_select_input_sorting
[25.0] Fix form select input sorting
2 parents f29fe3a + cad00b6 commit c645eda

File tree

3 files changed

+43
-47
lines changed

3 files changed

+43
-47
lines changed

‎client/src/components/Form/Elements/FormData/FormData.test.js‎

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,11 @@ describe("FormData", () => {
6868
values: [{ id: "dce4", src: "dce", map_over_type: null }],
6969
};
7070
const value_1 = {
71+
batch: false,
72+
product: false,
73+
values: [{ id: "hda1", src: "hda", map_over_type: null }],
74+
};
75+
const value_2 = {
7176
batch: false,
7277
product: false,
7378
values: [{ id: "hda4", src: "hda", map_over_type: null }],
@@ -88,7 +93,7 @@ describe("FormData", () => {
8893
await elements_0.at(2).find("span").trigger("click");
8994
expect(wrapper.emitted().input.length).toEqual(2);
9095
expect(wrapper.emitted().input[1][0]).toEqual(value_1);
91-
await wrapper.setProps({ value: value_1 });
96+
await wrapper.setProps({ value: value_2 });
9297
expect(wrapper.find(SELECTED_VALUE).text()).toContain("4: hdaName4");
9398
});
9499

@@ -131,8 +136,8 @@ describe("FormData", () => {
131136
expect(wrapper.emitted().input.length).toEqual(1);
132137
const selectedValues = wrapper.findAll(SELECTED_VALUE);
133138
expect(selectedValues.length).toBe(2);
134-
expect(selectedValues.at(0).text()).toContain("3: hdaName3");
135-
expect(selectedValues.at(1).text()).toContain("2: hdaName2");
139+
expect(selectedValues.at(0).text()).toContain("2: hdaName2");
140+
expect(selectedValues.at(1).text()).toContain("3: hdaName3");
136141
const value_0 = {
137142
batch: false,
138143
product: false,
@@ -146,15 +151,15 @@ describe("FormData", () => {
146151
const value_1 = {
147152
batch: false,
148153
product: false,
149-
values: [{ id: "hda2", map_over_type: null, src: "hda" }],
154+
values: [{ id: "hda3", map_over_type: null, src: "hda" }],
150155
};
151156
expect(wrapper.emitted().input[1][0]).toEqual(value_1);
152157
await wrapper.setProps({ value: value_1 });
153158
await selectedValues.at(1).trigger("click");
154159
const value_2 = {
155160
batch: false,
156161
product: false,
157-
values: [{ id: "hda2", map_over_type: null, src: "hda" }],
162+
values: [{ id: "hda3", map_over_type: null, src: "hda" }],
158163
};
159164
expect(wrapper.emitted().input[1][0]).toEqual(value_2);
160165
await wrapper.setProps({ value: value_2 });
@@ -165,7 +170,7 @@ describe("FormData", () => {
165170
it("properly sorts multiple datasets", async () => {
166171
const wrapper = createTarget({
167172
value: {
168-
// the order of values does not matter here
173+
// the order of values does matter here
169174
values: [
170175
{ id: "hda2", src: "hda" },
171176
{ id: "hda3", src: "hda" },
@@ -178,18 +183,18 @@ describe("FormData", () => {
178183
});
179184
const selectedValues = wrapper.findAll(SELECTED_VALUE);
180185
expect(selectedValues.length).toBe(3);
181-
// the values in the multiselect are sorted by hid DESC
182-
expect(selectedValues.at(0).text()).toContain("3: hdaName3");
186+
// the values in the multiselect are sorted by hid ASC
187+
expect(selectedValues.at(0).text()).toContain("1: hdaName1");
183188
expect(selectedValues.at(1).text()).toContain("2: hdaName2");
184-
expect(selectedValues.at(2).text()).toContain("1: hdaName1");
189+
expect(selectedValues.at(2).text()).toContain("3: hdaName3");
185190
await selectedValues.at(0).trigger("click");
186191
const value_sorted = {
187192
batch: false,
188193
product: false,
189194
values: [
190195
// the values in the emitted input are sorted by hid ASC
191-
{ id: "hda1", map_over_type: null, src: "hda" },
192196
{ id: "hda2", map_over_type: null, src: "hda" },
197+
{ id: "hda3", map_over_type: null, src: "hda" },
193198
],
194199
};
195200
expect(wrapper.emitted().input[1][0]).toEqual(value_sorted);
@@ -227,24 +232,20 @@ describe("FormData", () => {
227232
});
228233
const selectedValues = wrapper.findAll(SELECTED_VALUE);
229234
expect(selectedValues.length).toBe(5);
230-
// when dces are mixed in their values are shown first and are
231-
// ordered by id descending
232235
expect(selectedValues.at(0).text()).toContain("dceName4 (as dataset)");
233236
expect(selectedValues.at(1).text()).toContain("dceName3 (as dataset)");
234237
expect(selectedValues.at(2).text()).toContain("dceName2 (as dataset)");
235-
expect(selectedValues.at(3).text()).toContain("2: hdaName2");
236-
expect(selectedValues.at(4).text()).toContain("1: hdaName1");
238+
expect(selectedValues.at(3).text()).toContain("1: hdaName1");
239+
expect(selectedValues.at(4).text()).toContain("2: hdaName2");
237240
await selectedValues.at(0).trigger("click");
238241
const value_sorted = {
239242
batch: false,
240243
product: false,
241244
values: [
242-
// when dces are mixed in, they are emitted first
243-
// and are sorted by id descending
244-
{ id: "dce3", map_over_type: null, src: "dce" },
245-
{ id: "dce2", map_over_type: null, src: "dce" },
246245
{ id: "hda1", map_over_type: null, src: "hda" },
246+
{ id: "dce2", map_over_type: null, src: "dce" },
247247
{ id: "hda2", map_over_type: null, src: "hda" },
248+
{ id: "dce3", map_over_type: null, src: "dce" },
248249
],
249250
};
250251
expect(wrapper.emitted().input[1][0]).toEqual(value_sorted);
@@ -446,7 +447,7 @@ describe("FormData", () => {
446447
const value_0 = {
447448
batch: true,
448449
product: false,
449-
values: [{ id: "hda3", map_over_type: null, src: "hda" }],
450+
values: [{ id: "hda2", map_over_type: null, src: "hda" }],
450451
};
451452
expect(wrapper.emitted().input[2][0]).toEqual(value_0);
452453
await wrapper.setProps({ value: value_0 });
@@ -455,7 +456,7 @@ describe("FormData", () => {
455456
batch: true,
456457
product: false,
457458
values: [
458-
{ id: "hda3", map_over_type: null, src: "hda" },
459+
{ id: "hda2", map_over_type: null, src: "hda" },
459460
{ id: "dce4", map_over_type: null, src: "dce" },
460461
],
461462
};
@@ -474,7 +475,7 @@ describe("FormData", () => {
474475
batch: true,
475476
product: true,
476477
values: [
477-
{ id: "hda3", map_over_type: null, src: "hda" },
478+
{ id: "hda2", map_over_type: null, src: "hda" },
478479
{ id: "dce4", map_over_type: null, src: "dce" },
479480
],
480481
});
@@ -516,16 +517,16 @@ describe("FormData", () => {
516517
});
517518
const select_0 = wrapper_0.findAll(SELECT_OPTIONS);
518519
expect(select_0.length).toBe(4);
519-
expect(select_0.at(2).text()).toContain("2: hdaName2");
520-
expect(select_0.at(3).text()).toContain("1: hdaName1");
520+
expect(select_0.at(2).text()).toContain("1: hdaName1");
521+
expect(select_0.at(3).text()).toContain("2: hdaName2");
521522
const wrapper_1 = createTarget({
522523
tag: "tag2",
523524
options: defaultOptions,
524525
});
525526
const select_1 = wrapper_1.findAll(SELECT_OPTIONS);
526527
expect(select_1.length).toBe(4);
527-
expect(select_1.at(2).text()).toContain("3: hdaName3");
528-
expect(select_1.at(3).text()).toContain("2: hdaName2");
528+
expect(select_1.at(2).text()).toContain("2: hdaName2");
529+
expect(select_1.at(3).text()).toContain("3: hdaName3");
529530
const wrapper_2 = createTarget({
530531
tag: "tag3",
531532
options: defaultOptions,

‎client/src/components/Form/Elements/FormData/FormData.vue‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -145,17 +145,6 @@ const currentValue = computed({
145145
return undefined;
146146
},
147147
set: (val) => {
148-
if (val && Array.isArray(val) && val.length > 0) {
149-
val.sort((a, b) => {
150-
const aHid = a.hid;
151-
const bHid = b.hid;
152-
if (aHid && bHid) {
153-
return aHid - bHid;
154-
} else {
155-
return 0;
156-
}
157-
});
158-
}
159148
$emit("input", createValue(val));
160149
},
161150
});
@@ -222,16 +211,6 @@ const formattedOptions = computed(() => {
222211
}
223212
}
224213
});
225-
// Sort entries by hid
226-
result.sort((a, b) => {
227-
const aHid = a.value && a.value.hid;
228-
const bHid = b.value && b.value.hid;
229-
if (aHid && bHid) {
230-
return bHid - aHid;
231-
} else {
232-
return 0;
233-
}
234-
});
235214
// Add optional entry
236215
if (!currentVariant.value?.multiple && props.optional) {
237216
result.unshift({

‎client/src/components/Form/Elements/FormSelect.vue‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,23 @@ const trackBy = computed(() => {
139139
* Tracks current value and emits changes
140140
*/
141141
const currentValue = computed({
142-
get: () => props.options.filter((option: SelectOption) => isSelected(option.value)).map(getSelectOption),
142+
get: () => {
143+
// Preserve the order of props.value
144+
const values = Array.isArray(props.value) ? props.value : [props.value];
145+
return values
146+
.map((val) => {
147+
// Find the matching option in props.options
148+
const option = props.options.find(
149+
(opt) =>
150+
isSelected(opt.value) &&
151+
(isDataOptionObject(val)
152+
? isDataOptionObject(opt.value) && itemUniqueKey(opt.value) === itemUniqueKey(val)
153+
: opt.value === val)
154+
);
155+
return option ? getSelectOption(option) : undefined;
156+
})
157+
.filter((v) => v !== undefined);
158+
},
143159
set: (val: Array<SelectOption> | SelectOption): void => {
144160
if (Array.isArray(val)) {
145161
if (val.length > 0) {

0 commit comments

Comments
 (0)