Skip to content

refactor(cdk-experimental/ui-patterns): track focus by item not index #31644

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.

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

Merged
merged 2 commits into from
Aug 6, 2025
Merged
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
4 changes: 2 additions & 2 deletions src/cdk-experimental/accordion/accordion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,9 @@ export class CdkAccordionGroup {
/** The UI pattern instance for this accordion group. */
readonly pattern: AccordionGroupPattern = new AccordionGroupPattern({
...this,
// TODO(ok7sai): Consider making `activeIndex` an internal state in the pattern and call
// TODO(ok7sai): Consider making `activeItem` an internal state in the pattern and call
// `setDefaultState` in the CDK.
activeIndex: signal(0),
activeItem: signal(undefined),
items: computed(() => this._triggers().map(trigger => trigger.pattern)),
expandedIds: this.value,
// TODO(ok7sai): Investigate whether an accordion should support horizontal mode.
Expand Down
2 changes: 1 addition & 1 deletion src/cdk-experimental/listbox/listbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export class CdkListbox<V> {
pattern: ListboxPattern<V> = new ListboxPattern<V>({
...this,
items: this.items,
activeIndex: signal(0), // TODO: Use linkedSignal to ensure this doesn't get fked up.
activeItem: signal(undefined),
textDirection: this.textDirection,
});

Expand Down
2 changes: 1 addition & 1 deletion src/cdk-experimental/radio-group/radio-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ export class CdkRadioGroup<V> {
...this,
items: this.items,
value: this._value,
activeIndex: signal(0),
activeItem: signal(undefined),
textDirection: this.textDirection,
});

Expand Down
2 changes: 1 addition & 1 deletion src/cdk-experimental/tabs/tabs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ export class CdkTabList implements OnInit, OnDestroy {
...this,
items: this.tabs,
value: this._selection,
activeIndex: signal(0),
activeItem: signal(undefined),
});

/** Whether the tree has received focus yet. */
Expand Down
2 changes: 1 addition & 1 deletion src/cdk-experimental/tree/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export class CdkTree<V> {
allItems: computed(() =>
[...this._unorderedItems()].sort(sortDirectives).map(item => item.pattern),
),
activeIndex: signal(0),
activeItem: signal(undefined),
});

/** Whether the tree has received focus yet. */
Expand Down
32 changes: 17 additions & 15 deletions src/cdk-experimental/ui-patterns/accordion/accordion.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe('Accordion Pattern', () => {
groupInputs = {
orientation: signal('vertical'),
textDirection: signal('ltr'),
activeIndex: signal(0),
activeItem: signal(undefined),
disabled: signal(false),
multiExpandable: signal(true),
items: signal([]),
Expand Down Expand Up @@ -104,6 +104,8 @@ describe('Accordion Pattern', () => {
new AccordionTriggerPattern(triggerInputs[2]),
];

groupPattern.inputs.activeItem.set(triggerPatterns[0]);

// Initiate a list of AccordionPanelPattern.
panelInputs = [
{
Expand Down Expand Up @@ -167,15 +169,15 @@ describe('Accordion Pattern', () => {
});

it('navigates to first accordion trigger with home key.', () => {
groupInputs.activeIndex.set(2);
groupInputs.activeItem.set(groupInputs.items()[2]);
expect(triggerPatterns[2].active()).toBeTrue();
triggerPatterns[2].onKeydown(home());
expect(triggerPatterns[2].active()).toBeFalse();
expect(triggerPatterns[0].active()).toBeTrue();
});

it('navigates to last accordion trigger with end key.', () => {
groupInputs.activeIndex.set(0);
groupInputs.activeItem.set(groupInputs.items()[0]);
expect(triggerPatterns[0].active()).toBeTrue();
triggerPatterns[0].onKeydown(end());
expect(triggerPatterns[0].active()).toBeFalse();
Expand All @@ -184,7 +186,7 @@ describe('Accordion Pattern', () => {

describe('Vertical Orientation (orientation=vertical)', () => {
it('navigates to the next trigger with down key.', () => {
groupInputs.activeIndex.set(0);
groupInputs.activeItem.set(groupInputs.items()[0]);
expect(triggerPatterns[0].active()).toBeTrue();
expect(triggerPatterns[1].active()).toBeFalse();
triggerPatterns[0].onKeydown(down());
Expand All @@ -193,7 +195,7 @@ describe('Accordion Pattern', () => {
});

it('navigates to the previous trigger with up key.', () => {
groupInputs.activeIndex.set(1);
groupInputs.activeItem.set(groupInputs.items()[1]);
expect(triggerPatterns[0].active()).toBeFalse();
expect(triggerPatterns[1].active()).toBeTrue();
triggerPatterns[1].onKeydown(up());
Expand All @@ -207,7 +209,7 @@ describe('Accordion Pattern', () => {
});

it('navigates to the last trigger with up key from first trigger.', () => {
groupInputs.activeIndex.set(0);
groupInputs.activeItem.set(groupInputs.items()[0]);
expect(triggerPatterns[0].active()).toBeTrue();
expect(triggerPatterns[2].active()).toBeFalse();
triggerPatterns[0].onKeydown(up());
Expand All @@ -216,7 +218,7 @@ describe('Accordion Pattern', () => {
});

it('navigates to the first trigger with down key from last trigger.', () => {
groupInputs.activeIndex.set(2);
groupInputs.activeItem.set(groupInputs.items()[2]);
expect(triggerPatterns[0].active()).toBeFalse();
expect(triggerPatterns[2].active()).toBeTrue();
triggerPatterns[2].onKeydown(down());
Expand All @@ -231,14 +233,14 @@ describe('Accordion Pattern', () => {
});

it('stays on the first trigger with up key from first trigger.', () => {
groupInputs.activeIndex.set(0);
groupInputs.activeItem.set(groupInputs.items()[0]);
expect(triggerPatterns[0].active()).toBeTrue();
triggerPatterns[0].onKeydown(up());
expect(triggerPatterns[0].active()).toBeTrue();
});

it('stays on the last trigger with down key from last trigger.', () => {
groupInputs.activeIndex.set(2);
groupInputs.activeItem.set(groupInputs.items()[2]);
expect(triggerPatterns[2].active()).toBeTrue();
triggerPatterns[2].onKeydown(down());
expect(triggerPatterns[2].active()).toBeTrue();
Expand All @@ -252,7 +254,7 @@ describe('Accordion Pattern', () => {
});

it('navigates to the next trigger with right key.', () => {
groupInputs.activeIndex.set(0);
groupInputs.activeItem.set(groupInputs.items()[0]);
expect(triggerPatterns[0].active()).toBeTrue();
expect(triggerPatterns[1].active()).toBeFalse();
triggerPatterns[0].onKeydown(right());
Expand All @@ -261,7 +263,7 @@ describe('Accordion Pattern', () => {
});

it('navigates to the previous trigger with left key.', () => {
groupInputs.activeIndex.set(1);
groupInputs.activeItem.set(groupInputs.items()[1]);
expect(triggerPatterns[0].active()).toBeFalse();
expect(triggerPatterns[1].active()).toBeTrue();
triggerPatterns[1].onKeydown(left());
Expand All @@ -275,7 +277,7 @@ describe('Accordion Pattern', () => {
});

it('navigates to the last trigger with left key from first trigger.', () => {
groupInputs.activeIndex.set(0);
groupInputs.activeItem.set(groupInputs.items()[0]);
expect(triggerPatterns[0].active()).toBeTrue();
expect(triggerPatterns[2].active()).toBeFalse();
triggerPatterns[0].onKeydown(left());
Expand All @@ -284,7 +286,7 @@ describe('Accordion Pattern', () => {
});

it('navigates to the first trigger with right key from last trigger.', () => {
groupInputs.activeIndex.set(2);
groupInputs.activeItem.set(groupInputs.items()[2]);
expect(triggerPatterns[2].active()).toBeTrue();
expect(triggerPatterns[0].active()).toBeFalse();
triggerPatterns[2].onKeydown(right());
Expand All @@ -299,14 +301,14 @@ describe('Accordion Pattern', () => {
});

it('stays on the first trigger with left key from first trigger.', () => {
groupInputs.activeIndex.set(0);
groupInputs.activeItem.set(groupInputs.items()[0]);
expect(triggerPatterns[0].active()).toBeTrue();
triggerPatterns[0].onKeydown(left());
expect(triggerPatterns[0].active()).toBeTrue();
});

it('stays on the last trigger with right key from last trigger.', () => {
groupInputs.activeIndex.set(2);
groupInputs.activeItem.set(groupInputs.items()[2]);
expect(triggerPatterns[2].active()).toBeTrue();
triggerPatterns[2].onKeydown(right());
expect(triggerPatterns[2].active()).toBeTrue();
Expand Down
10 changes: 6 additions & 4 deletions src/cdk-experimental/ui-patterns/accordion/accordion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export class AccordionGroupPattern {
this.wrap = inputs.wrap;
this.orientation = inputs.orientation;
this.textDirection = inputs.textDirection;
this.activeIndex = inputs.activeIndex;
this.activeItem = inputs.activeItem;
this.disabled = inputs.disabled;
this.multiExpandable = inputs.multiExpandable;
this.items = inputs.items;
Expand All @@ -70,8 +70,7 @@ export class AccordionGroupPattern {
}

/** Inputs for the AccordionTriggerPattern. */
export type AccordionTriggerInputs = ListNavigationItem &
ListFocusItem &
export type AccordionTriggerInputs = Omit<ListNavigationItem & ListFocusItem, 'index'> &
Omit<ExpansionItem, 'expansionId' | 'expandable'> & {
/** A local unique identifier for the trigger. */
value: SignalLike<string>;
Expand Down Expand Up @@ -99,7 +98,7 @@ export class AccordionTriggerPattern {
expansionControl: ExpansionControl;

/** Whether the trigger is active. */
active = computed(() => this.inputs.accordionGroup().focusManager.activeItem() === this);
active = computed(() => this.inputs.accordionGroup().activeItem() === this);

/** Id of the accordion panel controlled by the trigger. */
controls = computed(() => this.inputs.accordionPanel()?.id());
Expand All @@ -110,6 +109,9 @@ export class AccordionTriggerPattern {
/** Whether the trigger is disabled. Disabling an accordion group disables all the triggers. */
disabled = computed(() => this.inputs.disabled() || this.inputs.accordionGroup().disabled());

/** The index of the trigger within its accordion group. */
index = computed(() => this.inputs.accordionGroup().items().indexOf(this));

constructor(readonly inputs: AccordionTriggerInputs) {
this.id = inputs.id;
this.element = inputs.element;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@ type TestInputs = Partial<ListFocusInputs<ListFocusItem>> & {
};

export function getListFocus(inputs: TestInputs = {}): ListFocus<ListFocusItem> {
const items = inputs.items || getItems(inputs.numItems ?? 5);
return new ListFocus({
activeIndex: signal(0),
activeItem: signal(items()[0]),
disabled: signal(false),
skipDisabled: signal(false),
focusMode: signal('roving'),
items: getItems(inputs.numItems ?? 5),
items: items,
...inputs,
});
}
Expand All @@ -35,6 +36,7 @@ function getItems(length: number): Signal<ListFocusItem[]> {
id: signal(`${i}`),
disabled: signal(false),
element: signal({focus: () => {}} as HTMLElement),
index: signal(i),
};
}),
);
Expand All @@ -58,7 +60,7 @@ describe('List Focus', () => {

it('should set the tabindex based on the active index', () => {
const items = focusManager.inputs.items() as TestItem[];
focusManager.inputs.activeIndex.set(2);
focusManager.inputs.activeItem.set(focusManager.inputs.items()[2]);
expect(focusManager.getItemTabindex(items[0])).toBe(-1);
expect(focusManager.getItemTabindex(items[1])).toBe(-1);
expect(focusManager.getItemTabindex(items[2])).toBe(0);
Expand All @@ -84,7 +86,7 @@ describe('List Focus', () => {

it('should set the tabindex of all items to -1', () => {
const items = focusManager.inputs.items() as TestItem[];
focusManager.inputs.activeIndex.set(0);
focusManager.inputs.activeItem.set(focusManager.inputs.items()[0]);
expect(focusManager.getItemTabindex(items[0])).toBe(-1);
expect(focusManager.getItemTabindex(items[1])).toBe(-1);
expect(focusManager.getItemTabindex(items[2])).toBe(-1);
Expand All @@ -93,7 +95,7 @@ describe('List Focus', () => {
});

it('should update the activedescendant of the list when navigating', () => {
focusManager.inputs.activeIndex.set(1);
focusManager.inputs.activeItem.set(focusManager.inputs.items()[1]);
expect(focusManager.getActiveDescendant()).toBe(focusManager.inputs.items()[1].id());
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ export interface ListFocusItem {

/** Whether an item is disabled. */
disabled: SignalLike<boolean>;

/** The index of the item in the list. */
index: SignalLike<number>;
}

/** Represents the required inputs for a collection that contains focusable items. */
Expand All @@ -32,20 +35,23 @@ export interface ListFocusInputs<T extends ListFocusItem> {
/** The items in the list. */
items: SignalLike<T[]>;

/** The index of the current active item. */
activeIndex: WritableSignalLike<number>;
/** The active item. */
activeItem: WritableSignalLike<T | undefined>;

/** Whether disabled items in the list should be skipped when navigating. */
skipDisabled: SignalLike<boolean>;
}

/** Controls focus for a list of items. */
export class ListFocus<T extends ListFocusItem> {
/** The last index that was active. */
prevActiveIndex = signal(0);
/** The last item that was active. */
prevActiveItem = signal<T | undefined>(undefined);

/** The index of the last item that was active. */
prevActiveIndex = computed(() => this.prevActiveItem()?.index() ?? -1);

/** The current active item. */
activeItem = computed(() => this.inputs.items()[this.inputs.activeIndex()]);
/** The current active index in the list. */
activeIndex = computed(() => this.inputs.activeItem()?.index() ?? -1);

constructor(readonly inputs: ListFocusInputs<T>) {}

Expand All @@ -62,7 +68,7 @@ export class ListFocus<T extends ListFocusItem> {
if (this.inputs.focusMode() === 'roving') {
return undefined;
}
return this.inputs.items()[this.inputs.activeIndex()].id();
return this.inputs.activeItem()?.id() ?? undefined;
}

/** The tabindex for the list. */
Expand All @@ -81,7 +87,7 @@ export class ListFocus<T extends ListFocusItem> {
if (this.inputs.focusMode() === 'activedescendant') {
return -1;
}
return this.activeItem() === item ? 0 : -1;
return this.inputs.activeItem() === item ? 0 : -1;
}

/** Moves focus to the given item if it is focusable. */
Expand All @@ -90,9 +96,8 @@ export class ListFocus<T extends ListFocusItem> {
return false;
}

this.prevActiveIndex.set(this.inputs.activeIndex());
const index = this.inputs.items().indexOf(item);
this.inputs.activeIndex.set(index);
this.prevActiveItem.set(this.inputs.activeItem());
this.inputs.activeItem.set(item);

if (this.inputs.focusMode() === 'roving') {
item.element().focus();
Expand Down
Loading
Loading