Skip to content

feat(core): refactor keyboard navigation #2268

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.

Sign up for GitHub

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

feat(core): refactor keyboard navigation #2268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8e433e2
feat(core): refactor keyboard navigation
danielleroux Nov 14, 2025
cc35d1c
add fork comment
danielleroux Nov 14, 2025
5d6ffdb
initial select keyboard nav
danielleroux Nov 17, 2025
3e470b5
select virtual focus
danielleroux Nov 17, 2025
4d9ce58
Add better focus order
danielleroux Nov 18, 2025
19dbd82
fix handling of select items
danielleroux Nov 18, 2025
5c7f55f
remove add new item by enter
danielleroux Nov 19, 2025
6d0e3ba
adapt select component test cases
danielleroux Nov 19, 2025
2136ccc
fix tests and major issue of the group
danielleroux Nov 19, 2025
df279b0
fix build
danielleroux Nov 19, 2025
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
8 changes: 8 additions & 0 deletions .changeset/thick-owls-give.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@siemens/ix-angular': patch
'@siemens/ix-react': patch
'@siemens/ix': patch
'@siemens/ix-vue': patch
---

Prevent _ix-group_ from crashing during construction
17 changes: 11 additions & 6 deletions packages/angular/src/components.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -817,20 +817,21 @@ export declare interface IxDrawer extends Components.IxDrawer {


@ProxyCmp({
inputs: ['anchor', 'closeBehavior', 'header', 'placement', 'positioningStrategy', 'show', 'suppressAutomaticPlacement', 'trigger'],
inputs: ['anchor', 'closeBehavior', 'disableFocusHandling', 'header', 'placement', 'positioningStrategy', 'show', 'suppressAutomaticPlacement', 'trigger'],
methods: ['updatePosition']
})
@Component({
selector: 'ix-dropdown',
changeDetection: ChangeDetectionStrategy.OnPush,
template: '<ng-content></ng-content>',
// eslint-disable-next-line @angular-eslint/no-inputs-metadata-property
inputs: ['anchor', 'closeBehavior', 'header', 'placement', 'positioningStrategy', 'show', 'suppressAutomaticPlacement', 'trigger'],
outputs: ['showChanged'],
inputs: ['anchor', 'closeBehavior', 'disableFocusHandling', 'header', 'placement', 'positioningStrategy', 'show', 'suppressAutomaticPlacement', 'trigger'],
outputs: ['showChange', 'showChanged'],
standalone: false
})
export class IxDropdown {
protected el: HTMLIxDropdownElement;
@Output() showChange = new EventEmitter<CustomEvent<boolean>>();
@Output() showChanged = new EventEmitter<CustomEvent<boolean>>();
constructor(c: ChangeDetectorRef, r: ElementRef, protected z: NgZone) {
c.detach();
Expand All @@ -841,7 +842,11 @@ export class IxDropdown {

export declare interface IxDropdown extends Components.IxDropdown {
/**
* Fire event after visibility of dropdown has changed
* Fire event before visibility of dropdown has changed, preventing event will cancel showing dropdown
*/
showChange: EventEmitter<CustomEvent<boolean>>;
/**
* Fire event after visibility of dropdown has changed, preventing event will prevent nothing
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 14, 2025

Choose a reason for hiding this comment

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

medium

The phrase "preventing event will prevent nothing" is a bit confusing. It could be rephrased for better clarity, for example: "This event is not preventable." or "Calling preventDefault() on this event has no effect." This comment also applies to other generated files where this documentation appears.

Suggested change
* Fire event after visibility of dropdown has changed, preventing event will prevent nothing
* Fire event after visibility of dropdown has changed. This event is not preventable.

*/
showChanged: EventEmitter<CustomEvent<boolean>>;
}
Expand Down Expand Up @@ -2306,15 +2311,15 @@ export declare interface IxRow extends Components.IxRow {}


@ProxyCmp({
inputs: ['allowClear', 'ariaLabelChevronDownIconButton', 'ariaLabelClearIconButton', 'collapseMultipleSelection', 'disabled', 'dropdownMaxWidth', 'dropdownWidth', 'editable', 'helperText', 'hideListHeader', 'i18nAllSelected', 'i18nNoMatches', 'i18nPlaceholder', 'i18nPlaceholderEditable', 'i18nSelectListHeader', 'infoText', 'invalidText', 'label', 'mode', 'name', 'readonly', 'required', 'showTextAsTooltip', 'validText', 'value', 'warningText'],
inputs: ['allowClear', 'ariaLabelAddItem', 'ariaLabelChevronDownIconButton', 'ariaLabelClearIconButton', 'collapseMultipleSelection', 'disabled', 'dropdownMaxWidth', 'dropdownWidth', 'editable', 'helperText', 'hideListHeader', 'i18nAllSelected', 'i18nNoMatches', 'i18nPlaceholder', 'i18nPlaceholderEditable', 'i18nSelectListHeader', 'infoText', 'invalidText', 'label', 'mode', 'name', 'readonly', 'required', 'showTextAsTooltip', 'validText', 'value', 'warningText'],
methods: ['getNativeInputElement', 'focusInput']
})
@Component({
selector: 'ix-select',
changeDetection: ChangeDetectionStrategy.OnPush,
template: '<ng-content></ng-content>',
// eslint-disable-next-line @angular-eslint/no-inputs-metadata-property
inputs: ['allowClear', 'ariaLabelChevronDownIconButton', 'ariaLabelClearIconButton', 'collapseMultipleSelection', 'disabled', 'dropdownMaxWidth', 'dropdownWidth', 'editable', 'helperText', 'hideListHeader', 'i18nAllSelected', 'i18nNoMatches', 'i18nPlaceholder', 'i18nPlaceholderEditable', 'i18nSelectListHeader', 'infoText', 'invalidText', 'label', 'mode', 'name', 'readonly', 'required', 'showTextAsTooltip', 'validText', 'value', 'warningText'],
inputs: ['allowClear', 'ariaLabelAddItem', 'ariaLabelChevronDownIconButton', 'ariaLabelClearIconButton', 'collapseMultipleSelection', 'disabled', 'dropdownMaxWidth', 'dropdownWidth', 'editable', 'helperText', 'hideListHeader', 'i18nAllSelected', 'i18nNoMatches', 'i18nPlaceholder', 'i18nPlaceholderEditable', 'i18nSelectListHeader', 'infoText', 'invalidText', 'label', 'mode', 'name', 'readonly', 'required', 'showTextAsTooltip', 'validText', 'value', 'warningText'],
outputs: ['valueChange', 'inputChange', 'addItem', 'ixBlur'],
standalone: false
})
Expand Down
17 changes: 11 additions & 6 deletions packages/angular/standalone/src/components.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -918,19 +918,20 @@ export declare interface IxDrawer extends Components.IxDrawer {

@ProxyCmp({
defineCustomElementFn: defineIxDropdown,
inputs: ['anchor', 'closeBehavior', 'header', 'placement', 'positioningStrategy', 'show', 'suppressAutomaticPlacement', 'trigger'],
inputs: ['anchor', 'closeBehavior', 'disableFocusHandling', 'header', 'placement', 'positioningStrategy', 'show', 'suppressAutomaticPlacement', 'trigger'],
methods: ['updatePosition']
})
@Component({
selector: 'ix-dropdown',
changeDetection: ChangeDetectionStrategy.OnPush,
template: '<ng-content></ng-content>',
// eslint-disable-next-line @angular-eslint/no-inputs-metadata-property
inputs: ['anchor', 'closeBehavior', 'header', 'placement', 'positioningStrategy', 'show', 'suppressAutomaticPlacement', 'trigger'],
outputs: ['showChanged'],
inputs: ['anchor', 'closeBehavior', 'disableFocusHandling', 'header', 'placement', 'positioningStrategy', 'show', 'suppressAutomaticPlacement', 'trigger'],
outputs: ['showChange', 'showChanged'],
})
export class IxDropdown {
protected el: HTMLIxDropdownElement;
@Output() showChange = new EventEmitter<CustomEvent<boolean>>();
@Output() showChanged = new EventEmitter<CustomEvent<boolean>>();
constructor(c: ChangeDetectorRef, r: ElementRef, protected z: NgZone) {
c.detach();
Expand All @@ -941,7 +942,11 @@ export class IxDropdown {

export declare interface IxDropdown extends Components.IxDropdown {
/**
* Fire event after visibility of dropdown has changed
* Fire event before visibility of dropdown has changed, preventing event will cancel showing dropdown
*/
showChange: EventEmitter<CustomEvent<boolean>>;
/**
* Fire event after visibility of dropdown has changed, preventing event will prevent nothing
*/
showChanged: EventEmitter<CustomEvent<boolean>>;
}
Expand Down Expand Up @@ -2407,15 +2412,15 @@ export declare interface IxRow extends Components.IxRow {}

@ProxyCmp({
defineCustomElementFn: defineIxSelect,
inputs: ['allowClear', 'ariaLabelChevronDownIconButton', 'ariaLabelClearIconButton', 'collapseMultipleSelection', 'disabled', 'dropdownMaxWidth', 'dropdownWidth', 'editable', 'helperText', 'hideListHeader', 'i18nAllSelected', 'i18nNoMatches', 'i18nPlaceholder', 'i18nPlaceholderEditable', 'i18nSelectListHeader', 'infoText', 'invalidText', 'label', 'mode', 'name', 'readonly', 'required', 'showTextAsTooltip', 'validText', 'value', 'warningText'],
inputs: ['allowClear', 'ariaLabelAddItem', 'ariaLabelChevronDownIconButton', 'ariaLabelClearIconButton', 'collapseMultipleSelection', 'disabled', 'dropdownMaxWidth', 'dropdownWidth', 'editable', 'helperText', 'hideListHeader', 'i18nAllSelected', 'i18nNoMatches', 'i18nPlaceholder', 'i18nPlaceholderEditable', 'i18nSelectListHeader', 'infoText', 'invalidText', 'label', 'mode', 'name', 'readonly', 'required', 'showTextAsTooltip', 'validText', 'value', 'warningText'],
methods: ['getNativeInputElement', 'focusInput']
})
@Component({
selector: 'ix-select',
changeDetection: ChangeDetectionStrategy.OnPush,
template: '<ng-content></ng-content>',
// eslint-disable-next-line @angular-eslint/no-inputs-metadata-property
inputs: ['allowClear', 'ariaLabelChevronDownIconButton', 'ariaLabelClearIconButton', 'collapseMultipleSelection', 'disabled', 'dropdownMaxWidth', 'dropdownWidth', 'editable', 'helperText', 'hideListHeader', 'i18nAllSelected', 'i18nNoMatches', 'i18nPlaceholder', 'i18nPlaceholderEditable', 'i18nSelectListHeader', 'infoText', 'invalidText', 'label', 'mode', 'name', 'readonly', 'required', 'showTextAsTooltip', 'validText', 'value', 'warningText'],
inputs: ['allowClear', 'ariaLabelAddItem', 'ariaLabelChevronDownIconButton', 'ariaLabelClearIconButton', 'collapseMultipleSelection', 'disabled', 'dropdownMaxWidth', 'dropdownWidth', 'editable', 'helperText', 'hideListHeader', 'i18nAllSelected', 'i18nNoMatches', 'i18nPlaceholder', 'i18nPlaceholderEditable', 'i18nSelectListHeader', 'infoText', 'invalidText', 'label', 'mode', 'name', 'readonly', 'required', 'showTextAsTooltip', 'validText', 'value', 'warningText'],
outputs: ['valueChange', 'inputChange', 'addItem', 'ixBlur'],
})
export class IxSelect {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/playwright.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ const config: PlaywrightTestConfig = {
command: 'pnpm run host-root',
port: 8080,
},
retries: process.env.CI ? 3 : 1,
retries: 3,
};

export default config;
12 changes: 12 additions & 0 deletions packages/core/scss/mixins/shadow-dom/_focus-visible.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* SPDX-FileCopyrightText: 2025 Siemens AG
*
* SPDX-License-Identifier: MIT
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
@mixin ix-focus-visible() {
outline: 1px solid var(--theme-color-focus-bdr) !important;
outline-offset: var(--theme-btn--focus--outline-offset) !important;
Comment on lines +10 to +11
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 14, 2025

Choose a reason for hiding this comment

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

medium

The use of !important can make styles difficult to override and debug. Is it possible to achieve the desired focus styling by increasing specificity or reordering CSS rules to avoid using !important?

}
66 changes: 64 additions & 2 deletions packages/core/src/components.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1295,6 +1295,11 @@ export namespace Components {
* @default 'both'
*/
"closeBehavior": CloseBehavior;
/**
* Suppress automatic focus when the dropdown is shown
* @default false
*/
"disableFocusHandling": boolean;
/**
* @default false
*/
Expand Down Expand Up @@ -1415,6 +1420,10 @@ export namespace Components {
"disabled": boolean;
"emitItemClick": () => Promise<void>;
"getDropdownItemElement": () => Promise<HTMLIxDropdownItemElement>;
/**
* @default false
*/
"hasVisualFocus": boolean;
/**
* Display hover state
* @default false
Expand All @@ -1436,6 +1445,11 @@ export namespace Components {
* @default false
*/
"suppressChecked": boolean;
/**
* Can has focus
* @default false
*/
"suppressFocus": boolean;
}
interface IxDropdownQuickActions {
}
Expand Down Expand Up @@ -2983,12 +2997,18 @@ export namespace Components {
* @default false
*/
"allowClear": boolean;
/**
* ARIA label for the add item
* @since TODO: Define
* @default 'Add item'
*/
"ariaLabelAddItem": string;
/**
* ARIA label for the chevron down icon button Will be set as aria-label on the nested HTML button element
* @since 3.2.0
* @default 'Open select dropdown'
*/
"ariaLabelChevronDownIconButton"?: string;
"ariaLabelChevronDownIconButton": string;
/**
* ARIA label for the clear icon button Will be set as aria-label on the nested HTML button element
* @since 3.2.0
Expand Down Expand Up @@ -3116,6 +3136,10 @@ export namespace Components {
}
interface IxSelectItem {
"getDropdownItemElement": () => Promise<HTMLIxDropdownItemElement>;
/**
* @default false
*/
"hasVisualFocus": boolean;
/**
* @default false
*/
Expand Down Expand Up @@ -4720,7 +4744,11 @@ declare global {
new (): HTMLIxDrawerElement;
};
interface HTMLIxDropdownElementEventMap {
"showChange": boolean;
"showChanged": boolean;
"experimentalRequestFocus": {
keyEvent: KeyboardEvent;
};
}
interface HTMLIxDropdownElement extends Components.IxDropdown, HTMLStencilElement {
addEventListener<K extends keyof HTMLIxDropdownElementEventMap>(type: K, listener: (this: HTMLIxDropdownElement, ev: IxDropdownCustomEvent<HTMLIxDropdownElementEventMap[K]>) => any, options?: boolean | AddEventListenerOptions): void;
Expand Down Expand Up @@ -7147,6 +7175,11 @@ declare namespace LocalJSX {
* @default 'both'
*/
"closeBehavior"?: CloseBehavior;
/**
* Suppress automatic focus when the dropdown is shown
* @default false
*/
"disableFocusHandling"?: boolean;
/**
* @default false
*/
Expand All @@ -7168,7 +7201,17 @@ declare namespace LocalJSX {
alignmentAxis?: number;
};
/**
* Fire event after visibility of dropdown has changed
* Will be fired only after dropdown changed visibility to "true"
*/
"onExperimentalRequestFocus"?: (event: IxDropdownCustomEvent<{
keyEvent: KeyboardEvent;
}>) => void;
/**
* Fire event before visibility of dropdown has changed, preventing event will cancel showing dropdown
*/
"onShowChange"?: (event: IxDropdownCustomEvent<boolean>) => void;
/**
* Fire event after visibility of dropdown has changed, preventing event will prevent nothing
*/
"onShowChanged"?: (event: IxDropdownCustomEvent<boolean>) => void;
"overwriteDropdownStyle"?: (delegate: {
Expand Down Expand Up @@ -7264,6 +7307,10 @@ declare namespace LocalJSX {
* @default false
*/
"disabled"?: boolean;
/**
* @default false
*/
"hasVisualFocus"?: boolean;
/**
* Display hover state
* @default false
Expand All @@ -7286,6 +7333,11 @@ declare namespace LocalJSX {
* @default false
*/
"suppressChecked"?: boolean;
/**
* Can has focus
* @default false
*/
"suppressFocus"?: boolean;
}
interface IxDropdownQuickActions {
}
Expand Down Expand Up @@ -8951,6 +9003,12 @@ declare namespace LocalJSX {
* @default false
*/
"allowClear"?: boolean;
/**
* ARIA label for the add item
* @since TODO: Define
* @default 'Add item'
*/
"ariaLabelAddItem"?: string;
/**
* ARIA label for the chevron down icon button Will be set as aria-label on the nested HTML button element
* @since 3.2.0
Expand Down Expand Up @@ -9085,6 +9143,10 @@ declare namespace LocalJSX {
"warningText"?: string;
}
interface IxSelectItem {
/**
* @default false
*/
"hasVisualFocus"?: boolean;
/**
* @default false
*/
Expand Down
7 changes: 7 additions & 0 deletions packages/core/src/components/avatar/avatar.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
@use 'mixins/text-truncation';
@use 'mixins/hover';
@use 'mixins/scrollbar';
@use 'mixins/shadow-dom/focus-visible' as focus-visible;

:host {
display: flex;
Expand Down Expand Up @@ -119,3 +120,9 @@
transform: scale(0.8);
}
}

:host(.avatar-button.ix-focused) {
button {
@include focus-visible.ix-focus-visible();
}
}
5 changes: 4 additions & 1 deletion packages/core/src/components/avatar/avatar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,10 @@ export class Avatar {

if (this.isClosestApplicationHeader) {
return (
<Host slot="ix-application-header-avatar" class={'avatar-button'}>
<Host
slot="ix-application-header-avatar"
class={'avatar-button ix-focusable'}
>
<BaseButton
disabled={false}
iconOval={false}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/components/button/base-button.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export const BaseButton: FunctionalComponent<BaseButtonProps> = (

const commonAttributes = {
...ariaAttributes,
tabindex: props.disabled ? -1 : (props.tabIndex ?? 0),
// tabindex: props.disabled ? -1 : (props.tabIndex ?? 0),
class: {
...getButtonClasses(
props.variant,
Expand Down
27 changes: 20 additions & 7 deletions packages/core/src/components/button/button-mixin.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@
border-top-right-radius: var(--ix-button-border-radius-right);
border-bottom-right-radius: var(--ix-button-border-radius-right);

&,
&.focus,
&:focus-visible {
& {
background-color: var(--theme-btn-#{$name}--background);
color: var(--theme-btn-#{$name}--color);

Expand All @@ -32,10 +30,10 @@
border-style: solid;
}

@include hover.focus-visible {
outline: 1px solid var(--theme-color-focus-bdr);
outline-offset: var(--theme-btn--focus--outline-offset);
}
// @include hover.focus-visible {
// outline: 1px solid var(--theme-color-focus-bdr);
// outline-offset: var(--theme-btn--focus--outline-offset);
// }

&.selected {
background-color: var(--theme-btn-#{$name}--background--pressed);
Expand Down Expand Up @@ -160,6 +158,21 @@
:host(.disabled) {
cursor: default;
}

:host(.ix-focused) {
.btn {
outline: 1px solid var(--theme-color-focus-bdr) !important;
outline-offset: var(--theme-btn--focus--outline-offset) !important;
Comment on lines +164 to +165
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Nov 14, 2025

Choose a reason for hiding this comment

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

medium

The use of !important here can make these styles hard to override. Could this be avoided by using higher specificity selectors or a different approach?

}
}

:host(:focus-visible) {
outline: none;

.btn {
outline: none;
}
}
}

@mixin all-btn-variants {
Expand Down
Loading
Loading