Skip to content

fix(core): Combobox, RovingTabIndex and ATFocus controller fixes for rh-select #2963

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
adamjohnson wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix(core): Combobox, RovingTabIndex and ATFocus controller fixes for rh-select #2963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aae6b06
fix(core): if `relatedTarget` is toggle, let `#onClickButton` manage …
adamjohnson Jan 20, 2026
66793e6
fix(core): ensure RTI syncs AT focus when using a combo of mouse and …
adamjohnson Jan 20, 2026
97e9288
fix(core): force to always search forward for Home and backward for …
adamjohnson Jan 20, 2026
4132c4c
fix(core): hide listbox on Shift+Tab when moving to the toggle button
adamjohnson Jan 21, 2026
ba6a0dc
feat(core): add optional `setItems` callback to listbox and combobox …
adamjohnson Feb 2, 2026
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
7 changes: 6 additions & 1 deletion core/pfe-core/controllers/at-focus-controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,13 @@ export abstract class ATFocusController<Item extends HTMLElement> {

set atFocusedItemIndex(index: number) {
const previousIndex = this.#atFocusedItemIndex;
const direction = index > previousIndex ? 1 : -1;
const { items, atFocusableItems } = this;
// - Home (index=0): always search forward to find first focusable item
// - End (index=last): always search backward to find last focusable item
// - Other cases: use comparison to determine direction
const direction = index === 0 ? 1
: index >= items.length - 1 ? -1
: index > previousIndex ? 1 : -1;
const itemsIndexOfLastATFocusableItem = items.indexOf(this.atFocusableItems.at(-1)!);
let itemToGainFocus = items.at(index);
let itemToGainFocusIsFocusable = atFocusableItems.includes(itemToGainFocus!);
Expand Down
51 changes: 47 additions & 4 deletions core/pfe-core/controllers/combobox-controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,17 @@ export interface ComboboxControllerOptions<Item extends HTMLElement> extends
* By default, toggles the `hidden` attribute on the item
*/
setItemHidden?(item: Item, hidden: boolean): void;
/**
* Optional. When provided, passed to ListboxController so it does not set
* aria-setsize/aria-posinset on items.
*/
setItems?(items: Item[]): void;
/**
* Optional. Returns position-in-set and set size for the focused item when
* building the Safari VoiceOver live-region announcement ("N of M").
* When not provided, the controller reads aria-posinset/aria-setsize from the item.
*/
getItemPosition?(item: Item, items: Item[]): { posInSet: number; setSize: number } | null;
}

/**
Expand Down Expand Up @@ -242,6 +253,7 @@ export class ComboboxController<
#button: HTMLElement | null = null;
#listbox: HTMLElement | null = null;
#buttonInitialRole: string | null = null;
#buttonHasMouseDown = false;
#mo = new MutationObserver(() => this.#initItems());
#microcopy = new Map<string, Record<Lang, string>>(Object.entries({
dimmed: {
Expand Down Expand Up @@ -351,6 +363,7 @@ export class ComboboxController<
getATFocusedItem: () => this.items[this.#fc?.atFocusedItemIndex ?? -1] ?? null,
isItemDisabled: this.options.isItemDisabled,
setItemSelected: this.options.setItemSelected,
setItems: this.options.setItems,
});
ComboboxController.instances.set(host, this);
ComboboxController.hosts.add(host);
Expand Down Expand Up @@ -425,6 +438,8 @@ export class ComboboxController<
#initButton() {
this.#button?.removeEventListener('click', this.#onClickButton);
this.#button?.removeEventListener('keydown', this.#onKeydownButton);
this.#button?.removeEventListener('mousedown', this.#onMousedownButton);
this.#button?.removeEventListener('mouseup', this.#onMouseupButton);
this.#button = this.options.getToggleButton();
if (!this.#button) {
throw new Error('ComboboxController getToggleButton() option must return an element');
Expand All @@ -434,6 +449,8 @@ export class ComboboxController<
this.#button.setAttribute('aria-controls', this.#listbox?.id ?? '');
this.#button.addEventListener('click', this.#onClickButton);
this.#button.addEventListener('keydown', this.#onKeydownButton);
this.#button.addEventListener('mousedown', this.#onMousedownButton);
this.#button.addEventListener('mouseup', this.#onMouseupButton);
}

#initInput() {
Expand Down Expand Up @@ -546,11 +563,21 @@ export class ComboboxController<
if (this.#lb.isSelected(item)) {
text += `, (${this.#translate('selected', langKey)})`;
}
if (item.hasAttribute('aria-setsize') && item.hasAttribute('aria-posinset')) {
const position =
typeof this.options.getItemPosition === 'function' ?
this.options.getItemPosition(item, this.items)
: null;
const posInSet =
position?.posInSet
?? (item.hasAttribute('aria-posinset') ? item.getAttribute('aria-posinset') : null);
const setSize =
position?.setSize
?? (item.hasAttribute('aria-setsize') ? item.getAttribute('aria-setsize') : null);
if (posInSet != null && setSize != null) {
if (langKey === 'ja') {
text += `, (${item.getAttribute('aria-setsize')} 件中 ${item.getAttribute('aria-posinset')} 件目)`;
text += `, (${setSize} 件中 ${posInSet} 件目)`;
} else {
text += `, (${item.getAttribute('aria-posinset')} ${this.#translate('of', langKey)} ${item.getAttribute('aria-setsize')})`;
text += `, (${posInSet} ${this.#translate('of', langKey)} ${setSize})`;
}
}
ComboboxController.#alert.lang = lang;
Expand Down Expand Up @@ -580,6 +607,17 @@ export class ComboboxController<
}
};

/**
* Distinguish click-to-toggle vs Tab/Shift+Tab
*/
#onMousedownButton = () => {
this.#buttonHasMouseDown = true;
};

