-
Notifications
You must be signed in to change notification settings - Fork 17
feat(select): add hotkey prop for items & improve keyboard navigation
#3990
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: main
Are you sure you want to change the base?
Changes from all commits
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,89 @@ | ||
| import { | ||
| LimelSelectCustomEvent, | ||
| Option, | ||
| ListSeparator, | ||
| } from '@limetech/lime-elements'; | ||
| import { Component, h, Host, State } from '@stencil/core'; | ||
|
|
||
| /** | ||
| * Select with option hotkeys. | ||
| * | ||
| * Use `hotkey` on options to bind keyboard interaction while the select | ||
| * dropdown is open. | ||
| * | ||
| * :::note | ||
| * 1. Hotkeys work only while the custom dropdown is open. | ||
| * 2. On mobile/iOS, `limel-select` uses a native `<select>`, so option | ||
| * hotkeys are not active. | ||
| * 3. Some keys are reserved for dropdown navigation and are ignored as | ||
| * option hotkeys: `tab` and arrow keys are always reserved; `enter`, `escape`, | ||
| * and `space` are reserved unless used with a modifier (for example | ||
| * `alt+enter`). | ||
| * ::: | ||
| * | ||
| * :::important | ||
| * `meta` means the Meta key. | ||
| * It is rendered as <kbd>⌘</kbd> on Apple devices and <kbd>⊞ Win</kbd> on | ||
| * Windows/Linux. (`cmd`, `command`, `win`, `windows` are aliases for `meta`.) | ||
| * | ||
| * `ctrl` means the Control key on all platforms. | ||
| * | ||
| * Choose hotkeys that do not conflict with common browser shortcuts. | ||
| * ::: | ||
| */ | ||
| @Component({ | ||
| shadow: true, | ||
| tag: 'limel-example-select-hotkeys', | ||
| }) | ||
| export class SelectHotkeysExample { | ||
| @State() | ||
| private value: Option; | ||
|
|
||
| private readonly options: (Option | ListSeparator)[] = [ | ||
| { | ||
| text: 'Started', | ||
| value: 'started', | ||
| hotkey: 's', | ||
| }, | ||
| { | ||
| text: 'In progress', | ||
| value: 'in-progress', | ||
| hotkey: 'p', | ||
| }, | ||
| { | ||
| text: 'Blocked', | ||
| value: 'blocked', | ||
| hotkey: 'b', | ||
| }, | ||
| { | ||
| text: 'Done', | ||
| value: 'done', | ||
| hotkey: 'd', | ||
| }, | ||
| { separator: true }, | ||
| { | ||
| text: 'Closed', | ||
| secondaryText: 'as not planned', | ||
| value: 'closed', | ||
| hotkey: 'cmd+d', | ||
| }, | ||
| ]; | ||
|
|
||
| public render() { | ||
| return ( | ||
| <Host> | ||
| <limel-select | ||
| label="Task status" | ||
| value={this.value} | ||
| options={this.options} | ||
| onChange={this.handleChange} | ||
| /> | ||
| <limel-example-value value={this.value} /> | ||
| </Host> | ||
| ); | ||
| } | ||
|
|
||
| private readonly handleChange = (event: LimelSelectCustomEvent<Option>) => { | ||
| this.value = event.detail; | ||
| }; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,11 @@ import { | |
| } from '@stencil/core'; | ||
| import { isMobileDevice } from '../../util/device'; | ||
| import { ENTER, SPACE } from '../../util/keycodes'; | ||
| import { | ||
| hotkeyFromKeyboardEvent, | ||
| normalizeHotkeyString, | ||
| tokenizeHotkeyString, | ||
| } from '../../util/hotkeys'; | ||
| import { isMultiple } from '../../util/multiple'; | ||
| import { createRandomString } from '../../util/random-string'; | ||
| import { SelectTemplate, triggerIconColorWarning } from './select.template'; | ||
|
|
@@ -28,6 +33,7 @@ import { SelectTemplate, triggerIconColorWarning } from './select.template'; | |
| * @exampleComponent limel-example-select-with-empty-option | ||
| * @exampleComponent limel-example-select-preselected | ||
| * @exampleComponent limel-example-select-change-options | ||
| * @exampleComponent limel-example-select-hotkeys | ||
| * @exampleComponent limel-example-select-dialog | ||
| */ | ||
| @Component({ | ||
|
|
@@ -126,6 +132,7 @@ export class Select { | |
| private portalId: string; | ||
| private focusObserver: IntersectionObserver; | ||
| private focusTimeoutId: ReturnType<typeof setTimeout>; | ||
| private readonly normalizedHotkeyCache = new Map<string, string | null>(); | ||
|
|
||
| constructor() { | ||
| this.handleMenuChange = this.handleMenuChange.bind(this); | ||
|
|
@@ -174,6 +181,11 @@ export class Select { | |
| } | ||
|
|
||
| public disconnectedCallback() { | ||
| document.removeEventListener( | ||
| 'keydown', | ||
| this.handleDocumentKeyDown, | ||
| true | ||
| ); | ||
| this.cancelPendingFocus(); | ||
|
|
||
| if (this.mdcFloatingLabel) { | ||
|
|
@@ -224,6 +236,20 @@ export class Select { | |
|
|
||
| @Watch('menuOpen') | ||
| protected watchOpen(newValue: boolean, oldValue: boolean) { | ||
| if (newValue) { | ||
| document.addEventListener( | ||
| 'keydown', | ||
| this.handleDocumentKeyDown, | ||
| true | ||
| ); | ||
| } else { | ||
| document.removeEventListener( | ||
| 'keydown', | ||
| this.handleDocumentKeyDown, | ||
| true | ||
| ); | ||
| } | ||
|
|
||
| if (this.checkValid) { | ||
| return; | ||
| } | ||
|
|
@@ -234,6 +260,11 @@ export class Select { | |
| } | ||
| } | ||
|
|
||
| @Watch('options') | ||
| protected watchOptions() { | ||
| this.normalizedHotkeyCache.clear(); | ||
| } | ||
|
|
||
| private setMenuFocus() { | ||
| if (this.isMobileDevice) { | ||
| return; | ||
|
|
@@ -367,6 +398,105 @@ export class Select { | |
| this.setTriggerFocus(); | ||
| } | ||
|
|
||
| private readonly handleDocumentKeyDown = (event: KeyboardEvent) => { | ||
| if ( | ||
| this.isMobileDevice || | ||
| !this.menuOpen || | ||
| event.defaultPrevented || | ||
| event.repeat | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| const pressedHotkey = hotkeyFromKeyboardEvent(event); | ||
| if (!pressedHotkey || this.isReservedSelectHotkey(pressedHotkey)) { | ||
| return; | ||
| } | ||
|
|
||
| const matchedOption = this.findOptionByHotkey(pressedHotkey); | ||
| if (!matchedOption || matchedOption.disabled) { | ||
| return; | ||
| } | ||
|
|
||
| event.stopPropagation(); | ||
| event.preventDefault(); | ||
|
|
||
| if (this.multiple) { | ||
| const currentValue = isMultiple(this.value) ? this.value : []; | ||
| const hasSelectedOption = currentValue.some( | ||
| (option) => option.value === matchedOption.value | ||
| ); | ||
|
|
||
| const nextValue = hasSelectedOption | ||
| ? currentValue.filter( | ||
| (option) => option.value !== matchedOption.value | ||
| ) | ||
| : [...currentValue, matchedOption]; | ||
|
|
||
| this.change.emit(nextValue); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| this.change.emit(matchedOption); | ||
| this.menuOpen = false; | ||
| this.setTriggerFocus(); | ||
| }; | ||
|
|
||
| private isReservedSelectHotkey(hotkey: string): boolean { | ||
|
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. @Kiarokh Small observation on In menu, the logic is: if (hasModifiers) {
return false; // any modified key is allowed
}
return key === 'arrowup' || ... || key === 'tab' || key === 'enter' || key === 'space' || key === 'escape';In select, arrow keys and tab are reserved regardless of modifiers: if (key === 'arrowup' || ... || key === 'arrowright') {
return true; // always reserved, even with modifiers
}
if (key === 'tab') {
return true; // always reserved, even with modifiers
}
const hasModifiers = tokens.length > 1;
return !hasModifiers && (key === 'enter' || key === 'space' || key === 'escape');This means a hotkey like Was this divergence intentional, or should they be unified?
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. Kia answered in Slack. The divergence is unintentional, so we should break out a helper and use it in both places.
Contributor
Author
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. yes, they both can share the same logic 😊 |
||
| const tokens = tokenizeHotkeyString(hotkey); | ||
| const key = tokens.at(-1); | ||
| if (!key) { | ||
| return false; | ||
| } | ||
|
|
||
| if ( | ||
| key === 'arrowup' || | ||
| key === 'arrowdown' || | ||
| key === 'arrowleft' || | ||
| key === 'arrowright' | ||
| ) { | ||
| return true; | ||
| } | ||
|
|
||
| if (key === 'tab') { | ||
| return true; | ||
| } | ||
|
|
||
| const hasModifiers = tokens.length > 1; | ||
| return ( | ||
| !hasModifiers && | ||
| (key === 'enter' || key === 'space' || key === 'escape') | ||
| ); | ||
| } | ||
|
|
||
| private findOptionByHotkey(pressedHotkey: string): Option | null { | ||
| for (const option of this.getOptionsExcludingSeparators()) { | ||
| if (option.disabled || !option.hotkey) { | ||
| continue; | ||
| } | ||
|
|
||
| const normalized = this.getNormalizedHotkey(option.hotkey); | ||
| if (normalized && normalized === pressedHotkey) { | ||
| return option; | ||
| } | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| private getNormalizedHotkey(raw: string): string | null { | ||
| const cacheKey = raw.trim(); | ||
| if (this.normalizedHotkeyCache.has(cacheKey)) { | ||
| return this.normalizedHotkeyCache.get(cacheKey) ?? null; | ||
| } | ||
|
|
||
| const normalized = normalizeHotkeyString(cacheKey); | ||
| this.normalizedHotkeyCache.set(cacheKey, normalized); | ||
|
|
||
| return normalized; | ||
| } | ||
|
|
||
| private openMenu() { | ||
| const autoSelectOption = this.getFirstNativeAutoSelectOption(); | ||
| if (autoSelectOption) { | ||
|
|
||
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.
Missing
cancelPendingFocus()call before closing menu.When selecting via hotkey in single-select mode,
cancelPendingFocus()is not called before settingmenuOpen = false. Compare withcloseMenu()(line 537-541) andhandleMenuChange()(line 397) which both callcancelPendingFocus(). This could leave a staleIntersectionObserveror timeout running.Proposed fix
this.change.emit(matchedOption); this.menuOpen = false; + this.cancelPendingFocus(); this.setTriggerFocus();📝 Committable suggestion
🤖 Prompt for AI Agents