Skip to content

Commit

Permalink
fix(material/chips): navigate between rows on up/down arrow (#29364)
Browse files Browse the repository at this point in the history
Currently the up/down arrows behave in the same way as left/right, however that's incorrect based on [the reference implementation](https://www.w3.org/WAI/ARIA/apg/patterns/grid/examples/layout-grids/#ex2_label) which shows them navigating between the rows.

These changes update the logic in the chip grid so that it matches the expected behavior.

Fixes #29359.

(cherry picked from commit 0519174)
  • Loading branch information
crisbeto committed Jul 3, 2024
1 parent f61b05f commit 5a97c03
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 27 deletions.
83 changes: 61 additions & 22 deletions src/material/chips/chip-grid.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ import {Direction, Directionality} from '@angular/cdk/bidi';
import {
BACKSPACE,
DELETE,
DOWN_ARROW,
END,
ENTER,
HOME,
LEFT_ARROW,
RIGHT_ARROW,
SPACE,
TAB,
UP_ARROW,
} from '@angular/cdk/keycodes';
import {
createKeyboardEvent,
Expand Down Expand Up @@ -309,6 +311,48 @@ describe('MDC-based MatChipGrid', () => {
.withContext('Expected focused item not to have changed.')
.toBe(previousActiveElement);
});

it('should focus primary action in next row when pressing DOWN ARROW on primary action', () => {
chips.first.focus();
expect(document.activeElement).toBe(primaryActions[0]);

dispatchKeyboardEvent(primaryActions[0], 'keydown', DOWN_ARROW);
fixture.detectChanges();

expect(document.activeElement).toBe(primaryActions[1]);
});

it('should focus primary action in previous row when pressing UP ARROW on primary action', () => {
const lastIndex = primaryActions.length - 1;
chips.last.focus();
expect(document.activeElement).toBe(primaryActions[lastIndex]);

dispatchKeyboardEvent(primaryActions[lastIndex], 'keydown', UP_ARROW);
fixture.detectChanges();

expect(document.activeElement).toBe(primaryActions[lastIndex - 1]);
});

it('should focus(trailing action in next row when pressing DOWN ARROW on(trailing action', () => {
trailingActions[0].focus();
expect(document.activeElement).toBe(trailingActions[0]);

dispatchKeyboardEvent(trailingActions[0], 'keydown', DOWN_ARROW);
fixture.detectChanges();

expect(document.activeElement).toBe(trailingActions[1]);
});

it('should focus trailing action in previous row when pressing UP ARROW on trailing action', () => {
const lastIndex = trailingActions.length - 1;
trailingActions[lastIndex].focus();
expect(document.activeElement).toBe(trailingActions[lastIndex]);

dispatchKeyboardEvent(trailingActions[lastIndex], 'keydown', UP_ARROW);
fixture.detectChanges();

expect(document.activeElement).toBe(trailingActions[lastIndex - 1]);
});
});

