Skip to content

Commit 6eed316

Browse files
adamjohnsonadamjohnson
authored
Merge cdf6ea1 into f43c1bc 
2 parents f43c1bc + cdf6ea1 commit 6eed316

5 files changed

Lines changed: 124 additions & 15 deletions

File tree

‎core/pfe-core/controllers/at-focus-controller.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,13 @@ export abstract class ATFocusController<Item extends HTMLElement> {
4949

5050
set atFocusedItemIndex(index: number) {
5151
const previousIndex = this.#atFocusedItemIndex;
52-
const direction = index > previousIndex ? 1 : -1;
5352
const { items, atFocusableItems } = this;
53+
// - Home (index=0): always search forward to find first focusable item
54+
// - End (index=last): always search backward to find last focusable item
55+
// - Other cases: use comparison to determine direction
56+
const direction = index === 0 ? 1
57+
: index >= items.length - 1 ? -1
58+
: index > previousIndex ? 1 : -1;
5459
const itemsIndexOfLastATFocusableItem = items.indexOf(this.atFocusableItems.at(-1)!);
5560
let itemToGainFocus = items.at(index);
5661
let itemToGainFocusIsFocusable = atFocusableItems.includes(itemToGainFocus!);

‎core/pfe-core/controllers/combobox-controller.ts‎

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ export class ComboboxController<
242242
#button: HTMLElement | null = null;
243243
#listbox: HTMLElement | null = null;
244244
#buttonInitialRole: string | null = null;
245+
#buttonHasMouseDown = false;
245246
#mo = new MutationObserver(() => this.#initItems());
246247
#microcopy = new Map<string, Record<Lang, string>>(Object.entries({
247248
dimmed: {
@@ -425,6 +426,8 @@ export class ComboboxController<
425426
#initButton() {
426427
this.#button?.removeEventListener('click', this.#onClickButton);
427428
this.#button?.removeEventListener('keydown', this.#onKeydownButton);
429+
this.#button?.removeEventListener('mousedown', this.#onMousedownButton);
430+
this.#button?.removeEventListener('mouseup', this.#onMouseupButton);
428431
this.#button = this.options.getToggleButton();
429432
if (!this.#button) {
430433
throw new Error('ComboboxController getToggleButton() option must return an element');
@@ -434,6 +437,8 @@ export class ComboboxController<
434437
this.#button.setAttribute('aria-controls', this.#listbox?.id ?? '');
435438
this.#button.addEventListener('click', this.#onClickButton);
436439
this.#button.addEventListener('keydown', this.#onKeydownButton);
440+
this.#button.addEventListener('mousedown', this.#onMousedownButton);
441+
this.#button.addEventListener('mouseup', this.#onMouseupButton);
437442
}
438443

439444
#initInput() {
@@ -531,26 +536,32 @@ export class ComboboxController<
531536
return strings?.[lang] ?? key;
532537
}
533538

534-
// TODO(bennypowers): perhaps move this to ActivedescendantController
535-
#announce(item: Item) {
539+
/**
540+
* Announces the focused item to a live region (e.g. for Safari VoiceOver).
541+
* @param item - The listbox option item to announce.
542+
* TODO(bennypowers): perhaps move this to ActivedescendantController
543+
*/
544+
#announce(item: Item): void {
536545
const value = this.options.getItemValue(item);
537546
ComboboxController.#alert?.remove();
538547
const fragment = ComboboxController.#alertTemplate.content.cloneNode(true) as DocumentFragment;
539548
ComboboxController.#alert = fragment.firstElementChild as HTMLElement;
540549
let text = value;
541550
const lang = deepClosest(this.#listbox, '[lang]')?.getAttribute('lang') ?? 'en';
542-
const langKey = lang?.match(ComboboxController.langsRE)?.at(0) as Lang ?? 'en';
551+
const langKey = (lang?.match(ComboboxController.langsRE)?.at(0) as Lang) ?? 'en';
543552
if (this.options.isItemDisabled(item)) {
544553
text += ` (${this.#translate('dimmed', langKey)})`;
545554
}
546555
if (this.#lb.isSelected(item)) {
547556
text += `, (${this.#translate('selected', langKey)})`;
548557
}
549-
if (item.hasAttribute('aria-setsize') && item.hasAttribute('aria-posinset')) {
558+
const posInSet = InternalsController.getAriaPosInSet(item);
559+
const setSize = InternalsController.getAriaSetSize(item);
560+
if (posInSet != null && setSize != null) {
550561
if (langKey === 'ja') {
551-
text += `, (${item.getAttribute('aria-setsize')} 件中 ${item.getAttribute('aria-posinset')} 件目)`;
562+
text += `, (${setSize} 件中 ${posInSet} 件目)`;
552563
} else {
553-
text += `, (${item.getAttribute('aria-posinset')} ${this.#translate('of', langKey)} ${item.getAttribute('aria-setsize')})`;
564+
text += `, (${posInSet} ${this.#translate('of', langKey)} ${setSize})`;
554565
}
555566
}
556567
ComboboxController.#alert.lang = lang;
@@ -580,6 +591,17 @@ export class ComboboxController<
580591
}
581592
};
582593

594+
/**
595+
* Distinguish click-to-toggle vs Tab/Shift+Tab
596+
*/
597+
#onMousedownButton = () => {
598+
this.#buttonHasMouseDown = true;
599+
};
600+
601+
#onMouseupButton = () => {
602+
this.#buttonHasMouseDown = false;
603+
};
604+
583605
#onClickListbox = (event: MouseEvent) => {
584606
if (!this.multi && event.composedPath().some(this.options.isItem)) {
585607
this.#hide();
@@ -735,9 +757,14 @@ export class ComboboxController<
735757
#onFocusoutListbox = (event: FocusEvent) => {
736758
if (!this.#hasTextInput && this.options.isExpanded()) {
737759
const root = this.#element?.getRootNode();
760+
// Check if focus moved to the toggle button via mouse click
761+
// If so, let the click handler manage toggle (prevents double-toggle)
762+
// But if focus moved via Shift+Tab (no mousedown), we should still hide
763+
const isClickOnToggleButton =
764+
event.relatedTarget === this.#button && this.#buttonHasMouseDown;
738765
if ((root instanceof ShadowRoot || root instanceof Document)
739766
&& !this.items.includes(event.relatedTarget as Item)
740-
) {
767+
&& !isClickOnToggleButton) {
741768
this.#hide();
742769
}
743770
}

‎core/pfe-core/controllers/internals-controller.ts‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,63 @@ export class InternalsController implements ReactiveController, ARIAMixin {
8383
return Array.from(this.instances.get(host)?.internals.labels ?? []) as Element[];
8484
}
8585

86+
/**
87+
* Sets aria-posinset on a listbox item. Uses ElementInternals when the host has
88+
* an InternalsController instance; otherwise sets/removes the host attribute.
89+
* @param host - The listbox item element (option or option-like).
90+
* @param value - Position in set (1-based), or null to clear.
91+
*/
92+
public static setAriaPosInSet(host: Element, value: number | string | null): void {
93+
const instance = this.instances.get(host as unknown as ReactiveControllerHost);
94+
if (instance) {
95+
instance.ariaPosInSet = value != null ? String(value) : null;
96+
} else if (value != null) {
97+
host.setAttribute('aria-posinset', String(value));
98+
} else {
99+
host.removeAttribute('aria-posinset');
100+
}
101+
}
102+
103+
/**
104+
* Sets aria-setsize on a listbox item. Uses ElementInternals when the host has
105+
* an InternalsController instance; otherwise sets/removes the host attribute.
106+
* @param host - The listbox item element (option or option-like).
107+
* @param value - Total set size, or null to clear.
108+
*/
109+
public static setAriaSetSize(host: Element, value: number | string | null): void {
110+
const instance = this.instances.get(host as unknown as ReactiveControllerHost);
111+
if (instance) {
112+
instance.ariaSetSize = value != null ? String(value) : null;
113+
} else if (value != null) {
114+
host.setAttribute('aria-setsize', String(value));
115+
} else {
116+
host.removeAttribute('aria-setsize');
117+
}
118+
}
119+
120+
/**
121+
* Gets aria-posinset from a listbox item (internals or attribute).
122+
* @param host - The listbox item element.
123+
*/
124+
public static getAriaPosInSet(host: Element): string | null {
125+
const instance = this.instances.get(host as unknown as ReactiveControllerHost);
126+
return instance != null ?
127+
instance.ariaPosInSet
128+
: host.getAttribute('aria-posinset');
129+
}
130+
131+
/**
132+
* Gets aria-setsize from a listbox item (internals or attribute).
133+
* @param host - The listbox item element.
134+
*/
135+
public static getAriaSetSize(host: Element): string | null {
136+
const instance = this.instances.get(host as unknown as ReactiveControllerHost);
137+
return instance != null ?
138+
instance.ariaSetSize
139+
: host.getAttribute('aria-setsize');
140+
}
141+
142+
86143
public static isSafari: boolean =
87144
!isServer && /^((?!chrome|android).)*safari/i.test(navigator.userAgent);
88145

‎core/pfe-core/controllers/listbox-controller.ts‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { RequireProps } from '../core.ts';
33

44
import { isServer } from 'lit';
55
import { arraysAreEquivalent } from '../functions/arraysAreEquivalent.js';
6+
import { InternalsController } from './internals-controller.js';
67

78
/**
89
* Options for listbox controller
@@ -192,16 +193,11 @@ export class ListboxController<Item extends HTMLElement> implements ReactiveCont
192193
}
193194

194195
/**
195-
* register's the host's Item elements as listbox controller items
196-
* sets aria-setsize and aria-posinset on items
197-
* @param items items
196+
* Registers the host's item elements as listbox controller items.
197+
* @param items - Array of listbox option elements.
198198
*/
199199
set items(items: Item[]) {
200200
this.#items = items;
201-
this.#items.forEach((item, index, _items) => {
202-
item.ariaSetSize = _items.length.toString();
203-
item.ariaPosInSet = (index + 1).toString();
204-
});
205201
}
206202

