Skip to content

Commit 439b0ca

Browse files
author
Salim Terres
committed
Bug #15375 [Vitam_ui] – In the usage selector, selecting one option automatically checks another option.
1 parent bb5c431 commit 439b0ca

File tree

2 files changed

+213
-67
lines changed

2 files changed

+213
-67
lines changed

ui/ui-frontend/projects/vitamui-library/src/lib/components/select/select.component.spec.ts

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,5 +231,90 @@ describe('SelectComponent', () => {
231231
const allValues = options.options.map((option) => option.key);
232232
expect(testHostComponent.control.value).toEqual(allValues);
233233
});
234+
235+
it('should only select chosen options and not others', async () => {
236+
const labelElement = hostFixture.debugElement.query(By.css('mat-label')).nativeElement;
237+
labelElement.click();
238+
239+
const selectOptions = await selectHarness.getOptions();
240+
await selectOptions[1].click();
241+
await selectOptions[2].click();
242+
243+
expect(testHostComponent.control.value).toEqual(['option1', 'option2']);
244+
expect(testHostComponent.control.value).not.toContain('option3');
245+
expect(testHostComponent.control.value).not.toContain('something-else');
246+
});
247+
248+
it('should correctly deselect an option without affecting others', async () => {
249+
testHostComponent.control.setValue(['option1', 'option2', 'option3']);
250+
hostFixture.detectChanges();
251+
await hostFixture.whenStable();
252+
253+
const labelElement = hostFixture.debugElement.query(By.css('mat-label')).nativeElement;
254+
labelElement.click();
255+
256+
const selectOptions = await selectHarness.getOptions();
257+
await selectOptions[2].click();
258+
259+
expect(testHostComponent.control.value).toEqual(['option1', 'option3']);
260+
expect(testHostComponent.control.value).not.toContain('option2');
261+
});
262+
263+
it('should display preselected values correctly on load', async () => {
264+
const preselectedValues = ['option2', 'option3'];
265+
testHostComponent.control.setValue(preselectedValues);
266+
hostFixture.detectChanges();
267+
await hostFixture.whenStable();
268+
269+
const valueElement = hostFixture.debugElement.query(By.css('mat-select-trigger')).nativeElement;
270+
expect(valueElement.textContent.trim().startsWith('2')).toBeTrue();
271+
});
272+
273+
it('should maintain selection integrity when options are reloaded', async () => {
274+
testHostComponent.control.setValue(['option1', 'option3']);
275+
hostFixture.detectChanges();
276+
await hostFixture.whenStable();
277+
278+
testHostComponent.options = {
279+
options: [
280+
{ key: 'option1', label: 'Updated Option 1' },
281+
{ key: 'option2', label: 'Updated Option 2' },
282+
{ key: 'option3', label: 'Updated Option 3' },
283+
],
284+
};
285+
hostFixture.detectChanges();
286+
await hostFixture.whenStable();
287+
288+
expect(testHostComponent.control.value).toEqual(['option1', 'option3']);
289+
});
290+
});
291+
292+
describe('in NON multiple mode (selection integrity)', () => {
293+
beforeEach(init(false));
294+
295+
it('should only select one option at a time', async () => {
296+
const labelElement = hostFixture.debugElement.query(By.css('mat-label')).nativeElement;
297+
298+
labelElement.click();
299+
const selectOptions = await selectHarness.getOptions();
300+
await selectOptions[0].click();
301+
302+
expect(testHostComponent.control.value).toBe('option1');
303+
304+
labelElement.click();
305+
const selectOptions2 = await selectHarness.getOptions();
306+
await selectOptions2[1].click();
307+
308+
expect(testHostComponent.control.value).toBe('option2');
309+
});
310+
311+
it('should display preselected value correctly on load', async () => {
312+
testHostComponent.control.setValue('option3');
313+
hostFixture.detectChanges();
314+
await hostFixture.whenStable();
315+
316+
const valueElement = hostFixture.debugElement.query(By.css('mat-select-trigger')).nativeElement;
317+
expect(valueElement.textContent.trim()).toBe('option 3');
318+
});
234319
});
235320
});

