Skip to content
Closed
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',
},
Comment on lines +64 to +69
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

Replace cmd+d with a portable demo shortcut.

cmd+d normalizes to meta+d, which becomes Win+D on Windows/Linux and collides with an OS-owned shortcut. That makes the example unreliable outside macOS; please use a non-meta combination here.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/select/examples/select-hotkeys.tsx` around lines 64 - 69, The
demo option object that sets hotkey: 'cmd+d' (the entry with value 'closed' /
text 'Closed') uses a meta-based shortcut which normalizes to OS-level shortcuts
on Windows/Linux; update that hotkey to a portable, non-meta combination (for
example 'ctrl+shift+d' or another ctrl/alt-based combo) so the example works
consistently across platforms and avoids OS shortcut collisions—locate the
object with value 'closed' in select-hotkeys.tsx and replace its hotkey property
accordingly.

];

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;
Comment on lines +424 to +438
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 | 🟠 Major

Keep the multi-select hotkey path on the existing scroll-preserving flow.

handleMenuChange() already snapshots and restores the menu surface scroll position for multi-select updates because rerendering otherwise jumps the menu back to the top. This new branch emits nextValue directly, so toggling items with hotkeys will regress that behavior in long dropdowns.

🤖 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 424 - 438, The multi-select
branch currently calls this.change.emit(nextValue) directly (inside the
this.multiple / isMultiple / matchedOption block), which bypasses the scroll
snapshot/restore logic in handleMenuChange() and causes the menu to jump;
instead, route the update through the existing handleMenuChange flow (call the
appropriate handleMenuChange path or helper with nextValue so the
snapshot/restore logic runs) rather than emitting nextValue directly, ensuring
you reference the same symbols (this.multiple, isMultiple(this.value),
matchedOption, nextValue) when making the change.

}

this.change.emit(matchedOption);
this.menuOpen = false;
this.setTriggerFocus();
};

private isReservedSelectHotkey(hotkey: string): boolean {
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;
Comment on lines +473 to +485
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

Hotkeys can still select options that the required menu intentionally hides.

findOptionByHotkey() scans every non-separator option, but createMenuItems() filters out empty-text options when required is true. A hidden “empty option” can therefore still be selected via hotkey, which breaks the required-select invariant. Please apply the same filter before matching here.

🔧 Suggested direction
     private findOptionByHotkey(pressedHotkey: string): Option | null {
-        for (const option of this.getOptionsExcludingSeparators()) {
+        const options = this.required
+            ? this.getOptionsExcludingSeparators().filter((option) => option.text)
+            : this.getOptionsExcludingSeparators();
+
+        for (const option of options) {
             if (option.disabled || !option.hotkey) {
                 continue;
             }
📝 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
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 findOptionByHotkey(pressedHotkey: string): Option | null {
const options = this.required
? this.getOptionsExcludingSeparators().filter((option) => option.text)
: this.getOptionsExcludingSeparators();
for (const option of options) {
if (option.disabled || !option.hotkey) {
continue;
}
const normalized = this.getNormalizedHotkey(option.hotkey);
if (normalized && normalized === pressedHotkey) {
return option;
}
}
return null;
🤖 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 473 - 485, findOptionByHotkey
currently iterates getOptionsExcludingSeparators() and may match options that
createMenuItems intentionally omits when this.selectProps.required is true
(e.g., empty-text/placeholder options). Update findOptionByHotkey to apply the
same filtering logic used in createMenuItems: skip any option that would be
filtered out when this.selectProps.required is true (for example, options with
empty text/value or the placeholder option), then normalize and compare hotkeys
as before; reference the findOptionByHotkey, getOptionsExcludingSeparators, and
createMenuItems logic to ensure the filters are identical.

}

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