Skip to content

Commit 30294bd

Browse files
radium-vradium-v
authored
fix(web-components): dropdown should emit change event when value changes via user input (#33885)
1 parent 1115b93 commit 30294bd

File tree

5 files changed

+147
-19
lines changed

5 files changed

+147
-19
lines changed

‎change/@fluentui-web-components-faa00d2e-ce58-4ae2-88af-ae49ab82bd76.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "prerelease",
3+
"comment": "fix: dropdown should emit a change event when the value changes via user input",
4+
"packageName": "@fluentui/web-components",
5+
"email": "863023+radium-v@users.noreply.github.com",
6+
"dependentChangeType": "patch"
7+
}

‎packages/web-components/docs/web-components.api.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -653,7 +653,7 @@ export class BaseDropdown extends FASTElement {
653653
// @internal
654654
get selectedIndex(): number;
655655
get selectedOptions(): DropdownOption[];
656-
selectOption(index?: number): void;
656+
selectOption(index?: number, shouldEmit?: boolean): void;
657657
// @internal
658658
setValidity(flags?: Partial<ValidityState>, message?: string, anchor?: HTMLElement): void;
659659
type: DropdownType;

‎packages/web-components/src/dropdown/dropdown.spec.ts‎

Lines changed: 118 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,20 @@
11
import { expect, test } from '../../test/playwright/index.js';
2+
import type { Dropdown } from './dropdown.js';
23

34
test.describe('Dropdown', () => {
45
test.use({
56
tagName: 'fluent-dropdown',
67
innerHTML: /* html */ `
78
<fluent-listbox>
8-
<fluent-option value="apple">Apple</fluent-option>
9-
<fluent-option value="banana">Banana</fluent-option>
10-
<fluent-option value="orange">Orange</fluent-option>
11-
<fluent-option value="mango">Mango</fluent-option>
12-
<fluent-option value="kiwi">Kiwi</fluent-option>
13-
<fluent-option value="cherry">Cherry</fluent-option>
14-
<fluent-option value="grapefruit">Grapefruit</fluent-option>
15-
<fluent-option value="papaya">Papaya</fluent-option>
16-
</fluent-listbox>
9+
<fluent-option value="apple">Apple</fluent-option>
10+
<fluent-option value="banana">Banana</fluent-option>
11+
<fluent-option value="orange">Orange</fluent-option>
12+
<fluent-option value="mango">Mango</fluent-option>
13+
<fluent-option value="kiwi">Kiwi</fluent-option>
14+
<fluent-option value="cherry">Cherry</fluent-option>
15+
<fluent-option value="grapefruit">Grapefruit</fluent-option>
16+
<fluent-option value="papaya">Papaya</fluent-option>
17+
</fluent-listbox>
1718
`,
1819
waitFor: ['fluent-listbox', 'fluent-option'],
1920
});
@@ -345,5 +346,113 @@ test.describe('Dropdown', () => {
345346

346347
await expect(input).toHaveValue('Kiwi');
347348
});
349+
350+
test('should emit a `change` event when the value changes and the control loses focus via blur', async ({
351+
fastPage,
352+
page,
353+
}) => {
354+
const { element } = fastPage;
355+
const input = element.locator('input');
356+
const listbox = element.locator('fluent-listbox');
357+
358+
await fastPage.setTemplate({ attributes: { type: 'combobox' } });
359+
360+
await input.fill('kiwi');
361+
362+
await expect(listbox).toBeVisible();
363+
364+
await expect(input).toHaveValue('kiwi');
365+
366+
await expect(input).toBeFocused();
367+
368+
await element.evaluate((el: Dropdown) => {
369+
el.addEventListener('change', () => el.insertAdjacentText('afterend', 'changed'), { once: true });
370+
});
371+
372+
await input.blur();
373+
374+
await expect(page.locator('text=changed')).toBeVisible();
375+
});
376+
});
377+
378+
test('should emit a `change` event when value changes and the control loses focus via click', async ({
379+
fastPage,
380+
page,
381+
}) => {
382+
const { element } = fastPage;
383+
const listbox = element.locator('fluent-listbox');
384+
385+
await element.click();
386+
387+
await expect(listbox).toBeVisible();
388+
389+
await expect(page.locator('text=changed')).toHaveCount(0);
390+
391+
await element.evaluate((el: Dropdown) => {
392+
el.addEventListener('change', () => el.insertAdjacentText('afterend', 'changed'), { once: true });
393+
});
394+
395+
await expect(page.locator('text=changed')).toHaveCount(0);
396+
397+
await element.locator('[value=kiwi]').click();
398+
399+
await expect(page.locator('text=changed')).toHaveCount(1);
400+
401+
await expect(page.locator('text=changed')).toBeVisible();
402+
});
403+
404+
test('should emit a `change` event when the value is confirmed by pressing Enter', async ({ fastPage }) => {
405+
const { element } = fastPage;
406+
const listbox = element.locator('fluent-listbox');
407+
408+
await element.click();
409+
410+
await expect(listbox).toBeVisible();
411+
412+
await element.press('ArrowDown');
413+
await element.press('ArrowDown');
414+
await element.press('ArrowDown');
415+
await element.press('ArrowDown');
416+
await element.press('ArrowDown');
417+
418+
await expect(element).toHaveJSProperty('activeIndex', 4);
419+
420+
const [wasChanged] = await Promise.all([
421+
element.evaluate(
422+
el =>
423+
new Promise((resolve, reject) => {
424+
const timeout = setTimeout(() => reject('change event was not emitted'), 1000);
425+
el.addEventListener(
426+
'change',
427+
() => {
428+
clearTimeout(timeout);
429+
resolve('changed');
430+
},
431+
{ once: true },
432+
);
433+
}),
434+
),
435+
element.evaluate(el => el.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' }))),
436+
]);
437+
438+
expect(wasChanged).toBe('changed');
439+
});
440+
441+
test('should NOT emit a `change` event when the value is changed programmatically', async ({ fastPage, page }) => {
442+
const { element } = fastPage;
443+
444+
await element.evaluate((el: Dropdown) => {
445+
el.addEventListener('change', () => el.insertAdjacentText('afterend', 'changed'), { once: true });
446+
});
447+
448+
await element.evaluate((el: Dropdown) => {
449+
el.value = 'kiwi';
450+
});
451+
452+
await expect(page.locator('text=changed')).toHaveCount(0);
453+
454+
await expect(element).toHaveJSProperty('value', 'kiwi');
455+
456+
await expect(element.locator('fluent-option[value=kiwi]')).toHaveJSProperty('selected', true);
348457
});
349458
});