ui/ui-frontend/projects/vitamui-library/src/lib/components/select/select.component.ts

Lines changed: 128 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -168,28 +168,25 @@ export class SelectComponent extends AbstractFormInputDirective implements After
168168

169169
@Input({ required: true })
170170
set options(optionsParam: VitamuiSelectOptions | any[]) {
171-
const options: VitamuiSelectOptions =
172-
optionsParam instanceof Array
173-
? optionsParam[0]?.key != null && optionsParam[0]?.label != null
174-
? { options: optionsParam }
175-
: {
176-
options: optionsParam.map((option) => ({
177-
key: option,
178-
label: option.toString(),
179-
})),
180-
}
181-
: optionsParam;
171+
const options: VitamuiSelectOptions = this.normalizeSelection(optionsParam);
172+
182173
this.allOptions = options?.options != null ? options.options : [];
183174
if (options?.customSorting != null) {
184175
this.customSorting = options.customSorting;
185176
this.allOptions.sort(this.customSorting);
186177
}
187178
this.displayedOptions = this.allOptions;
188179
this.resizeContainerHeightInSearchView();
189-
if (!this.selectedOptions.length)
190-
this.selectedOptions = this.allOptions.filter((option) => this.preselectedOptionKeys?.includes(option.key));
191-
if (this.control) this.control.setValue(this.control.value); // We force-update the control value after updating the options to make sure the mat-select updates the displayed value
180+
this.synchronizeSelectedOptions();
192181
this.resizeContainerHeightInSelectedItemsView();
182+
183+
// Force display update after options are loaded
184+
if (this.control?.value && this.allOptions.length > 0) {
185+
Promise.resolve().then(() => {
186+
this.updateMatSelectTriggerContent();
187+
this.cd.detectChanges();
188+
});
189+
}
193190
this.addEventListeners();
194191
}
195192

@@ -217,6 +214,7 @@ export class SelectComponent extends AbstractFormInputDirective implements After
217214
private _enableSearch = true;
218215
private _enableSelectAll?: boolean;
219216
private _enableDisplaySelected?: boolean;
217+
private isUpdatingTrigger = false; // Prevent infinite recursion
220218

221219
@ViewChild('searchBar') searchBar: SearchBarComponent;
222220
@ViewChild('scrollViewport') private cdkVirtualScrollViewport: CdkVirtualScrollViewport;
@@ -244,7 +242,6 @@ export class SelectComponent extends AbstractFormInputDirective implements After
244242
}
245243

246244
ngAfterViewChecked(): void {
247-
this.updateCheckboxes();
248245
this.updateSelectAll();
249246
}
250247

@@ -255,12 +252,18 @@ export class SelectComponent extends AbstractFormInputDirective implements After
255252
this.sd
256253
.scrolled()
257254
.pipe(filter((scrollable) => this.cdkVirtualScrollViewport === scrollable))
258-
.subscribe(() => {
259-
this.updateCheckboxes();
260-
this.updateSelectAll();
261-
});
255+
.subscribe(() => this.updateSelectAll());
262256

263257
this.addEventListeners();
258+
259+
// Force value display after complete initialization
260+
if (this.control?.value) {
261+
Promise.resolve().then(() => {
262+
this.synchronizeSelectedOptions();
263+
this.updateMatSelectTriggerContent();
264+
this.cd.detectChanges();
265+
});
266+
}
264267
}
265268

266269
private addEventListeners() {
@@ -285,23 +288,29 @@ export class SelectComponent extends AbstractFormInputDirective implements After
285288
// When the component is reset this method is called with selectedOptionKeys = null
286289
if (this.preselectedOptionKeys == null) {
287290
this.selectedOptions = [];
288-
} else {
291+
} else if (this.allOptions.length > 0) {
289292
this.selectedOptions = this.allOptions.filter((option) => this.preselectedOptionKeys.includes(option.key));
290293
}
291-
this.updateCheckboxes();
292-
this.updateSelectAll();
293294

294295
this.resizeContainerHeightInSelectedItemsView();
296+
this.updateMatSelectTriggerContent();
297+
this.cd.markForCheck();
295298
}
296299