#onMouseupButton = () => {
this.#buttonHasMouseDown = false;
};

#onClickListbox = (event: MouseEvent) => {
if (!this.multi && event.composedPath().some(this.options.isItem)) {
this.#hide();
Expand Down Expand Up @@ -735,9 +773,14 @@ export class ComboboxController<
#onFocusoutListbox = (event: FocusEvent) => {
if (!this.#hasTextInput && this.options.isExpanded()) {
const root = this.#element?.getRootNode();
// Check if focus moved to the toggle button via mouse click
// If so, let the click handler manage toggle (prevents double-toggle)
// But if focus moved via Shift+Tab (no mousedown), we should still hide
const isClickOnToggleButton =
event.relatedTarget === this.#button && this.#buttonHasMouseDown;
if ((root instanceof ShadowRoot || root instanceof Document)
&& !this.items.includes(event.relatedTarget as Item)
) {
&& !isClickOnToggleButton) {
this.#hide();
}
}
Expand Down
26 changes: 19 additions & 7 deletions core/pfe-core/controllers/listbox-controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ export interface ListboxControllerOptions<Item extends HTMLElement> {
* a combobox input.
*/
getControlsElements?(): HTMLElement[];
/**
* Optional callback when items are set. When provided, the controller does **not**
* set aria-setsize/aria-posinset on each item; the caller is responsible for list
* semantics (e.g. via ElementInternals).
*/
setItems?(items: Item[]): void;
}

/**
Expand Down Expand Up @@ -192,16 +198,22 @@ export class ListboxController<Item extends HTMLElement> implements ReactiveCont
}

/**
* register's the host's Item elements as listbox controller items
* sets aria-setsize and aria-posinset on items
* @param items items
* Registers the host's Item elements as listbox controller items.
* If options provides a setItems function, that function is called with the items.
* Otherwise, sets aria-setsize and aria-posinset on each item.
* @param items - The Item elements to register
*/
set items(items: Item[]) {
this.#items = items;
this.#items.forEach((item, index, _items) => {
item.ariaSetSize = _items.length.toString();
item.ariaPosInSet = (index + 1).toString();
});
const { setItems } = this.#options;
if (typeof setItems === 'function') {
setItems(items);
} else {
this.#items.forEach((item, index, _items) => {
item.ariaSetSize = _items.length.toString();
item.ariaPosInSet = (index + 1).toString();
});
}
}

/**
Expand Down
15 changes: 15 additions & 0 deletions core/pfe-core/controllers/roving-tabindex-controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,21 @@ export class RovingTabindexController<
if (container instanceof HTMLElement) {
container.addEventListener('focusin', () =>
this.#gainedInitialFocus = true, { once: true });
// Sync atFocusedItemIndex when an item receives DOM focus (e.g., via mouse click)
// This ensures keyboard navigation starts from the correct position
container.addEventListener('focusin', (event: FocusEvent) => {
const target = event.target as Item;
const index = this.items.indexOf(target);
// Only update if the target is a valid item and index differs
if (index >= 0 && index !== this.atFocusedItemIndex) {
// Update index via setter, but avoid the focus() call by temporarily
// clearing #gainedInitialFocus to prevent redundant focus
const hadInitialFocus = this.#gainedInitialFocus;
this.#gainedInitialFocus = false;
this.atFocusedItemIndex = index;
this.#gainedInitialFocus = hadInitialFocus;
}
});
} else {
this.#logger.warn('RovingTabindexController requires a getItemsContainer function');
}
Expand Down
Loading