207203
/**
@@ -268,6 +264,10 @@ export class ListboxController<Item extends HTMLElement> implements ReactiveCont
268264
}
269265
}
270266

267+
/**
268+
* Called during host update; syncs control element listeners and
269+
* applies aria-posinset/aria-setsize to each item via InternalsController.
270+
*/
271271
hostUpdate(): void {
272272
const last = this.#controlsElements;
273273
this.#controlsElements = this.#options.getControlsElements?.() ?? [];
@@ -278,6 +278,11 @@ export class ListboxController<Item extends HTMLElement> implements ReactiveCont
278278
el.addEventListener('keyup', this.#onKeyup);
279279
}
280280
}
281+
const items = this.#items;
282+
items.forEach((item, index) => {
283+
InternalsController.setAriaPosInSet(item, index + 1);
284+
InternalsController.setAriaSetSize(item, items.length);
285+
});
281286
}
282287

283288
hostUpdated(): void {

‎core/pfe-core/controllers/roving-tabindex-controller.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,21 @@ export class RovingTabindexController<
7777
if (container instanceof HTMLElement) {
7878
container.addEventListener('focusin', () =>
7979
this.#gainedInitialFocus = true, { once: true });
80+
// Sync atFocusedItemIndex when an item receives DOM focus (e.g., via mouse click)
81+
// This ensures keyboard navigation starts from the correct position
82+
container.addEventListener('focusin', (event: FocusEvent) => {
83+
const target = event.target as Item;
84+
const index = this.items.indexOf(target);
85+
// Only update if the target is a valid item and index differs
86+
if (index >= 0 && index !== this.atFocusedItemIndex) {
87+
// Update index via setter, but avoid the focus() call by temporarily
88+
// clearing #gainedInitialFocus to prevent redundant focus
89+
const hadInitialFocus = this.#gainedInitialFocus;
90+
this.#gainedInitialFocus = false;
91+
this.atFocusedItemIndex = index;
92+
this.#gainedInitialFocus = hadInitialFocus;
93+
}
94+
});
8095
} else {
8196
this.#logger.warn('RovingTabindexController requires a getItemsContainer function');
8297
}

0 commit comments

Comments
 (0)