297300
protected openedChange(opened: boolean): void {
298-
// Attend que overlay du select soit rendu
299-
setTimeout(() => {
300-
this.viewport.checkViewportSize();
301-
});
301+
if (opened) {
302+
setTimeout(() => {
303+
this.ensureValueDisplay();
304+
this.viewport?.checkViewportSize();
305+
306+
if (this.selectedOptions.length > 0) {
307+
this.scrollToSelectedOption();
308+
}
309+
});
310+
}
302311

303312
if (opened && this.enableSearch) {
304-
this.searchBar.onFocus();
313+
this.searchBar?.onFocus();
305314
}
306315
}
307316

@@ -374,7 +383,10 @@ export class SelectComponent extends AbstractFormInputDirective implements After
374383
if (uncheckingOption) {
375384
this.selectedOptions = this.selectedOptions.filter((selectedOption) => selectedOption.key !== value);
376385
} else {
377-
this.selectedOptions.push(this.allOptions.filter((selectedOption) => selectedOption.key === value)[0]);
386+
const optionToAdd = this.allOptions.find((selectedOption) => selectedOption.key === value);
387+
if (optionToAdd) {
388+
this.selectedOptions.push(optionToAdd);
389+
}
378390
}
379391
} else {
380392
this.selectedOptions = uncheckingOption ? [] : this.allOptions.filter((selectedOption) => selectedOption.key === value);
@@ -390,19 +402,43 @@ export class SelectComponent extends AbstractFormInputDirective implements After
390402
}
391403

392404
this.updateMatSelectTriggerContent();
393-
this.normalizeSelection();
394405
}
395406

396-
protected compareOptions(o1: { key: string } | null, o2: { key: string } | null): boolean {
397-
return !!o1 && !!o2 ? o1.key === o2.key : o1 === o2;
398-
}
407+
public readonly compareOptions = (o1: any, o2: any): boolean => {
408+
if (o1 == null || o2 == null) {
409+
return o1 === o2;
410+
}
399411

400-
private normalizeSelection() {
401-
if (this.multiple && Array.isArray(this.control.value)) {
402-
const set = new Set(this.control.value);
403-
const normalized = this.allOptions.map((o) => o.key).filter((k) => set.has(k));
404-
this.control.setValue(normalized, { emitEvent: false });
412+
if (this.multiple && Array.isArray(o1) && Array.isArray(o2)) {
413+
if (o1.length !== o2.length) return false;
414+
return o1.every((val) => o2.includes(val));
405415
}
416+
417+
const val1 = typeof o1 === 'object' && o1 !== null ? o1.key : o1;
418+
const val2 = typeof o2 === 'object' && o2 !== null ? o2.key : o2;
419+
420+
return String(val1) === String(val2);
421+
};
422+
423+
private normalizeSelection(optionsParam: VitamuiSelectOptions | any[]): VitamuiSelectOptions {
424+
if (optionsParam instanceof Array) {
425+
if (optionsParam.length === 0) {
426+
return { options: [] };
427+
}
428+
429+
const firstItem = optionsParam[0];
430+
if (firstItem?.key != null && firstItem?.label != null) {
431+
return { options: optionsParam };
432+
} else {
433+
return {
434+
options: optionsParam.map((option) => ({
435+
key: option,
436+
label: option.toString(),
437+
})),
438+
};
439+
}
440+
}
441+
return optionsParam;
406442
}
407443

408444
private overrideControlMethods() {
@@ -448,12 +484,10 @@ export class SelectComponent extends AbstractFormInputDirective implements After
448484
this.selectedOptions = [...this.allOptions];
449485
const selectedKeys = [...this.selectedOptions.map((option) => option.key)].sort();
450486
this.onChange(selectedKeys);
451-
this.updateCheckboxes();
452487
} else {
453488
this.clearAllSelectedOptions();
454489
}
455490
this.resizeContainerHeightInSelectedItemsView();
456-
this.normalizeSelection();
457491
}
458492

459493
private resizeContainerHeightInSearchView(): void {
@@ -503,44 +537,71 @@ export class SelectComponent extends AbstractFormInputDirective implements After
503537
}
504538
}
505539