describe('RTL', () => {
Expand Down Expand Up @@ -1034,11 +1078,8 @@ describe('MDC-based MatChipGrid', () => {
template: `
<mat-chip-grid [tabIndex]="tabIndex" [role]="role" #chipGrid>
@for (i of chips; track i) {
<mat-chip-row
[editable]="editable">
{{name}} {{i + 1}}
</mat-chip-row>
}
<mat-chip-row [editable]="editable">{{name}} {{i + 1}}</mat-chip-row>
}
</mat-chip-grid>
<input name="test" [matChipInputFor]="chipGrid"/>`,
})
Expand All @@ -1056,8 +1097,8 @@ class StandardChipGrid {
<mat-label>Add a chip</mat-label>
<mat-chip-grid #chipGrid>
@for (chip of chips; track chip) {
<mat-chip-row (removed)="remove(chip)">{{chip}}</mat-chip-row>
}
<mat-chip-row (removed)="remove(chip)">{{chip}}</mat-chip-row>
}
</mat-chip-grid>
<input name="test" [matChipInputFor]="chipGrid"/>
</mat-form-field>
Expand All @@ -1081,10 +1122,10 @@ class FormFieldChipGrid {
<mat-label>New food...</mat-label>
<mat-chip-grid #chipGrid placeholder="Food" [formControl]="control">
@for (food of foods; track food) {
<mat-chip-row [value]="food.value" (removed)="remove(food)">
{{ food.viewValue }}
</mat-chip-row>
}
<mat-chip-row [value]="food.value" (removed)="remove(food)">
{{ food.viewValue }}
</mat-chip-row>
}
</mat-chip-grid>
<input
[matChipInputFor]="chipGrid"
Expand Down Expand Up @@ -1143,10 +1184,8 @@ class InputChipGrid {
<mat-form-field>
<mat-chip-grid #chipGrid [formControl]="formControl">
@for (food of foods; track food) {
<mat-chip-row [value]="food.value">
{{food.viewValue}}
</mat-chip-row>
}
<mat-chip-row [value]="food.value">{{food.viewValue}}</mat-chip-row>
}
</mat-chip-grid>
<input name="test" [matChipInputFor]="chipGrid"/>
<mat-hint>Please select a chip, or type to add a new chip</mat-hint>
Expand Down Expand Up @@ -1179,8 +1218,8 @@ class ChipGridWithFormErrorMessages {
template: `
<mat-chip-grid #chipGrid>
@for (i of numbers; track i) {
<mat-chip-row (removed)="remove(i)">{{i}}</mat-chip-row>
}
<mat-chip-row (removed)="remove(i)">{{i}}</mat-chip-row>
}
<input name="test" [matChipInputFor]="chipGrid"/>
</mat-chip-grid>`,
animations: [
Expand Down Expand Up @@ -1208,11 +1247,11 @@ class StandardChipGridWithAnimations {
<mat-form-field>
<mat-chip-grid #chipGrid>
@for (i of chips; track i) {
<mat-chip-row [value]="i" (removed)="removeChip($event)">
Chip {{i + 1}}
<span matChipRemove>Remove</span>
</mat-chip-row>
}
<mat-chip-row [value]="i" (removed)="removeChip($event)">
Chip {{i + 1}}
<span matChipRemove>Remove</span>
</mat-chip-row>
}
</mat-chip-grid>
<input name="test" [matChipInputFor]="chipGrid"/>
</mat-form-field>
Expand Down
31 changes: 26 additions & 5 deletions src/material/chips/chip-grid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import {Directionality} from '@angular/cdk/bidi';
import {hasModifierKey, TAB} from '@angular/cdk/keycodes';
import {DOWN_ARROW, hasModifierKey, TAB, UP_ARROW} from '@angular/cdk/keycodes';
import {
AfterContentInit,
AfterViewInit,
Expand Down Expand Up @@ -426,7 +426,10 @@ export class MatChipGrid

/** Handles custom keyboard events. */
override _handleKeydown(event: KeyboardEvent) {
if (event.keyCode === TAB) {
const keyCode = event.keyCode;
const activeItem = this._keyManager.activeItem;

if (keyCode === TAB) {
if (
this._chipInput.focused &&
hasModifierKey(event, 'shiftKey') &&
Expand All @@ -435,8 +438,8 @@ export class MatChipGrid
) {
event.preventDefault();

if (this._keyManager.activeItem) {
this._keyManager.setActiveItem(this._keyManager.activeItem);
if (activeItem) {
this._keyManager.setActiveItem(activeItem);
} else {
this._focusLastChip();
}
Expand All @@ -447,7 +450,25 @@ export class MatChipGrid
super._allowFocusEscape();
}
} else if (!this._chipInput.focused) {
super._handleKeydown(event);
// The up and down arrows are supposed to navigate between the individual rows in the grid.
// We do this by filtering the actions down to the ones that have the same `_isPrimary`
// flag as the active action and moving focus between them ourseles instead of delegating
// to the key manager. For more information, see #29359 and:
// https://www.w3.org/WAI/ARIA/apg/patterns/grid/examples/layout-grids/#ex2_label
if ((keyCode === UP_ARROW || keyCode === DOWN_ARROW) && activeItem) {
const eligibleActions = this._chipActions.filter(
action => action._isPrimary === activeItem._isPrimary && !this._skipPredicate(action),
);
const currentIndex = eligibleActions.indexOf(activeItem);
const delta = event.keyCode === UP_ARROW ? -1 : 1;

event.preventDefault();
if (currentIndex > -1 && this._isValidIndex(currentIndex + delta)) {
this._keyManager.setActiveItem(eligibleActions[currentIndex + delta]);
}
} else {
super._handleKeydown(event);
}
}

this.stateChanges.next();
Expand Down

0 comments on commit 5a97c03

Please sign in to comment.