diff --git a/change/@ni-nimble-components-9da2496e-e544-4019-a666-d7fca3046771.json b/change/@ni-nimble-components-9da2496e-e544-4019-a666-d7fca3046771.json new file mode 100644 index 0000000000..3cdc357bd2 --- /dev/null +++ b/change/@ni-nimble-components-9da2496e-e544-4019-a666-d7fca3046771.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Change Select to not update value as you navigate options in dropdown.", + "packageName": "@ni/nimble-components", + "email": "26874831+atmgrifter00@users.noreply.github.com", + "dependentChangeType": "patch" +} diff --git a/packages/nimble-components/src/list-option/index.ts b/packages/nimble-components/src/list-option/index.ts index 6f60d343e2..a529ebd1d8 100644 --- a/packages/nimble-components/src/list-option/index.ts +++ b/packages/nimble-components/src/list-option/index.ts @@ -41,6 +41,17 @@ export class ListOption extends FoundationListboxOption { @attr({ attribute: 'visually-hidden', mode: 'boolean' }) public visuallyHidden = false; + /** + * @internal + * This attribute is used to control the visual selected state of an option. This + * is handled independently of the public 'selected' attribute, as 'selected' is + * representative of the current value of the container control. However, while + * a dropdown is open users can navigate through the options (requiring visual + * updates) without changing the value of the container control. + */ + @attr({ attribute: 'active-option', mode: 'boolean' }) + public activeOption = false; + /** @internal */ @observable public hasOverflow = false; diff --git a/packages/nimble-components/src/select/index.ts b/packages/nimble-components/src/select/index.ts index dce1e7557b..05fac5618b 100644 --- a/packages/nimble-components/src/select/index.ts +++ b/packages/nimble-components/src/select/index.ts @@ -18,6 +18,7 @@ import { DelegatesARIASelect } from '@microsoft/fast-foundation'; import { + findLastIndex, keyArrowDown, keyArrowUp, keyEnd, @@ -25,7 +26,6 @@ import { keyEscape, keyHome, keySpace, - keyTab, uniqueId } from '@microsoft/fast-web-utilities'; import { arrowExpanderDown16X16 } from '@ni/nimble-tokens/dist/icons/js'; @@ -53,7 +53,7 @@ declare global { // eslint-disable-next-line @typescript-eslint/no-invalid-void-type type BooleanOrVoid = boolean | void; -const isNimbleListOption = (el: Element): el is ListOption => { +const isNimbleListOption = (el: Element | undefined): el is ListOption => { return el instanceof ListOption; }; @@ -196,7 +196,7 @@ export class Select private _value = ''; private forcedPosition = false; - private indexWhenOpened?: number; + private openActiveIndex?: number; /** * @internal @@ -204,7 +204,9 @@ export class Select public override connectedCallback(): void { super.connectedCallback(); this.forcedPosition = !!this.positionAttribute; - this.initializeOpenState(); + if (this.open) { + this.initializeOpenState(); + } } public override get value(): string { @@ -338,8 +340,7 @@ export class Select super.clickHandler(e); this.open = this.collapsible && !this.open; - - if (!this.open && this.indexWhenOpened !== this.selectedIndex) { + if (!this.open && this.selectedIndex !== -1) { this.updateValue(true); } } @@ -468,23 +469,22 @@ export class Select */ public inputHandler(e: InputEvent): boolean { this.filter = this.filterInput?.value ?? ''; - this.clearSelection(); this.filterOptions(); - if (this.filteredOptions.length > 0) { - const enabledOptions = this.filteredOptions.filter( - o => !o.disabled - ); - if (enabledOptions.length > 0) { - enabledOptions[0]!.selected = true; - } else { - // only filtered option is disabled - this.selectedOptions = []; - this.selectedIndex = -1; - } - } else if (this.committedSelectedOption) { - this.committedSelectedOption.selected = true; + const enabledOptions = this.filteredOptions.filter(o => !o.disabled); + let activeOptionIndex = this.filter !== '' + ? this.openActiveIndex ?? this.selectedIndex + : this.selectedIndex; + + if ( + enabledOptions.length > 0 + && !enabledOptions.find(o => o === this.options[activeOptionIndex]) + ) { + activeOptionIndex = this.options.indexOf(enabledOptions[0]!); + } else if (enabledOptions.length === 0) { + activeOptionIndex = -1; } + this.setActiveOption(activeOptionIndex); if (e.inputType.includes('deleteContent') || !this.filter.length) { return true; @@ -510,12 +510,14 @@ export class Select } if (!this.options?.includes(focusTarget as ListboxOption)) { + let currentActiveIndex = this.openActiveIndex ?? this.selectedIndex; this.open = false; - if (this.selectedIndex === -1) { - this.selectedIndex = this.indexWhenOpened!; + if (currentActiveIndex === -1) { + currentActiveIndex = this.selectedIndex; } - if (this.indexWhenOpened !== this.selectedIndex) { + if (this.selectedIndex !== currentActiveIndex) { + this.selectedIndex = currentActiveIndex; this.updateValue(true); } } @@ -526,12 +528,14 @@ export class Select * @internal */ public override keydownHandler(e: KeyboardEvent): BooleanOrVoid { + const initialSelectedIndex = this.selectedIndex; super.keydownHandler(e); const key = e.key; if (e.ctrlKey || e.shiftKey) { return true; } + let currentActiveIndex = this.openActiveIndex ?? this.selectedIndex; switch (key) { case keySpace: { // when dropdown is open allow user to enter a space for filter text @@ -571,35 +575,28 @@ export class Select if (!this.open) { break; } + if (this.collapsible && this.open) { e.preventDefault(); this.open = false; } - if (this.selectedIndex !== this.indexWhenOpened!) { - this.options[this.selectedIndex]!.selected = false; - this.selectedIndex = this.indexWhenOpened!; - } + currentActiveIndex = this.selectedIndex; this.focus(); break; } - case keyTab: { - if (this.collapsible && this.open) { - e.preventDefault(); - this.open = false; - } - - return true; - } default: { break; } } - if (!this.open && this.indexWhenOpened !== this.selectedIndex) { + if (!this.open && this.selectedIndex !== currentActiveIndex) { + this.selectedIndex = currentActiveIndex; + } + + if (!this.open && initialSelectedIndex !== this.selectedIndex) { this.updateValue(true); - this.indexWhenOpened = this.selectedIndex; } return !(key === keyArrowDown || key === keyArrowUp); @@ -622,9 +619,34 @@ export class Select // implementation handles skipping non-selected disabled options for the initial // selected value. this.setSelectedOptions(); + if (this.open) { + this.setActiveOption(this.selectedIndex); + } this.updateValue(); } + /** + * @internal + * Fork of Listbox implementation, so that the selectedIndex is not changed while the dropdown + * is open. + */ + public override typeaheadBufferChanged(_: string, __: string): void { + if (this.$fastController.isConnected) { + const typeaheadMatches = this.getTypeaheadMatches(); + + if (typeaheadMatches.length) { + const activeOptionIndex = this.options.indexOf( + typeaheadMatches[0] as ListOption + ); + if (!(this.open && this.filterMode !== FilterMode.none)) { + this.setActiveOption(activeOptionIndex); + } + } + + this.typeaheadExpired = false; + } + } + /** * Synchronize the `aria-disabled` property when the `disabled` property changes. * @@ -655,36 +677,65 @@ export class Select } } + /** + * @internal + */ public override selectNextOption(): void { // don't call super.selectNextOption as that relies on side-effecty // behavior to not select disabled option (which no longer works) - for (let i = this.selectedIndex + 1; i < this.options.length; i++) { + const startIndex = this.openActiveIndex ?? this.selectedIndex; + for (let i = startIndex + 1; i < this.options.length; i++) { const listOption = this.options[i]!; if ( isNimbleListOption(listOption) && isOptionSelectable(listOption) ) { - this.selectedIndex = i; + this.setActiveOption(i); break; } } } + /** + * @internal + */ public override selectPreviousOption(): void { // don't call super.selectPreviousOption as that relies on side-effecty // behavior to not select disabled option (which no longer works) - for (let i = this.selectedIndex - 1; i >= 0; i--) { + const startIndex = this.openActiveIndex ?? this.selectedIndex; + for (let i = startIndex - 1; i >= 0; i--) { const listOption = this.options[i]!; if ( isNimbleListOption(listOption) && isOptionSelectable(listOption) ) { - this.selectedIndex = i; + this.setActiveOption(i); break; } } } + /** + * @internal + */ + public override selectFirstOption(): void { + const newActiveOptionIndex = this.options.findIndex( + o => isNimbleListOption(o) && isOptionSelectable(o) + ); + this.setActiveOption(newActiveOptionIndex); + } + + /** + * @internal + */ + public override selectLastOption(): void { + const newActiveOptionIndex = findLastIndex( + this.options, + o => isNimbleListOption(o) && isOptionSelectable(o) + ); + this.setActiveOption(newActiveOptionIndex); + } + /** * @internal */ @@ -709,7 +760,6 @@ export class Select if (this.open && this.selectedIndex === -1) { return; } - super.setSelectedOptions(); } @@ -722,6 +772,12 @@ export class Select } } + protected override getTypeaheadMatches(): ListboxOption[] { + const matches = super.getTypeaheadMatches(); + // Don't allow placeholder to be matched + return matches.filter(o => !o.hidden && !o.disabled); + } + protected positionChanged( _: SelectPosition | undefined, next: SelectPosition | undefined @@ -757,11 +813,14 @@ export class Select if (this.open) { this.initializeOpenState(); - this.indexWhenOpened = this.selectedIndex; - return; } + const activeOption = this.options[this.openActiveIndex ?? this.selectedIndex]; + if (isNimbleListOption(activeOption)) { + activeOption.activeOption = false; + } + this.openActiveIndex = undefined; this.filter = ''; if (this.filterInput) { this.filterInput.value = ''; @@ -837,6 +896,46 @@ export class Select this.committedSelectedOption = options[this.selectedIndex]; } + private setActiveOption(newActiveIndex: number): void { + const activeOption = this.options[newActiveIndex]; + if (this.open) { + if (isNimbleListOption(activeOption)) { + activeOption.activeOption = true; + } + + const previousActiveIndex = this.openActiveIndex ?? this.selectedIndex; + const previousActiveOption = this.options[previousActiveIndex]; + if ( + previousActiveIndex !== newActiveIndex + && isNimbleListOption(previousActiveOption) + ) { + previousActiveOption.activeOption = false; + } + + this.openActiveIndex = newActiveIndex; + this.focusAndScrollActiveOptionIntoView(); + } else { + this.selectedIndex = newActiveIndex; + } + + this.ariaActiveDescendant = activeOption?.id ?? ''; + } + + private focusAndScrollActiveOptionIntoView(): void { + const optionToFocus = this.options[this.openActiveIndex ?? this.selectedIndex]; + // Copied from FAST: To ensure that the browser handles both `focus()` and + // `scrollIntoView()`, the timing here needs to guarantee that they happen on + // different frames. Since this function is typically called from the `openChanged` + // observer, `DOM.queueUpdate` causes the calls to be grouped into the same frame. + // To prevent this, `requestAnimationFrame` is used instead of `DOM.queueUpdate`. + if (optionToFocus !== undefined && this.contains(optionToFocus)) { + optionToFocus.focus(); + requestAnimationFrame(() => { + optionToFocus.scrollIntoView({ block: 'nearest' }); + }); + } + } + private committedSelectedOptionChanged(): void { this.updateDisplayValue(); } @@ -946,12 +1045,6 @@ export class Select } } - private clearSelection(): void { - this.options.forEach(option => { - option.selected = false; - }); - } - private filterChanged(): void { this.filterOptions(); } @@ -961,13 +1054,8 @@ export class Select } private initializeOpenState(): void { - if (!this.open) { - this.ariaExpanded = 'false'; - this.ariaControls = ''; - return; - } - this.committedSelectedOption = this.options[this.selectedIndex]; + this.setActiveOption(this.selectedIndex); this.ariaControls = this.listboxId; this.ariaExpanded = 'true'; diff --git a/packages/nimble-components/src/select/styles.ts b/packages/nimble-components/src/select/styles.ts index d543d15819..28f87e02d0 100644 --- a/packages/nimble-components/src/select/styles.ts +++ b/packages/nimble-components/src/select/styles.ts @@ -6,6 +6,8 @@ import { borderRgbPartialColor, borderWidth, controlHeight, + fillHoverSelectedColor, + fillSelectedColor, mediumPadding, placeholderFontColor, smallPadding @@ -116,6 +118,18 @@ export const styles = css` overflow: auto; } + ::slotted([role='option']) { + background-color: transparent; + } + + ::slotted([role='option'][active-option]) { + background-color: ${fillSelectedColor}; + } + + ::slotted([role='option'][active-option]:hover) { + background-color: ${fillHoverSelectedColor}; + } + .no-results-label { color: ${placeholderFontColor}; height: ${controlHeight}; diff --git a/packages/nimble-components/src/select/testing/select.pageobject.ts b/packages/nimble-components/src/select/testing/select.pageobject.ts index 4ea4cda6db..e649f58ca9 100644 --- a/packages/nimble-components/src/select/testing/select.pageobject.ts +++ b/packages/nimble-components/src/select/testing/select.pageobject.ts @@ -3,7 +3,8 @@ import { keyEscape, keyArrowDown, keyArrowUp, - keySpace + keySpace, + keyTab } from '@microsoft/fast-web-utilities'; import type { Select } from '..'; import type { ListOption } from '../../list-option'; @@ -56,6 +57,20 @@ export class SelectPageObject { return (this.selectElement.selectedOptions[0] as ListOption) ?? null; } + public getActiveOption(): ListOption | null { + return ( + (this.selectElement.options.find( + o => (o as ListOption).activeOption + ) as ListOption) ?? null + ); + } + + public getDisplayText(): string { + const displayText = this.selectElement.shadowRoot?.querySelector('.selected-value') + ?.textContent ?? ''; + return displayText.trim(); + } + /** * Either opens or closes the dropdown depending on its current state */ @@ -63,12 +78,16 @@ export class SelectPageObject { this.selectElement.click(); } - public clickSelectedItem(): void { + public clickActiveItem(): void { if (!this.selectElement.open) { throw new Error('Select must be open to click selectedItem'); } - this.clickOption(this.selectElement.selectedIndex); + const selectedOption = this.getActiveOption(); + if (!selectedOption) { + throw new Error('No option is selected to click'); + } + this.clickOption(this.selectElement.options.indexOf(selectedOption)); } public async clickFilterInput(): Promise { @@ -126,6 +145,13 @@ export class SelectPageObject { ); } + public pressTabKey(): void { + this.selectElement.dispatchEvent( + new KeyboardEvent('keydown', { key: keyTab }) + ); + this.selectElement.dispatchEvent(new FocusEvent('focusout')); + } + public pressArrowDownKey(): void { this.selectElement.dispatchEvent( new KeyboardEvent('keydown', { key: keyArrowDown }) @@ -138,6 +164,30 @@ export class SelectPageObject { ); } + public pressCharacterKey(character: string): void { + if (character.length !== 1) { + throw new Error( + 'character parameter must contain only a single character' + ); + } + + if ( + this.selectElement.open + && this.selectElement.filterMode !== FilterMode.none + ) { + const filterInput = this.selectElement.filterInput!; + filterInput.value += character; + } + const inputElement = this.selectElement.open + && this.selectElement.filterMode !== FilterMode.none + ? this.selectElement.filterInput + : this.selectElement; + inputElement!.dispatchEvent(new InputEvent('input')); + this.selectElement.dispatchEvent( + new KeyboardEvent('keydown', { key: character }) + ); + } + public async pressSpaceKey(): Promise { const alreadyOpen = this.selectElement.open; this.selectElement.dispatchEvent( @@ -193,7 +243,21 @@ export class SelectPageObject { } public getFilterInputText(): string { - return this.getFilterInput()?.value ?? ''; + return this.selectElement.filterInput?.value ?? ''; + } + + public async waitForChange(): Promise { + return new Promise(resolve => { + this.selectElement.addEventListener( + 'change', + () => { + resolve(this.selectElement.value); + }, + { + once: true + } + ); + }); } private getFilterInput(): HTMLInputElement | null | undefined { @@ -202,8 +266,6 @@ export class SelectPageObject { 'Select has filterMode of "none" so there is no filter input' ); } - return this.selectElement.shadowRoot?.querySelector( - '.filter-input' - ); + return this.selectElement.filterInput; } } diff --git a/packages/nimble-components/src/select/tests/select.foundation.spec.ts b/packages/nimble-components/src/select/tests/select.foundation.spec.ts index df84a45042..8405d03cf5 100644 --- a/packages/nimble-components/src/select/tests/select.foundation.spec.ts +++ b/packages/nimble-components/src/select/tests/select.foundation.spec.ts @@ -1,4 +1,8 @@ -// Based on tests in FAST repo: https://github.com/microsoft/fast/blob/085cb27d348ed6f59d080c167fa62aeaa1e3940e/packages/web-components/fast-foundation/src/select/select.spec.ts +/** + * Based on tests in FAST repo: https://github.com/microsoft/fast/blob/085cb27d348ed6f59d080c167fa62aeaa1e3940e/packages/web-components/fast-foundation/src/select/select.spec.ts + * One notable change to the tests is that we no longer expect the selectedIndex to change for the Select as the user + * navigates the dropdown menu. As as result, some test names, and the relevant expect assertions have changed. + */ import { keyArrowDown, keyArrowUp, @@ -298,7 +302,7 @@ describe('Select', () => { await disconnect(); }); - describe("should NOT emit a 'change' event when the value changes by user input while open", () => { + describe("should NOT emit a 'change' event while open", () => { it('via arrow down key', async () => { const { element, connect, disconnect } = await setup(); @@ -322,7 +326,7 @@ describe('Select', () => { expect(wasChanged).toBeFalse(); - expect(element.value).toEqual('two'); + expect(element.value).toEqual('one'); await disconnect(); }); @@ -354,7 +358,7 @@ describe('Select', () => { expect(wasChanged).toBeFalse(); - expect(element.value).toEqual('one'); + expect(element.value).toEqual('two'); await disconnect(); }); @@ -412,13 +416,13 @@ describe('Select', () => { expect(wasChanged).toBeFalse(); - expect(element.value).toEqual('three'); + expect(element.value).toEqual('one'); await disconnect(); }); }); - describe("should NOT emit an 'input' event when the value changes by user input while open", () => { + describe("should NOT emit an 'input' event while open", () => { it('via arrow down key', async () => { const { element, connect, disconnect } = await setup(); @@ -442,7 +446,7 @@ describe('Select', () => { expect(wasInput).toBeFalse(); - expect(element.value).toEqual('two'); + expect(element.value).toEqual('one'); await disconnect(); }); @@ -474,7 +478,7 @@ describe('Select', () => { expect(wasInput).toBeFalse(); - expect(element.value).toEqual('one'); + expect(element.value).toEqual('two'); await disconnect(); }); @@ -532,7 +536,7 @@ describe('Select', () => { expect(wasInput).toBeFalse(); - expect(element.value).toEqual('three'); + expect(element.value).toEqual('one'); await disconnect(); }); diff --git a/packages/nimble-components/src/select/tests/select.spec.ts b/packages/nimble-components/src/select/tests/select.spec.ts index 7f23f76d8f..0be6a103c8 100644 --- a/packages/nimble-components/src/select/tests/select.spec.ts +++ b/packages/nimble-components/src/select/tests/select.spec.ts @@ -184,8 +184,6 @@ describe('Select', () => { pageObject.clickSelect(); pageObject.pressArrowDownKey(); await waitForUpdatesAsync(); - expect(element.selectedIndex).toBe(1); - pageObject.pressEscapeKey(); await waitForUpdatesAsync(); @@ -201,7 +199,7 @@ describe('Select', () => { pageObject.pressArrowDownKey(); await waitForUpdatesAsync(); - expect(element.displayValue).toBe('One'); + expect(pageObject.getDisplayText()).toBe('One'); await disconnect(); }); @@ -310,10 +308,10 @@ describe('Select', () => { await waitForUpdatesAsync(); await clickAndWaitForOpen(element); pageObject.pressArrowDownKey(); - expect(pageObject.getSelectedOption()?.value).toBe('three'); + expect(pageObject.getActiveOption()?.value).toBe('three'); pageObject.pressArrowUpKey(); - expect(pageObject.getSelectedOption()?.value).toBe('one'); + expect(pageObject.getActiveOption()?.value).toBe('one'); await disconnect(); }); @@ -326,7 +324,78 @@ describe('Select', () => { const selectedOption = pageObject.getSelectedOption(); selectedOption!.textContent = 'foo'; await waitForUpdatesAsync(); - expect(element.displayValue).toBe('foo'); + expect(pageObject.getDisplayText()).toBe('foo'); + + await disconnect(); + }); + + it('after forcing Select to be blank, user can arrow down to first available option in dropdown', async () => { + const { element, connect, disconnect } = await setup(); + const pageObject = new SelectPageObject(element); + await connect(); + await waitForUpdatesAsync(); + element.selectedIndex = -1; + await waitForUpdatesAsync(); + + pageObject.clickSelect(); + pageObject.pressArrowDownKey(); + expect(pageObject.getActiveOption()?.value).toBe('one'); + + await disconnect(); + }); + + it('after forcing Select to be blank, pressing arrow down sets value to first available option', async () => { + const { element, connect, disconnect } = await setup(); + const pageObject = new SelectPageObject(element); + await connect(); + await waitForUpdatesAsync(); + element.selectedIndex = -1; + await waitForUpdatesAsync(); + + pageObject.pressArrowDownKey(); + expect(pageObject.getSelectedOption()?.value).toBe('one'); + + await disconnect(); + }); + + it('selecting option via typing character while dropdown is closed changes value', async () => { + const { element, connect, disconnect } = await setup(); + const pageObject = new SelectPageObject(element); + await connect(); + await waitForUpdatesAsync(); + pageObject.pressCharacterKey('t'); + await waitForUpdatesAsync(); + + expect(element.value).toBe('two'); + + await disconnect(); + }); + + it('selecting option via typing character while dropdown is open with no filter does not change value', async () => { + const { element, connect, disconnect } = await setup(); + const pageObject = new SelectPageObject(element); + await connect(); + await waitForUpdatesAsync(); + pageObject.clickSelect(); + pageObject.pressCharacterKey('t'); + await waitForUpdatesAsync(); + + expect(element.value).toBe('one'); + expect(pageObject.getActiveOption()?.value).toBe('two'); + + await disconnect(); + }); + + it('while navigating dropdown, ariaActiveDescendant is set to active option', async () => { + const { element, connect, disconnect } = await setup(); + const pageObject = new SelectPageObject(element); + await connect(); + await waitForUpdatesAsync(); + pageObject.clickSelect(); + pageObject.pressArrowDownKey(); + expect(element.ariaActiveDescendant).toBe( + pageObject.getActiveOption()!.id + ); await disconnect(); }); @@ -358,6 +427,23 @@ describe('Select', () => { await disconnect(); }); + it('when handling change event, value of select element matches what was selected in dropdown on focusout', async () => { + const { element, connect, disconnect } = await setup(); + await connect(); + await waitForUpdatesAsync(); + const pageObject = new SelectPageObject(element); + await clickAndWaitForOpen(element); + + const changeValuePromise = pageObject.waitForChange(); + pageObject.pressArrowDownKey(); + await pageObject.clickAway(); + const selectValue = await changeValuePromise; + + expect(selectValue).toBe('two'); + + await disconnect(); + }); + describe('with 500 options', () => { async function setup500Options(): Promise> { // prettier-ignore @@ -568,6 +654,13 @@ describe('Select', () => { pageObject.pressEnterKey(); expect(document.activeElement).toBe(element); }); + + it('after closing dropdown by committing a value with , activeElement is not Select element', () => { + pageObject.clickSelect(); + pageObject.pressArrowDownKey(); + pageObject.pressTabKey(); + expect(document.activeElement).not.toBe(element); + }); }); }); }); @@ -630,12 +723,12 @@ describe('Select', () => { expect(element.value).toBe('one'); await pageObject.openAndSetFilterText('T'); // Matches 'Two' and 'Three' - currentSelection = pageObject.getSelectedOption(); + currentSelection = pageObject.getActiveOption(); expect(currentSelection?.text).toBe('Two'); pageObject.pressEscapeKey(); pageObject.clickSelect(); - currentSelection = pageObject.getSelectedOption(); + currentSelection = pageObject.getActiveOption(); expect(currentSelection?.text).toBe('One'); }); @@ -648,7 +741,7 @@ describe('Select', () => { pageObject.pressEnterKey(); pageObject.clickSelect(); - currentSelection = pageObject.getSelectedOption(); + currentSelection = pageObject.getActiveOption(); expect(currentSelection?.selected).toBeTrue(); }); @@ -663,13 +756,24 @@ describe('Select', () => { expect(element.open).toBeFalse(); }); - it('filtering out current selected item and then clicking selected option changes value and closes popup', async () => { + it('filtering out current selected item and then pressing changes value and closes popup', async () => { const currentSelection = pageObject.getSelectedOption(); expect(currentSelection?.text).toBe('One'); expect(element.value).toBe('one'); await pageObject.openAndSetFilterText('T'); // Matches 'Two' and 'Three' - pageObject.clickSelectedItem(); + pageObject.pressTabKey(); + expect(element.value).toBe('two'); // 'Two' is first option in list so it should be selected now + expect(element.open).toBeFalse(); + }); + + it('filtering out current selected item and then clicking active option changes value and closes popup', async () => { + const currentSelection = pageObject.getSelectedOption(); + expect(currentSelection?.text).toBe('One'); + expect(element.value).toBe('one'); + + await pageObject.openAndSetFilterText('T'); // Matches 'Two' and 'Three' + pageObject.clickActiveItem(); expect(element.value).toBe('two'); // 'Two' is first option in list so it should be selected now expect(element.open).toBeFalse(); }); @@ -698,7 +802,7 @@ describe('Select', () => { it('allows to be used as part of filter text', async () => { await pageObject.openAndSetFilterText(' '); // Matches 'Has Space' - const currentSelection = pageObject.getSelectedOption(); + const currentSelection = pageObject.getActiveOption(); expect(currentSelection?.text).toBe('Has Space'); expect(element.open).toBeTrue(); }); @@ -743,7 +847,7 @@ describe('Select', () => { currentFilteredOptions ); expect(element.open).toBeTrue(); - expect(pageObject.getSelectedOption()?.text).toBe('Two'); + expect(pageObject.getActiveOption()?.text).toBe('Two'); }); it('filtering to only disabled item, then pressing does not close popup or change value', async () => { @@ -753,6 +857,34 @@ describe('Select', () => { expect(element.value).toBe('one'); }); + it('filtering to only disabled item, then pressing closes popup and does not change value or selectedIndex', async () => { + await pageObject.openAndSetFilterText('Disabled'); + pageObject.pressEscapeKey(); + expect(element.open).toBeFalse(); + expect(element.value).toBe('one'); + expect(element.selectedIndex).toBe(0); + }); + + it('filtering to only disabled item, then pressing closes popup and does not change value or selectedIndex', async () => { + await pageObject.openAndSetFilterText('Disabled'); + pageObject.pressTabKey(); + expect(element.open).toBeFalse(); + expect(element.value).toBe('one'); + expect(element.selectedIndex).toBe(0); + }); + + it('filtering to no available options, then pressing does not close popup or change value', async () => { + await pageObject.openAndSetFilterText('abc'); + pageObject.pressEnterKey(); + expect(element.open).toBeTrue(); + expect(element.value).toBe('one'); + }); + + it('filtering to no available options sets ariaActiveDescendent to empty string', async () => { + await pageObject.openAndSetFilterText('abc'); + expect(element.ariaActiveDescendant).toBe(''); + }); + it('filtering to no available options, then pressing does not close popup or change value', async () => { await pageObject.openAndSetFilterText('abc'); pageObject.pressEnterKey(); @@ -769,7 +901,7 @@ describe('Select', () => { it('filtering to only disabled item does not select item', async () => { await pageObject.openAndSetFilterText('Disabled'); - expect(pageObject.getSelectedOption()).toBeNull(); + expect(pageObject.getActiveOption()).toBeNull(); }); it('updating slottedOptions while open applies filter to new options', async () => { @@ -813,11 +945,11 @@ describe('Select', () => { await pageObject.setOptions(newOptions); await pageObject.openAndSetFilterText('tw'); pageObject.pressArrowDownKey(); - let currentSelection = pageObject.getSelectedOption(); + let currentSelection = pageObject.getActiveOption(); expect(currentSelection?.value).toBe('twenty'); pageObject.pressArrowUpKey(); - currentSelection = pageObject.getSelectedOption(); + currentSelection = pageObject.getActiveOption(); expect(currentSelection?.value).toBe('two'); }); @@ -837,7 +969,7 @@ describe('Select', () => { expect(currentSelection?.value).toBe('one'); }); - it('cant not select option that has been filtered out pressing arrowDown', async () => { + it('can not select option that has been filtered out pressing arrowDown', async () => { await pageObject.openAndSetFilterText('tw'); pageObject.pressArrowDownKey(); pageObject.pressEnterKey(); @@ -851,6 +983,55 @@ describe('Select', () => { currentSelection = pageObject.getSelectedOption(); expect(currentSelection?.value).toBe('three'); }); + + it('when dropdown is closed, entering text executes typeahead and sets value', () => { + pageObject.pressCharacterKey('t'); + expect(element.value).toBe('two'); + }); + + it('opening dropdown after pressing during filter text entry, maintains original display text', async () => { + await clickAndWaitForOpen(element); + pageObject.pressCharacterKey('t'); + pageObject.pressEscapeKey(); + pageObject.clickSelect(); + await waitForUpdatesAsync(); + + expect(pageObject.getDisplayText()).toBe('One'); + }); + + it('filtering options does not change selected option in dropdown', async () => { + element.value = 'three'; + await pageObject.openAndSetFilterText('t'); // filters to 'Two' and 'Three' + + expect(pageObject.getActiveOption()?.value).toBe('three'); + }); + + it('filtering options does not change selected option in dropdown after navigating with arrow keys', async () => { + await clickAndWaitForOpen(element); + pageObject.pressArrowDownKey(); + pageObject.pressArrowDownKey(); // option 'Three' should be active + pageObject.pressCharacterKey('t'); // filters to 'Two' and 'Three' + + expect(pageObject.getActiveOption()?.value).toBe('three'); + }); + + it('dismissing dropdown with after navigation and then filtering to no options, does not update value', async () => { + await clickAndWaitForOpen(element); + pageObject.pressArrowDownKey(); + pageObject.pressCharacterKey('?'); + pageObject.pressTabKey(); + + expect(pageObject.getSelectedOption()?.value).toBe('one'); + }); + + it('dismissing dropdown by clicking away after navigation and then filtering to no options, does not update value', async () => { + await clickAndWaitForOpen(element); + pageObject.pressArrowDownKey(); + pageObject.pressCharacterKey('?'); + await pageObject.clickAway(); + + expect(pageObject.getSelectedOption()?.value).toBe('one'); + }); }); describe('placeholder', () => { @@ -869,6 +1050,7 @@ describe('Select', () => { element.filterMode = FilterMode.standard; await connect(); pageObject = new SelectPageObject(element); + await waitForUpdatesAsync(); }); afterEach(async () => { @@ -881,12 +1063,12 @@ describe('Select', () => { }); it('selecting option will replace placeholder text with selected option text', async () => { - expect(element.displayValue).toBe('One'); + expect(pageObject.getDisplayText()).toBe('One'); await clickAndWaitForOpen(element); pageObject.clickOption(1); await waitForUpdatesAsync(); - expect(element.displayValue).toBe('Two'); + expect(pageObject.getDisplayText()).toBe('Two'); }); it('placeholder can be changed to another option programmatically', async () => { @@ -897,12 +1079,22 @@ describe('Select', () => { element.options[1]!.selected = true; await waitForUpdatesAsync(); - expect(element.displayValue).toBe('Two'); + expect(pageObject.getDisplayText()).toBe('Two'); expect(element.value).toBe('two'); await clickAndWaitForOpen(element); expect(pageObject.isOptionVisible(0)).toBeTrue(); expect(pageObject.isOptionVisible(1)).toBeFalse(); }); + + it('selecting option via typing will not select placeholder', async () => { + const newOptions = element.options.map(o => o as ListOption); + newOptions.push(new ListOption('One one', 'one one')); + await pageObject.setOptions(newOptions); + expect(pageObject.getDisplayText()).toBe('One'); + pageObject.pressCharacterKey('o'); + await waitForUpdatesAsync(); + expect(pageObject.getDisplayText()).toBe('One one'); + }); }); describe('PageObject', () => { diff --git a/packages/storybook/src/nimble/select/select-matrix.stories.ts b/packages/storybook/src/nimble/select/select-matrix.stories.ts index 300bd0e829..e2c8761fd8 100644 --- a/packages/storybook/src/nimble/select/select-matrix.stories.ts +++ b/packages/storybook/src/nimble/select/select-matrix.stories.ts @@ -1,13 +1,15 @@ import type { StoryFn, Meta } from '@storybook/html'; import { html, ViewTemplate } from '@microsoft/fast-element'; +import { keyArrowDown } from '@microsoft/fast-web-utilities'; import { controlLabelFont, controlLabelFontColor, standardPadding } from '@ni/nimble-components/dist/esm/theme-provider/design-tokens'; import { listOptionTag } from '@ni/nimble-components/dist/esm/list-option'; -import { selectTag } from '@ni/nimble-components/dist/esm/select'; +import { Select, selectTag } from '@ni/nimble-components/dist/esm/select'; import { DropdownAppearance } from '@ni/nimble-components/dist/esm/patterns/dropdown/types'; +import { waitForUpdatesAsync } from '@ni/nimble-components/dist/esm/testing/async-helpers'; import { createStory } from '../../utilities/storybook'; import { createMatrixThemeStory, @@ -100,6 +102,29 @@ export const blankListOption: StoryFn = createStory( ` ); +const playFunction = async (): Promise => { + await Promise.all( + Array.from(document.querySelectorAll