Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions etc/lime-elements.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -4224,6 +4224,7 @@ export type OpenDirection = 'left-start' | 'left' | 'left-end' | 'right-start' |
// @public
interface Option_2<T extends string = string> {
disabled?: boolean;
hotkey?: string;
icon?: string | Icon;
// @deprecated
iconColor?: Color;
Expand Down
89 changes: 89 additions & 0 deletions src/components/select/examples/select-hotkeys.tsx
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;
};
}
6 changes: 6 additions & 0 deletions src/components/select/option.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ export interface Option<T extends string = string> {
*/
value: T;

/**
* Optional keyboard shortcut for selecting this option when the custom
* dropdown is open.
*/
hotkey?: string;

/**
* Set to `true` to make this option disabled and not possible to select.
*/
Expand Down
27 changes: 24 additions & 3 deletions src/components/select/select.template.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,12 @@ const SelectDropdown: FunctionalComponent<SelectTemplateProps> = (props) => {
};

const MenuDropdown: FunctionalComponent<SelectTemplateProps> = (props) => {
const items = createMenuItems(props.options, props.value, props.required);
const items = createMenuItems(
props.options,
props.value,
props.required,
props.isOpen
);

return (
<limel-portal
Expand Down Expand Up @@ -274,7 +279,8 @@ function isSelected(option: Option, value: Option | Option[]): boolean {
function createMenuItems(
options: Array<Option | ListSeparator>,
value: Option | Option[],
selectIsRequired = false
selectIsRequired = false,
isOpen = false
): Array<ListItem<Option> | ListSeparator> {
const menuOptionFilter = getMenuOptionFilter(selectIsRequired);

Expand All @@ -287,17 +293,31 @@ function createMenuItems(
}

const selected = isSelected(option, value);
const { text, secondaryText, disabled } = option;
const { text, secondaryText, disabled, hotkey } = option;
const name = getIconName(option.icon);

const color = getIconColor(option.icon, option.iconColor);

const primaryComponent = hotkey
? {
name: 'limel-hotkey',
props: {
value: hotkey,
disabled: !isOpen || disabled,
style: {
order: '2',
},
},
}
: undefined;

if (!name) {
return {
text: text,
secondaryText: secondaryText,
selected: selected,
disabled: disabled,
primaryComponent,
value: option,
};
}
Expand All @@ -307,6 +327,7 @@ function createMenuItems(
secondaryText: secondaryText,
selected: selected,
disabled: disabled,
primaryComponent,
value: option,
icon: {
name: name,
Expand Down
130 changes: 130 additions & 0 deletions src/components/select/select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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({
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -174,6 +181,11 @@ export class Select {
}

public disconnectedCallback() {
document.removeEventListener(
'keydown',
this.handleDocumentKeyDown,
true
);
this.cancelPendingFocus();

if (this.mdcFloatingLabel) {
Expand Down Expand Up @@ -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;
}
Expand All @@ -234,6 +260,11 @@ export class Select {
}
}

@Watch('options')
protected watchOptions() {
this.normalizedHotkeyCache.clear();
}

private setMenuFocus() {
if (this.isMobileDevice) {
return;
Expand Down Expand Up @@ -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();
Comment on lines +441 to +443
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Missing cancelPendingFocus() call before closing menu.

When selecting via hotkey in single-select mode, cancelPendingFocus() is not called before setting menuOpen = false. Compare with closeMenu() (line 537-541) and handleMenuChange() (line 397) which both call cancelPendingFocus(). This could leave a stale IntersectionObserver or timeout running.

Proposed fix
         this.change.emit(matchedOption);
         this.menuOpen = false;
+        this.cancelPendingFocus();
         this.setTriggerFocus();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
this.change.emit(matchedOption);
this.menuOpen = false;
this.setTriggerFocus();
this.change.emit(matchedOption);
this.menuOpen = false;
this.cancelPendingFocus();
this.setTriggerFocus();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/select/select.tsx` around lines 441 - 443, The selection path
emits change and then closes the menu without cancelling pending focus tasks;
update the block that calls this.change.emit(matchedOption), this.menuOpen =
false, this.setTriggerFocus() to first call cancelPendingFocus() (the same
helper used in closeMenu() and handleMenuChange()) before flipping menuOpen and
setting trigger focus so any IntersectionObserver/timeouts are cleared.

};

private isReservedSelectHotkey(hotkey: string): boolean {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kiarokh Small observation on isReservedSelectHotkey vs isReservedMenuHotkey:

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 ctrl+arrowdown would work in a menu but be blocked in a select. Since unmodified arrows are used for dropdown navigation in both components equally, it seems like the logic should be the same — i.e., the select version should also do the early if (hasModifiers) return false check, matching the menu.

Was this divergence intentional, or should they be unified?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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) {
Expand Down
Loading