Skip to content

Commit 4cc879f

Browse files
committed
refactor(cdk-experimental/ui-patterns): track focus by item not index
1 parent 11f9228 commit 4cc879f

File tree

27 files changed

+373
-374
lines changed

27 files changed

+373
-374
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: 2 additions & 2 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;
@@ -99,7 +99,7 @@ export class AccordionTriggerPattern {
9999
expansionControl: ExpansionControl;
100100

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

104104
/** Id of the accordion panel controlled by the trigger. */
105105
controls = computed(() => this.inputs.accordionPanel()?.id());

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

Lines changed: 6 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
}
@@ -58,7 +59,7 @@ describe('List Focus', () => {
5859

5960
it('should set the tabindex based on the active index', () => {
6061
const items = focusManager.inputs.items() as TestItem[];
61-
focusManager.inputs.activeIndex.set(2);
62+
focusManager.inputs.activeItem.set(focusManager.inputs.items()[2]);
6263
expect(focusManager.getItemTabindex(items[0])).toBe(-1);
6364
expect(focusManager.getItemTabindex(items[1])).toBe(-1);
6465
expect(focusManager.getItemTabindex(items[2])).toBe(0);
@@ -84,7 +85,7 @@ describe('List Focus', () => {
8485

8586
it('should set the tabindex of all items to -1', () => {
8687
const items = focusManager.inputs.items() as TestItem[];
87-
focusManager.inputs.activeIndex.set(0);
88+
focusManager.inputs.activeItem.set(focusManager.inputs.items()[0]);
8889
expect(focusManager.getItemTabindex(items[0])).toBe(-1);
8990
expect(focusManager.getItemTabindex(items[1])).toBe(-1);
9091
expect(focusManager.getItemTabindex(items[2])).toBe(-1);
@@ -93,7 +94,7 @@ describe('List Focus', () => {
9394
});
9495

9596
it('should update the activedescendant of the list when navigating', () => {
96-
focusManager.inputs.activeIndex.set(1);
97+
focusManager.inputs.activeItem.set(focusManager.inputs.items()[1]);
9798
expect(focusManager.getActiveDescendant()).toBe(focusManager.inputs.items()[1].id());
9899
});
99100
});

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

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,20 +32,27 @@ export interface ListFocusInputs<T extends ListFocusItem> {
3232
/** The items in the list. */
3333
items: SignalLike<T[]>;
3434

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

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

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

47-
/** The current active item. */
48-
activeItem = computed(() => this.inputs.items()[this.inputs.activeIndex()]);
47+
/** The index of the last item that was active. */
48+
prevActiveIndex = computed(() => {
49+
return this.prevActiveItem() ? this.inputs.items().indexOf(this.prevActiveItem()!) : -1;
50+
});
51+
52+
/** The current active index in the list. */
53+
activeIndex = computed(() => {
54+
return this.inputs.activeItem() ? this.inputs.items().indexOf(this.inputs.activeItem()!) : -1;
55+
});
4956

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

@@ -62,7 +69,7 @@ export class ListFocus<T extends ListFocusItem> {
6269
if (this.inputs.focusMode() === 'roving') {
6370
return undefined;
6471
}
65-
return this.inputs.items()[this.inputs.activeIndex()].id();
72+
return this.inputs.activeItem()?.id() ?? undefined;
6673
}
6774

6875
/** The tabindex for the list. */
@@ -81,7 +88,7 @@ export class ListFocus<T extends ListFocusItem> {
8188
if (this.inputs.focusMode() === 'activedescendant') {
8289
return -1;
8390
}
84-
return this.activeItem() === item ? 0 : -1;
91+
return this.inputs.activeItem() === item ? 0 : -1;
8592
}
8693

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

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

97103
if (this.inputs.focusMode() === 'roving') {
98104
item.element().focus();

0 commit comments

Comments
 (0)