Skip to content

Commit 94ab09a

Browse files
authored
refactor(cdk-experimental/ui-patterns): track focus by item not index (#31644)
* refactor(cdk-experimental/ui-patterns): track focus by item not index * fixup! refactor(cdk-experimental/ui-patterns): track focus by item not index
1 parent 4bf8ebf commit 94ab09a

File tree

27 files changed

+392
-392
lines changed

27 files changed

+392
-392
lines changed

src/cdk-experimental/accordion/accordion.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,9 @@ export class CdkAccordionGroup {
177177
/** The UI pattern instance for this accordion group. */
178178
readonly pattern: AccordionGroupPattern = new AccordionGroupPattern({
179179
...this,
180-
// TODO(ok7sai): Consider making `activeIndex` an internal state in the pattern and call
180+
// TODO(ok7sai): Consider making `activeItem` an internal state in the pattern and call
181181
// `setDefaultState` in the CDK.
182-
activeIndex: signal(0),
182+
activeItem: signal(undefined),
183183
items: computed(() => this._triggers().map(trigger => trigger.pattern)),
184184
expandedIds: this.value,
185185
// TODO(ok7sai): Investigate whether an accordion should support horizontal mode.

src/cdk-experimental/listbox/listbox.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ export class CdkListbox<V> {
103103
pattern: ListboxPattern<V> = new ListboxPattern<V>({
104104
...this,
105105
items: this.items,
106-
activeIndex: signal(0), // TODO: Use linkedSignal to ensure this doesn't get fked up.
106+
activeItem: signal(undefined),
107107
textDirection: this.textDirection,
108108
});
109109

src/cdk-experimental/radio-group/radio-group.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ export class CdkRadioGroup<V> {
129129
...this,
130130
items: this.items,
131131
value: this._value,
132-
activeIndex: signal(0),
132+
activeItem: signal(undefined),
133133
textDirection: this.textDirection,
134134
});
135135

src/cdk-experimental/tabs/tabs.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ export class CdkTabList implements OnInit, OnDestroy {
173173
...this,
174174
items: this.tabs,
175175
value: this._selection,
176-
activeIndex: signal(0),
176+
activeItem: signal(undefined),
177177
});
178178

179179
/** Whether the tree has received focus yet. */

src/cdk-experimental/tree/tree.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ export class CdkTree<V> {
126126
allItems: computed(() =>
127127
[...this._unorderedItems()].sort(sortDirectives).map(item => item.pattern),
128128
),
129-
activeIndex: signal(0),
129+
activeItem: signal(undefined),
130130
});
131131

132132
/** Whether the tree has received focus yet. */

src/cdk-experimental/ui-patterns/accordion/accordion.spec.ts

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ describe('Accordion Pattern', () => {
6161
groupInputs = {
6262
orientation: signal('vertical'),
6363
textDirection: signal('ltr'),
64-
activeIndex: signal(0),
64+
activeItem: signal(undefined),
6565
disabled: signal(false),
6666
multiExpandable: signal(true),
6767
items: signal([]),
@@ -104,6 +104,8 @@ describe('Accordion Pattern', () => {
104104
new AccordionTriggerPattern(triggerInputs[2]),
105105
];
106106

107+
groupPattern.inputs.activeItem.set(triggerPatterns[0]);
108+
107109
// Initiate a list of AccordionPanelPattern.
108110
panelInputs = [
109111
{
@@ -167,15 +169,15 @@ describe('Accordion Pattern', () => {
167169
});
168170

169171
it('navigates to first accordion trigger with home key.', () => {
170-
groupInputs.activeIndex.set(2);
172+
groupInputs.activeItem.set(groupInputs.items()[2]);
171173
expect(triggerPatterns[2].active()).toBeTrue();
172174
triggerPatterns[2].onKeydown(home());
173175
expect(triggerPatterns[2].active()).toBeFalse();
174176
expect(triggerPatterns[0].active()).toBeTrue();
175177
});
176178

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

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

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

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

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

233235
it('stays on the first trigger with up key from first trigger.', () => {
234-
groupInputs.activeIndex.set(0);
236+
groupInputs.activeItem.set(groupInputs.items()[0]);
235237
expect(triggerPatterns[0].active()).toBeTrue();
236238
triggerPatterns[0].onKeydown(up());
237239
expect(triggerPatterns[0].active()).toBeTrue();
238240
});
239241

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

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

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

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

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

301303
it('stays on the first trigger with left key from first trigger.', () => {
302-
groupInputs.activeIndex.set(0);
304+
groupInputs.activeItem.set(groupInputs.items()[0]);
303305
expect(triggerPatterns[0].active()).toBeTrue();
304306
triggerPatterns[0].onKeydown(left());
305307
expect(triggerPatterns[0].active()).toBeTrue();
306308
});
307309

308310
it('stays on the last trigger with right key from last trigger.', () => {
309-
groupInputs.activeIndex.set(2);
311+
groupInputs.activeItem.set(groupInputs.items()[2]);
310312
expect(triggerPatterns[2].active()).toBeTrue();
311313
triggerPatterns[2].onKeydown(right());
312314
expect(triggerPatterns[2].active()).toBeTrue();

src/cdk-experimental/ui-patterns/accordion/accordion.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export class AccordionGroupPattern {
4848
this.wrap = inputs.wrap;
4949
this.orientation = inputs.orientation;
5050
this.textDirection = inputs.textDirection;
51-
this.activeIndex = inputs.activeIndex;
51+
this.activeItem = inputs.activeItem;
5252
this.disabled = inputs.disabled;
5353
this.multiExpandable = inputs.multiExpandable;
5454
this.items = inputs.items;
@@ -70,8 +70,7 @@ export class AccordionGroupPattern {
7070
}
7171

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

101100
/** Whether the trigger is active. */
102-
active = computed(() => this.inputs.accordionGroup().focusManager.activeItem() === this);
101+
active = computed(() => this.inputs.accordionGroup().activeItem() === this);
103102

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

112+
/** The index of the trigger within its accordion group. */
113+
index = computed(() => this.inputs.accordionGroup().items().indexOf(this));
114+
113115
constructor(readonly inputs: AccordionTriggerInputs) {
114116
this.id = inputs.id;
115117
this.element = inputs.element;

src/cdk-experimental/ui-patterns/behaviors/list-focus/list-focus.spec.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,13 @@ type TestInputs = Partial<ListFocusInputs<ListFocusItem>> & {
1818
};
1919

2020
export function getListFocus(inputs: TestInputs = {}): ListFocus<ListFocusItem> {
21+
const items = inputs.items || getItems(inputs.numItems ?? 5);
2122
return new ListFocus({
22-
activeIndex: signal(0),
23+
activeItem: signal(items()[0]),
2324
disabled: signal(false),
2425
skipDisabled: signal(false),
2526
focusMode: signal('roving'),
26-
items: getItems(inputs.numItems ?? 5),
27+
items: items,
2728
...inputs,
2829
});
2930
}
@@ -35,6 +36,7 @@ function getItems(length: number): Signal<ListFocusItem[]> {
3536
id: signal(`${i}`),
3637
disabled: signal(false),
3738
element: signal({focus: () => {}} as HTMLElement),
39+
index: signal(i),
3840
};
3941
}),
4042
);
@@ -58,7 +60,7 @@ describe('List Focus', () => {
5860

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

8587
it('should set the tabindex of all items to -1', () => {
8688
const items = focusManager.inputs.items() as TestItem[];
87-
focusManager.inputs.activeIndex.set(0);
89+
focusManager.inputs.activeItem.set(focusManager.inputs.items()[0]);
8890
expect(focusManager.getItemTabindex(items[0])).toBe(-1);
8991
expect(focusManager.getItemTabindex(items[1])).toBe(-1);
9092
expect(focusManager.getItemTabindex(items[2])).toBe(-1);
@@ -93,7 +95,7 @@ describe('List Focus', () => {
9395
});
9496

9597
it('should update the activedescendant of the list when navigating', () => {
96-
focusManager.inputs.activeIndex.set(1);
98+
focusManager.inputs.activeItem.set(focusManager.inputs.items()[1]);
9799
expect(focusManager.getActiveDescendant()).toBe(focusManager.inputs.items()[1].id());
98100
});
99101
});

src/cdk-experimental/ui-patterns/behaviors/list-focus/list-focus.ts

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ export interface ListFocusItem {
1919

2020
/** Whether an item is disabled. */
2121
disabled: SignalLike<boolean>;
22+
23+
/** The index of the item in the list. */
24+
index: SignalLike<number>;
2225
}
2326

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

35-
/** The index of the current active item. */
36-
activeIndex: WritableSignalLike<number>;
38+
/** The active item. */
39+
activeItem: WritableSignalLike<T | undefined>;
3740

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

4245
/** Controls focus for a list of items. */
4346
export class ListFocus<T extends ListFocusItem> {
44-
/** The last index that was active. */
45-
prevActiveIndex = signal(0);
47+
/** The last item that was active. */
48+
prevActiveItem = signal<T | undefined>(undefined);
49+
50+
/** The index of the last item that was active. */
51+
prevActiveIndex = computed(() => this.prevActiveItem()?.index() ?? -1);
4652

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

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

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

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

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

93-
this.prevActiveIndex.set(this.inputs.activeIndex());
94-
const index = this.inputs.items().indexOf(item);
95-
this.inputs.activeIndex.set(index);
99+
this.prevActiveItem.set(this.inputs.activeItem());
100+
this.inputs.activeItem.set(item);
96101

97102
if (this.inputs.focusMode() === 'roving') {
98103
item.element().focus();

0 commit comments

Comments
 (0)