‎packages/web-components/src/dropdown/dropdown.ts‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ export class BaseDropdown extends FASTElement {
539539
? this.enabledOptions.findIndex(x => x.text === this.control.value)
540540
: this.enabledOptions.indexOf(e.target as DropdownOption);
541541

542-
this.selectOption(optionIndex);
542+
this.selectOption(optionIndex, true);
543543

544544
return true;
545545
}
@@ -570,17 +570,21 @@ export class BaseDropdown extends FASTElement {
570570
return true;
571571
}
572572

573-
if (isDropdownOption(target) && !this.multiple) {
573+
if (isDropdownOption(target)) {
574574
if (target.disabled) {
575575
return;
576576
}
577577

578-
if (this.isCombobox) {
579-
this.control.value = target.text;
580-
this.updateFreeformOption();
581-
}
578+
this.selectOption(this.enabledOptions.indexOf(target), true);
579+
580+
if (!this.multiple) {
581+
if (this.isCombobox) {
582+
this.control.value = target.text;
583+
this.updateFreeformOption();
584+
}
582585

583-
this.listbox.hidePopover();
586+
this.listbox.hidePopover();
587+
}
584588
}
585589

586590
return true;
@@ -732,7 +736,7 @@ export class BaseDropdown extends FASTElement {
732736
case 'Enter':
733737
case 'Tab': {
734738
if (this.open) {
735-
this.selectOption(this.activeIndex);
739+
this.selectOption(this.activeIndex, true);
736740
if (this.multiple) {
737741
break;
738742
}
@@ -797,13 +801,17 @@ export class BaseDropdown extends FASTElement {
797801
* @param index - The index of the option to select.
798802
* @public
799803
*/
800-
public selectOption(index: number = this.selectedIndex): void {
804+
public selectOption(index: number = this.selectedIndex, shouldEmit: boolean = false): void {
801805
this.listbox.selectOption(index);
802806
this.control.value = this.displayValue;
803807

804808
this.setValidity();
805809

806810
this.updateFreeformOption();
811+
812+
if (shouldEmit) {
813+
this.$emit('change');
814+
}
807815
}
808816

809817
/**

‎packages/web-components/src/listbox/listbox.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,10 @@ export class Listbox extends FASTElement {
144144
* @public
145145
*/
146146
public clickHandler(e: PointerEvent): boolean | void {
147+
if (this.dropdown) {
148+
return true;
149+
}
150+
147151
const target = e.target as HTMLElement;
148152

149153
if (isDropdownOption(target)) {

0 commit comments

Comments
 (0)