Skip to content

Commit caa2b3b

Browse files
committed
fix(multiple): prevent focus on disabled components (#32263)
(cherry picked from commit 4aad878)
1 parent e71befb commit caa2b3b

File tree

10 files changed

+29
-87
lines changed

10 files changed

+29
-87
lines changed

src/aria/accordion/accordion.spec.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -363,17 +363,10 @@ describe('AccordionGroup', () => {
363363
});
364364

365365
it('should not allow keyboard navigation if group is disabled', () => {
366-
configureAccordionComponent({disabledGroup: true, softDisabled: false});
367-
368-
downArrowKey(triggerElements[0]);
369-
expect(isTriggerActive(triggerElements[1])).toBeFalse();
370-
});
371-
372-
it('should allow keyboard navigation if group is disabled', () => {
373366
configureAccordionComponent({disabledGroup: true});
374367

375368
downArrowKey(triggerElements[0]);
376-
expect(isTriggerActive(triggerElements[1])).toBeTrue();
369+
expect(isTriggerActive(triggerElements[1])).toBeFalse();
377370
});
378371

379372
it('should not allow expansion if group is disabled', () => {

src/aria/listbox/listbox.spec.ts

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -195,14 +195,9 @@ describe('Listbox', () => {
195195
expect(listboxElement.getAttribute('tabindex')).toBe('-1');
196196
});
197197

198-
it('should set tabindex="0" for the listbox when disabled and focusMode is "roving when softDisabled is false"', () => {
199-
setupListbox({disabled: true, focusMode: 'roving', softDisabled: false});
200-
expect(listboxElement.getAttribute('tabindex')).toBe('0');
201-
});
202-
203-
it('should set tabindex="-1" for the listbox when disabled and focusMode is "roving"', () => {
198+
it('should set tabindex="0" for the listbox when disabled and focusMode is "roving"', () => {
204199
setupListbox({disabled: true, focusMode: 'roving'});
205-
expect(listboxElement.getAttribute('tabindex')).toBe('-1');
200+
expect(listboxElement.getAttribute('tabindex')).toBe('0');
206201
});
207202

208203
it('should set initial focus (tabindex="0") on the first non-disabled option if no value is set', () => {
@@ -645,7 +640,7 @@ describe('Listbox', () => {
645640
expect(isFocused(1)).toBe(true);
646641
});
647642

648-
it('should not skip disabled options with ArrowDown when completely disabled', () => {
643+
it('should not be focusable ArrowDown when completely disabled', () => {
649644
setupListbox({
650645
focusMode,
651646
orientation: 'vertical',
@@ -654,7 +649,7 @@ describe('Listbox', () => {
654649
});
655650

656651
down();
657-
expect(isFocused(0)).toBe(true);
652+
expect(isFocused(0)).toBe(false);
658653
});
659654
});
660655

src/aria/private/behaviors/grid/grid-focus.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ export class GridFocus<T extends GridFocusCell> {
143143

144144
/** Moves focus to the cell at the given coordinates if it's part of a focusable cell. */
145145
focusCoordinates(coords: RowCol): boolean {
146-
if (this.gridDisabled() && !this.inputs.softDisabled()) {
146+
if (this.gridDisabled()) {
147147
return false;
148148
}
149149

src/aria/private/behaviors/grid/grid-navigation.spec.ts

Lines changed: 9 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -158,13 +158,13 @@ describe('GridNavigation', () => {
158158
expect(gridNav.peek(direction.Up, from, 'continuous')).toEqual({row: 3, col: 2});
159159
});
160160

161-
it('should return the next coordinates even if all cells are disabled', () => {
161+
it('should return undefined if all cells are disabled', () => {
162162
cells.flat().forEach(cell => cell.disabled.set(true));
163163
gridNav.gotoCoords({row: 1, col: 0});
164164

165165
const nextCoords = gridNav.peek(direction.Up, gridFocus.activeCoords());
166166

167-
expect(nextCoords).toEqual({row: 0, col: 0});
167+
expect(nextCoords).toBeUndefined();
168168
});
169169

170170
it('should return undefined if all cells are disabled when softDisabled is false', () => {
@@ -210,23 +210,12 @@ describe('GridNavigation', () => {
210210
expect(gridNav.peek(direction.Down, from, 'continuous')).toEqual({row: 0, col: 2});
211211
});
212212

213-
it('should return the next coordinates even if all cells are disabled', () => {
213+
it('should return undefined if completely disabled', () => {
214214
cells.flat().forEach(cell => cell.disabled.set(true));
215215
gridNav.gotoCoords({row: 1, col: 0});
216216

217217
const nextCoords = gridNav.peek(direction.Down, gridFocus.activeCoords());
218218

219-
expect(nextCoords).toEqual({row: 2, col: 0});
220-
});
221-
222-
it('should return undefined if all cells are disabled when softDisabled is false', () => {
223-
const {gridNav} = setupGridNavigation(signal(cells), {
224-
softDisabled: signal(false),
225-
});
226-
cells.flat().forEach(cell => cell.disabled.set(true));
227-
228-
const nextCoords = gridNav.peek(direction.Down, {row: 1, col: 0});
229-
230219
expect(nextCoords).toBeUndefined();
231220
});
232221

@@ -262,22 +251,11 @@ describe('GridNavigation', () => {
262251
expect(gridNav.peek(direction.Left, from, 'continuous')).toEqual({row: 3, col: 2});
263252
});
264253

265-
it('should return the next coordinates even if all cells are disabled', () => {
266-
cells.flat().forEach(c => c.disabled.set(true));
267-
gridNav.gotoCoords({row: 1, col: 0});
268-
269-
const nextCoords = gridNav.peek(direction.Left, gridFocus.activeCoords());
270-
271-
expect(nextCoords).toEqual({row: 1, col: 2});
272-
});
273-
274-
it('should return undefined if all cells are disabled when softDisabled is false', () => {
275-
const {gridNav} = setupGridNavigation(signal(cells), {
276-
softDisabled: signal(false),
277-
});
254+
it('should return undefined if completely disabled', () => {
278255
cells.flat().forEach(cell => cell.disabled.set(true));
256+
gridNav.gotoCoords({row: 1, col: 0});
279257

280-
const nextCoords = gridNav.peek(direction.Left, {row: 1, col: 0});
258+
const nextCoords = gridNav.peek(direction.Down, gridFocus.activeCoords());
281259

282260
expect(nextCoords).toBeUndefined();
283261
});
@@ -314,22 +292,11 @@ describe('GridNavigation', () => {
314292
expect(gridNav.peek(direction.Right, from, 'continuous')).toEqual({row: 1, col: 0});
315293
});
316294

317-
it('should return the next coordinates even if all cells are disabled', () => {
318-
cells.flat().forEach(c => c.disabled.set(true));
319-
gridNav.gotoCoords({row: 1, col: 0});
320-
321-
const nextCoords = gridNav.peek(direction.Right, gridFocus.activeCoords());
322-
323-
expect(nextCoords).toEqual({row: 1, col: 1});
324-
});
325-
326-
it('should return undefined if all cells are disabled when softDisabled is false', () => {
327-
const {gridNav} = setupGridNavigation(signal(cells), {
328-
softDisabled: signal(false),
329-
});
295+
it('should return undefined if completely disabled', () => {
330296
cells.flat().forEach(cell => cell.disabled.set(true));
297+
gridNav.gotoCoords({row: 1, col: 0});
331298

332-
const nextCoords = gridNav.peek(direction.Right, {row: 1, col: 0});
299+
const nextCoords = gridNav.peek(direction.Down, gridFocus.activeCoords());
333300

334301
expect(nextCoords).toBeUndefined();
335302
});

src/aria/private/behaviors/grid/grid-navigation.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,8 @@ export class GridNavigation<T extends GridNavigationCell> {
146146
wrap: 'continuous' | 'loop' | 'nowrap',
147147
allowDisabled: boolean = false,
148148
): RowCol | undefined {
149+
if (this.inputs.gridFocus.gridDisabled()) return undefined;
150+
149151
const fromCell = this.inputs.grid.getCell(fromCoords);
150152
const maxRowCount = this.inputs.grid.maxRowCount();
151153
const maxColCount = this.inputs.grid.maxColCount();

src/aria/private/behaviors/grid/grid.spec.ts

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -354,23 +354,13 @@ describe('Grid', () => {
354354
it('should return false if no focusable cell is found when state is empty', () => {
355355
const cells = createTestGrid(createGridA);
356356
cells.flat().forEach(c => c.disabled.set(true));
357-
const grid = setupGrid(signal(cells), {softDisabled: signal(false)});
357+
const grid = setupGrid(signal(cells));
358358

359359
const result = grid.resetState();
360360
expect(result).toBe(false);
361361
expect(grid.focusBehavior.activeCell()).toBeUndefined();
362362
});
363363

364-
it('should return true and focus a cell if all cells are disabled but softDisabled is true', () => {
365-
const cells = createTestGrid(createGridA);
366-
cells.flat().forEach(c => c.disabled.set(true));
367-
const grid = setupGrid(signal(cells));
368-
369-
const result = grid.resetState();
370-
expect(result).toBe(true);
371-
expect(grid.focusBehavior.activeCell()).toBe(cells[0][0]);
372-
});
373-
374364
it('should re-focus the active cell if it is stale but still exists', () => {
375365
const cellsSignal = signal(createTestGrid(createGridA));
376366
const grid = setupGrid(cellsSignal);

src/aria/private/behaviors/list-focus/list-focus.ts

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -66,14 +66,9 @@ export class ListFocus<T extends ListFocusItem> {
6666
return this.inputs.disabled() || this.inputs.items().every(i => i.disabled());
6767
}
6868

69-
/** Whether the list is in a disabled state, but should still be focusable */
70-
isListDisabledFocusable(): boolean {
71-
return this.isListDisabled() && !this.inputs.softDisabled();
72-
}
73-
7469
/** The id of the current active item. */
7570
getActiveDescendant(): string | undefined {
76-
if (this.isListDisabledFocusable()) {
71+
if (this.isListDisabled()) {
7772
return undefined;
7873
}
7974
if (this.inputs.focusMode() === 'roving') {
@@ -84,15 +79,15 @@ export class ListFocus<T extends ListFocusItem> {
8479

8580
/** The tab index for the list. */
8681
getListTabIndex(): -1 | 0 {
87-
if (this.isListDisabledFocusable()) {
82+
if (this.isListDisabled()) {
8883
return 0;
8984
}
9085
return this.inputs.focusMode() === 'activedescendant' ? 0 : -1;
9186
}
9287

9388
/** Returns the tab index for the given item. */
9489
getItemTabIndex(item: T): -1 | 0 {
95-
if (this.isListDisabledFocusable()) {
90+
if (this.isListDisabled()) {
9691
return -1;
9792
}
9893
if (this.inputs.focusMode() === 'activedescendant') {
@@ -103,7 +98,7 @@ export class ListFocus<T extends ListFocusItem> {
10398

10499
/** Moves focus to the given item if it is focusable. */
105100
focus(item: T, opts?: {focusElement?: boolean}): boolean {
106-
if (this.isListDisabledFocusable() || !this.isFocusable(item)) {
101+
if (this.isListDisabled() || !this.isFocusable(item)) {
107102
return false;
108103
}
109104

src/aria/private/behaviors/list/list.spec.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -160,18 +160,18 @@ describe('List Behavior', () => {
160160
it('should not change active index on navigation', () => {
161161
expect(list.inputs.activeItem()).toBe(list.inputs.items()[0]);
162162
list.next();
163-
expect(list.inputs.activeItem()).toBe(list.inputs.items()[1]);
163+
expect(list.inputs.activeItem()).toBe(list.inputs.items()[0]);
164164
list.last();
165-
expect(list.inputs.activeItem()).toBe(list.inputs.items()[8]);
165+
expect(list.inputs.activeItem()).toBe(list.inputs.items()[0]);
166166
});
167167

168168
it('should not select items', () => {
169169
list.next({selectOne: true});
170170
expect(list.inputs.value()).toEqual([]);
171171
});
172172

173-
it('should have a tabindex of 0', () => {
174-
expect(list.tabIndex()).toBe(-1);
173+
it('should have a tab index of 0', () => {
174+
expect(list.tabIndex()).toBe(0);
175175
});
176176
});
177177

src/aria/private/behaviors/list/list.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ export class List<T extends ListItem<V>, V> {
226226

227227
const moved = operation();
228228

229-
if (moved && !this.disabled()) {
229+
if (moved) {
230230
this.updateSelection(opts);
231231
}
232232

src/aria/tree/tree.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,10 +358,10 @@ describe('Tree', () => {
358358
expect(treeElement.getAttribute('tabindex')).toBe('0');
359359
});
360360

361-
it('should set tabindex="0" for the tree when disabled', () => {
361+
it('should set tabindex="0" for the tree when disabled when softDisabled is true', () => {
362362
updateTree({disabled: true, focusMode: 'roving'});
363363

364-
expect(treeElement.getAttribute('tabindex')).toBe('-1');
364+
expect(treeElement.getAttribute('tabindex')).toBe('0');
365365
});
366366

367367
it('should set initial focus (tabindex="0") on the first non-disabled item if no value is set', () => {

0 commit comments

Comments
 (0)