-
Notifications
You must be signed in to change notification settings - Fork 2.9k
feat(web-components): add keyboard support for printable characters in Dropdown #36232
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
bd07222
1c52a59
4c17c50
853d810
ec562c7
0757332
ed5047a
748ee89
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "type": "prerelease", | ||
| "comment": "add keyboard support for printable characters in Dropdown", | ||
| "packageName": "@fluentui/web-components", | ||
| "email": "machi@microsoft.com", | ||
| "dependentChangeType": "patch" | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,11 @@ import { uniqueId } from '../utils/unique-id.js'; | |
| import { DropdownType } from './dropdown.options.js'; | ||
| import { dropdownButtonTemplate, dropdownInputTemplate } from './dropdown.template.js'; | ||
|
|
||
| /** | ||
| * The duration in milliseconds after the last character search keystroke before the search string is cleared. | ||
| */ | ||
| const SEARCH_TIMEOUT = 500; | ||
|
|
||
| /** | ||
| * A Dropdown Custom HTML Element. | ||
| * Implements the {@link https://w3c.github.io/aria/#combobox | ARIA combobox } role. | ||
|
|
@@ -835,6 +840,59 @@ export class BaseDropdown extends FASTElement { | |
| this._insertingControl = false; | ||
| } | ||
|
|
||
| /** | ||
| * The accumulated search string used to match option labels by prefix when printable characters are typed. | ||
| * | ||
| * @internal | ||
| */ | ||
| private searchString: string = ''; | ||
|
|
||
| /** | ||
| * The timeout id used to reset the search string. | ||
| * | ||
| * @internal | ||
| */ | ||
| private searchTimeout?: ReturnType<typeof setTimeout>; | ||
|
|
||
| /** | ||
| * Handles printable character input by moving {@link activeIndex} to the next option whose label matches the | ||
| * accumulated search string. When the string is a single character (or the same character repeated), matching | ||
| * options are cycled through; otherwise the string is treated as a prefix match. | ||
| * | ||
| * @param char - the printable character that was pressed | ||
| * @internal | ||
| */ | ||
| private handleSearchCharacter(char: string): void { | ||
| const isRepeating = this.searchString === char.repeat(this.searchString.length); | ||
| this.searchString += char; | ||
|
|
||
| let candidates = this.searchString.length > 1 ? this.filterOptions(this.searchString) : []; | ||
| let isCycling = false; | ||
|
|
||
| if (!candidates.length && isRepeating) { | ||
| candidates = this.filterOptions(char); | ||
| isCycling = true; | ||
| } | ||
|
|
||
| if (candidates.length) { | ||
| const activeOption = this.enabledOptions[this.activeIndex]; | ||
| const currentPos = candidates.indexOf(activeOption); | ||
| const nextMatch = isCycling | ||
| ? candidates[this.getEnabledIndexInBounds(currentPos + 1, candidates.length)] | ||
| : currentPos >= 0 | ||
| ? activeOption | ||
| : candidates[0]; | ||
|
|
||
| this.activeIndex = this.enabledOptions.indexOf(nextMatch); | ||
| } | ||
|
|
||
| clearTimeout(this.searchTimeout); | ||
| this.searchTimeout = setTimeout(() => { | ||
| this.searchString = ''; | ||
| this.searchTimeout = undefined; | ||
| }, SEARCH_TIMEOUT); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be configurable within the class instance |
||
| } | ||
|
|
||
| /** | ||
| * Handles the keydown events for the dropdown. | ||
| * | ||
|
|
@@ -857,16 +915,17 @@ export class BaseDropdown extends FASTElement { | |
| break; | ||
| } | ||
|
|
||
| case ' ': { | ||
| if (this.isCombobox) { | ||
| break; | ||
| } | ||
|
|
||
| e.preventDefault(); | ||
| } | ||
|
|
||
| case ' ': | ||
| case 'Enter': | ||
| case 'Tab': { | ||
| if (e.key === ' ') { | ||
| if (this.isCombobox) { | ||
| break; | ||
| } | ||
|
|
||
| e.preventDefault(); | ||
| } | ||
|
|
||
| if (this.open) { | ||
| this.selectOption(this.activeIndex, true); | ||
| if (this.multiple) { | ||
|
|
@@ -889,6 +948,12 @@ export class BaseDropdown extends FASTElement { | |
| } | ||
|
|
||
| if (!increment) { | ||
| if (!this.isCombobox && e.key.length === 1 && e.key !== ' ' && !e.ctrlKey && !e.metaKey && !e.altKey) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the behavior with VoiceOver/Narrator when a key combination is expected to open/close the listbox? |
||
| if (!this.open) { | ||
| this.listbox.showPopover(); | ||
| } | ||
| this.handleSearchCharacter(e.key); | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
|
|
@@ -1045,6 +1110,12 @@ export class BaseDropdown extends FASTElement { | |
| BaseDropdown.AnchorPositionFallbackObserver?.disconnect(); | ||
| this.debounceController?.abort(); | ||
|
|
||
| if (this.searchTimeout) { | ||
| clearTimeout(this.searchTimeout); | ||
| this.searchTimeout = undefined; | ||
| this.searchString = ''; | ||
| } | ||
|
|
||
| super.disconnectedCallback(); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -114,6 +114,38 @@ test.describe('Dropdown', () => { | |
| await expect(listbox).toBeVisible(); | ||
| }); | ||
|
|
||
| test('should open the dropdown when a character is pressed', async ({ fastPage }) => { | ||
| const { element } = fastPage; | ||
| const listbox = element.locator(ListboxTagName); | ||
| const button = element.locator('[role=combobox]'); | ||
|
|
||
| await fastPage.setTemplate(); | ||
|
|
||
| await button.press('a'); | ||
|
|
||
| await expect(listbox).toBeVisible(); | ||
| }); | ||
|
|
||
| test('should not open the dropdown when a character is pressed with Meta, Alt, or Ctrl', async ({ fastPage }) => { | ||
| const { element } = fastPage; | ||
| const listbox = element.locator(ListboxTagName); | ||
| const button = element.locator('[role=combobox]'); | ||
|
|
||
| await fastPage.setTemplate(); | ||
|
|
||
| await button.press('Meta+a'); | ||
|
|
||
| await expect(listbox).toBeHidden(); | ||
|
|
||
| await button.press('Alt+a'); | ||
|
|
||
| await expect(listbox).toBeHidden(); | ||
|
|
||
| await button.press('Control+a'); | ||
|
|
||
| await expect(listbox).toBeHidden(); | ||
| }); | ||
|
|
||
| test("should set the `name` property on options when it's set on the dropdown", async ({ fastPage }) => { | ||
| const { element } = fastPage; | ||
| const options = element.locator(OptionTagName); | ||
|
|
@@ -550,4 +582,92 @@ test.describe('Dropdown', () => { | |
|
|
||
| await expect(listbox).toBeHidden(); | ||
| }); | ||
|
|
||
| test.describe('search options by printable characters', () => { | ||
| test.beforeEach(async ({ fastPage }) => { | ||
| await fastPage.setTemplate({ | ||
| innerHTML: ` | ||
| <${ListboxTagName}> | ||
| <${OptionTagName} id="o1">Afoo</${OptionTagName}> | ||
| <${OptionTagName} id="o2">Bfoo</${OptionTagName}> | ||
| <${OptionTagName} id="o3">Bbfoo</${OptionTagName}> | ||
| <${OptionTagName} id="o4">Bcfoo</${OptionTagName}> | ||
| <${OptionTagName} id="o5">Cfoo</${OptionTagName}> | ||
| </${ListboxTagName}> | ||
| `, | ||
| }); | ||
| }); | ||
|
Comment on lines
+587
to
+599
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use |
||
|
|
||
| test('should set active descendant based on user typing', async ({ fastPage }) => { | ||
| const { element, page } = fastPage; | ||
| const combobox = element.getByRole('combobox'); | ||
|
|
||
| await combobox.focus(); | ||
| await page.keyboard.press('b'); | ||
|
|
||
| await expect(combobox).toHaveAttribute('aria-activedescendant', 'o2'); | ||
|
|
||
| await page.waitForTimeout(500); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| await page.keyboard.press('a'); | ||
|
|
||
| await expect(combobox).toHaveAttribute('aria-activedescendant', 'o1'); | ||
|
|
||
| await page.waitForTimeout(500); | ||
|
|
||
| await page.keyboard.press('c'); | ||
|
|
||
| await expect(combobox).toHaveAttribute('aria-activedescendant', 'o5'); | ||
|
|
||
| await page.waitForTimeout(500); | ||
|
|
||
| await page.keyboard.press('d'); | ||
|
|
||
| await expect(combobox).toHaveAttribute('aria-activedescendant', 'o5'); | ||
| }); | ||
|
|
||
| test('should cycle through matching options as active descendant based on user typing', async ({ fastPage }) => { | ||
| const { element, page } = fastPage; | ||
| const combobox = element.getByRole('combobox'); | ||
|
|
||
| await combobox.focus(); | ||
| await page.keyboard.press('b'); | ||
|
|
||
| await expect(combobox).toHaveAttribute('aria-activedescendant', 'o2'); | ||
|
|
||
| await page.keyboard.press('b'); | ||
|
|
||
| await expect(combobox).toHaveAttribute('aria-activedescendant', 'o3'); | ||
|
|
||
| await page.keyboard.press('b'); | ||
|
|
||
| await expect(combobox).toHaveAttribute('aria-activedescendant', 'o4'); | ||
|
|
||
| await page.keyboard.press('b'); | ||
|
|
||
| await expect(combobox).toHaveAttribute('aria-activedescendant', 'o2'); | ||
| }); | ||
|
|
||
| test('should set active descendant if its label has repeated character', async ({ fastPage }) => { | ||
| const { element, page } = fastPage; | ||
| const combobox = element.getByRole('combobox'); | ||
|
|
||
| await combobox.focus(); | ||
| await page.keyboard.type('bb', { delay: 100 }); | ||
|
|
||
| await expect(combobox).toHaveAttribute('aria-activedescendant', 'o3'); | ||
|
|
||
| await page.waitForTimeout(500); | ||
|
|
||
| await page.keyboard.type('bb', { delay: 100 }); | ||
|
|
||
| await expect(combobox).toHaveAttribute('aria-activedescendant', 'o3'); | ||
|
|
||
| await page.waitForTimeout(500); | ||
|
|
||
| await page.keyboard.type('bb', { delay: 600 }); | ||
|
|
||
| await expect(combobox).toHaveAttribute('aria-activedescendant', 'o2'); | ||
| }); | ||
| }); | ||
| }); | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🕵🏾♀️ visual changes to review in the Visual Change Report
vr-tests-web-components/TextInput 1 screenshots