506-
private updateCheckboxes(): void {
507-
if (this.optionKeys == null) {
508-
return;
509-
}
540+
private updateMatSelectTriggerContent(): void {
541+
if (!this.matSelect || this.isUpdatingTrigger) return;
510542

511-
let needUpdate = false;
543+
Object.defineProperties(this.matSelect, {
544+
empty: {
545+
value: this.selectedOptions.length <= 0,
546+
writable: true,
547+
},
548+
});
512549

513-
this.optionKeys.forEach((optionKey) => {
514-
const selected = this.selectedOptions.filter((selectedOption) => selectedOption.key === optionKey.value);
550+
if (this.control?.value != null) {
551+
this.isUpdatingTrigger = true;
515552

516-
if (selected.length > 0 && !optionKey.selected) {
517-
optionKey.select(false);
518-
needUpdate = true;
519-
} else if (selected.length === 0 && optionKey.selected) {
520-
optionKey.deselect(false);
521-
needUpdate = true;
553+
try {
554+
this.matSelect.value = this.control.value;
555+
this.matSelect._onChange(this.control.value);
556+
this.matSelect.stateChanges.next();
557+
} finally {
558+
this.isUpdatingTrigger = false;
522559
}
523-
});
524-
525-
if (needUpdate) {
526-
this.cd.detectChanges();
527560
}
561+
}
528562

529-
this.updateMatSelectTriggerContent();
563+
focus() {
564+
this.matSelect.focus();
530565
}
531566

532-
private updateMatSelectTriggerContent(): void {
533-
if (this.matSelect)
534-
Object.defineProperties(this.matSelect, {
535-
empty: {
536-
value: this.selectedOptions.length <= 0,
537-
writable: true,
538-
},
539-
});
567+
private scrollToSelectedOption(): void {
568+
if (!this.viewport || this.selectedOptions.length === 0) return;
569+
570+
const firstSelectedOption = this.selectedOptions[0];
571+
const index = this.displayedOptions.findIndex((opt) => opt.key === firstSelectedOption.key);
572+
573+
if (index >= 0) {
574+
setTimeout(() => {
575+
this.viewport.scrollToIndex(index, 'smooth');
576+
}, 100);
577+
}
540578
}
541579

542-
focus() {
543-
this.matSelect.focus();
580+
private synchronizeSelectedOptions(): void {
581+
const keysToSelect = this.preselectedOptionKeys?.length
582+
? this.preselectedOptionKeys
583+
: this.control?.value
584+
? Array.isArray(this.control.value)
585+
? this.control.value
586+
: [this.control.value]
587+
: [];
588+
589+
if (keysToSelect.length > 0 && this.allOptions.length > 0) {
590+
this.selectedOptions = this.allOptions.filter((option) => keysToSelect.includes(option.key));
591+
this.updateMatSelectTriggerContent();
592+
}
593+
}
594+
595+
private ensureValueDisplay(): void {
596+
if (!this.matSelect || !this.control?.value) return;
597+
598+
setTimeout(() => {
599+
this.updateMatSelectTriggerContent();
600+
if (this.multiple && Array.isArray(this.control.value)) {
601+
this.selectedOptions = this.allOptions.filter((option) => this.control.value.includes(option.key));
602+
}
603+
this.cd.detectChanges();
604+
});
544605
}
545606

546607
protected readonly Validators = Validators;

0 commit comments

Comments
 (0)