Skip to content

refactor(aria/grid): remove range selection to reduce computational overhead #33055

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.

Sign up for GitHub

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

Open
ok7sai wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor(aria/grid): remove range selection to reduce computational overhead #33055

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 3 additions & 4 deletions goldens/aria/grid/index.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ export class Grid {
readonly colWrap: _angular_core.InputSignal<"continuous" | "loop" | "nowrap">;
readonly disabled: _angular_core.InputSignalWithTransform<boolean, unknown>;
readonly element: HTMLElement;
readonly enableRangeSelection: _angular_core.InputSignalWithTransform<boolean, unknown>;
readonly enableSelection: _angular_core.InputSignalWithTransform<boolean, unknown>;
readonly focusMode: _angular_core.InputSignal<"roving" | "activedescendant">;
readonly multi: _angular_core.InputSignalWithTransform<boolean, unknown>;
Expand All @@ -25,7 +24,7 @@ export class Grid {
readonly softDisabled: _angular_core.InputSignalWithTransform<boolean, unknown>;
readonly textDirection: _angular_core.WritableSignal<_angular_cdk_bidi.Direction>;
// (undocumented)
static ɵdir: _angular_core.ɵɵDirectiveDeclaration<Grid, "[ngGrid]", ["ngGrid"], { "enableSelection": { "alias": "enableSelection"; "required": false; "isSignal": true; }; "disabled": { "alias": "disabled"; "required": false; "isSignal": true; }; "softDisabled": { "alias": "softDisabled"; "required": false; "isSignal": true; }; "focusMode": { "alias": "focusMode"; "required": false; "isSignal": true; }; "rowWrap": { "alias": "rowWrap"; "required": false; "isSignal": true; }; "colWrap": { "alias": "colWrap"; "required": false; "isSignal": true; }; "multi": { "alias": "multi"; "required": false; "isSignal": true; }; "selectionMode": { "alias": "selectionMode"; "required": false; "isSignal": true; }; "enableRangeSelection": { "alias": "enableRangeSelection"; "required": false; "isSignal": true; }; }, {}, ["_rows"], never, true, never>;
static ɵdir: _angular_core.ɵɵDirectiveDeclaration<Grid, "[ngGrid]", ["ngGrid"], { "enableSelection": { "alias": "enableSelection"; "required": false; "isSignal": true; }; "disabled": { "alias": "disabled"; "required": false; "isSignal": true; }; "softDisabled": { "alias": "softDisabled"; "required": false; "isSignal": true; }; "focusMode": { "alias": "focusMode"; "required": false; "isSignal": true; }; "rowWrap": { "alias": "rowWrap"; "required": false; "isSignal": true; }; "colWrap": { "alias": "colWrap"; "required": false; "isSignal": true; }; "multi": { "alias": "multi"; "required": false; "isSignal": true; }; "selectionMode": { "alias": "selectionMode"; "required": false; "isSignal": true; }; }, {}, ["_rows"], never, true, never>;
// (undocumented)
static ɵfac: _angular_core.ɵɵFactoryDeclaration<Grid, never>;
}
Expand Down Expand Up @@ -60,10 +59,10 @@ export class GridCell {
export class GridCellWidget {
constructor();
activate(): void;
readonly activated: _angular_core.OutputEmitterRef<FocusEvent | KeyboardEvent | undefined>;
readonly activated: _angular_core.OutputEmitterRef<KeyboardEvent | FocusEvent | undefined>;
readonly active: Signal<boolean>;
deactivate(): void;
readonly deactivated: _angular_core.OutputEmitterRef<FocusEvent | KeyboardEvent | undefined>;
readonly deactivated: _angular_core.OutputEmitterRef<KeyboardEvent | FocusEvent | undefined>;
readonly disabled: _angular_core.InputSignalWithTransform<boolean, unknown>;
readonly element: HTMLElement;
readonly focusTarget: _angular_core.InputSignal<ElementRef<any> | HTMLElement | undefined>;
Expand Down
5 changes: 0 additions & 5 deletions goldens/aria/private/index.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,6 @@ export class GridCellWidgetPattern implements ListNavigationItem {
// @public
export interface GridInputs extends Omit<GridInputs$1<GridCellPattern>, 'cells'> {
element: SignalLike<HTMLElement>;
enableRangeSelection: SignalLike<boolean>;
enableSelection: SignalLike<boolean>;
getCell: (e: Element | null) => GridCellPattern | undefined;
multi: SignalLike<boolean>;
Expand All @@ -389,7 +388,6 @@ export interface GridInputs extends Omit<GridInputs$1<GridCellPattern>, 'cells'>
// @public
export class GridPattern {
constructor(inputs: GridInputs);
readonly acceptsPointerMove: SignalLike<boolean>;
readonly activeCell: SignalLike<GridCellPattern | undefined>;
readonly activeDescendant: SignalLike<string | undefined>;
readonly anchorCell: SignalLike<GridCellPattern | undefined>;
Expand All @@ -409,11 +407,8 @@ export class GridPattern {
onFocusOut(event: FocusEvent): void;
onKeydown(event: KeyboardEvent): void;
onPointerdown(event: PointerEvent): void;
onPointermove(event: PointerEvent): void;
onPointerup(event: PointerEvent): void;
readonly pauseNavigation: SignalLike<boolean>;
readonly pointerdown: SignalLike<PointerEventManager<PointerEvent>>;
readonly pointerup: SignalLike<PointerEventManager<PointerEvent>>;
readonly prevColKey: SignalLike<"ArrowRight" | "ArrowLeft">;
resetFocusEffect(): void;
resetStateEffect(): void;
Expand Down
54 changes: 1 addition & 53 deletions src/aria/grid/grid.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,6 @@ describe('Grid directives', () => {
fixture.detectChanges();
};

const pointerMove = (target: HTMLElement | Window, eventInit: PointerEventInit = {}) => {
target.dispatchEvent(new PointerEvent('pointermove', {bubbles: true, ...eventInit}));
fixture.detectChanges();
};

const pointerUp = (target: HTMLElement | Window, eventInit: PointerEventInit = {}) => {
target.dispatchEvent(new PointerEvent('pointerup', {bubbles: true, ...eventInit}));
fixture.detectChanges();
};

const up = (modifierKeys?: ModifierKeys) => keydown('ArrowUp', modifierKeys);
const down = (modifierKeys?: ModifierKeys) => keydown('ArrowDown', modifierKeys);
const left = (modifierKeys?: ModifierKeys) => keydown('ArrowLeft', modifierKeys);
Expand Down Expand Up @@ -105,7 +95,6 @@ describe('Grid directives', () => {
softDisabled?: boolean;
enableSelection?: boolean;
selectionMode?: 'follow' | 'explicit';
enableRangeSelection?: boolean;
gridData?: RowConfig[];
}) {
TestBed.resetTestingModule();
Expand All @@ -122,8 +111,6 @@ describe('Grid directives', () => {
if (opts?.enableSelection !== undefined)
testComponent.enableSelection.set(opts.enableSelection);
if (opts?.selectionMode !== undefined) testComponent.selectionMode.set(opts.selectionMode);
if (opts?.enableRangeSelection !== undefined)
testComponent.enableRangeSelection.set(opts.enableRangeSelection);

if (opts?.gridData !== undefined) {
testComponent.gridData.set(opts.gridData);
Expand Down Expand Up @@ -388,7 +375,6 @@ describe('Grid directives', () => {
enableSelection: true,
selectionMode: 'explicit',
multi: true,
enableRangeSelection: true,
});
gridInstance._pattern.setDefaultStateEffect();
fixture.detectChanges();
Expand Down Expand Up @@ -488,7 +474,6 @@ describe('Grid directives', () => {
enableSelection: true,
selectionMode: 'explicit',
multi: true,
enableRangeSelection: true,
});
gridInstance._pattern.setDefaultStateEffect();
fixture.detectChanges();
Expand All @@ -503,41 +488,6 @@ describe('Grid directives', () => {
expect(cell.getAttribute('aria-selected')).toBe('true');
expect(getActiveCellId()).toBe('c1-1');
});

it('should expand selection on pointermove while dragging without changing active cell', () => {
const startCell = gridElement.querySelector('#c0-0') as HTMLElement;
const dragCell = gridElement.querySelector('#c1-1') as HTMLElement;

pointerDown(startCell);
pointerMove(dragCell);

expect(getActiveCellId()).toBe('c0-0');
// Dragging expands selection
expect(startCell.getAttribute('aria-selected')).toBe('true');
expect(dragCell.getAttribute('aria-selected')).toBe('true');
});

it('should stop dragging on pointerup', () => {
const startCell = gridElement.querySelector('#c0-0') as HTMLElement;
const endCell = gridElement.querySelector('#c1-1') as HTMLElement;

pointerDown(startCell);
pointerUp(gridElement);
pointerMove(endCell);

// Active cell should still be c0-0 because dragging stopped before moving to c1-1
expect(getActiveCellId()).toBe('c0-0');
expect(endCell.getAttribute('aria-selected')).toBe('false');
});

it('should not change active cell on pointermove outside of the grid cells', () => {
const startCell = gridElement.querySelector('#c0-0') as HTMLElement;

pointerDown(startCell);
pointerMove(gridElement);

expect(getActiveCellId()).toBe('c0-0');
});
});

describe('configuration', () => {
Expand Down Expand Up @@ -1009,8 +959,7 @@ describe('Grid directives', () => {
[focusMode]="focusMode()"
[softDisabled]="softDisabled()"
[enableSelection]="enableSelection()"
[selectionMode]="selectionMode()"
[enableRangeSelection]="enableRangeSelection()">
[selectionMode]="selectionMode()">
@for (row of gridData(); track $index; let rIndex = $index) {
<tr ngGridRow [rowIndex]="row.rowIndex">
@for (cell of row.cells; track $index; let cIndex = $index) {
Expand Down Expand Up @@ -1067,7 +1016,6 @@ class GridTestComponent {
readonly softDisabled = signal(true);
readonly enableSelection = signal(false);
readonly selectionMode = signal<'follow' | 'explicit'>('follow');
readonly enableRangeSelection = signal(false);
readonly gridData = signal<RowConfig[]>(createGridData());

onActivated = jasmine.createSpy('activated');
Expand Down
21 changes: 0 additions & 21 deletions src/aria/grid/grid.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
ElementRef,
inject,
input,
NgZone,
Signal,
} from '@angular/core';
import {Directionality} from '@angular/cdk/bidi';
Expand Down Expand Up @@ -56,7 +55,6 @@ import {GRID_ROW} from './grid-tokens';
'[attr.aria-activedescendant]': '_pattern.activeDescendant()',
'(keydown)': '_pattern.onKeydown($event)',
'(pointerdown)': '_pattern.onPointerdown($event)',
'(pointerup)': '_pattern.onPointerup($event)',
'(focusin)': '_pattern.onFocusIn($event)',
'(focusout)': '_pattern.onFocusOut($event)',
},
Expand Down Expand Up @@ -122,9 +120,6 @@ export class Grid {
*/
readonly selectionMode = input<'follow' | 'explicit'>('follow');

/** Whether enable range selections (with modifier keys or dragging). */
readonly enableRangeSelection = input(false, {transform: booleanAttribute});

/** The UI pattern for the grid. */
readonly _pattern = new GridPattern({
...this,
Expand All @@ -134,22 +129,6 @@ export class Grid {
});

constructor() {
const ngZone = inject(NgZone);

// Since `pointermove` fires on each pixel, we need to
// be careful not to hit the zone unless it's necessary.
ngZone.runOutsideAngular(() => {
this.element.addEventListener(
'pointermove',
event => {
if (this._pattern.acceptsPointerMove()) {
ngZone.run(() => this._pattern.onPointermove(event));
}
},
{passive: true},
);
});

afterRenderEffect(() => this._pattern.setDefaultStateEffect());
afterRenderEffect(() => this._pattern.resetStateEffect());
afterRenderEffect(() => this._pattern.resetFocusEffect());
Expand Down
106 changes: 0 additions & 106 deletions src/aria/private/grid/grid.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,6 @@ const end = (mods?: ModifierKeys) => createKeyboardEvent('keydown', 35, 'End', m
const space = (mods?: ModifierKeys) => createKeyboardEvent('keydown', 32, ' ', mods);
const enter = (mods?: ModifierKeys) => createKeyboardEvent('keydown', 13, 'Enter', mods);
const escape = (mods?: ModifierKeys) => createKeyboardEvent('keydown', 27, 'Escape', mods);
const shiftUp = () => up({shift: true});
const shiftDown = () => down({shift: true});
const shiftLeft = () => left({shift: true});
const shiftRight = () => right({shift: true});
const shiftHome = () => home({shift: true});
const shiftEnd = () => end({shift: true});

function createClickEvent(element: HTMLElement, mods?: ModifierKeys): PointerEvent {
return {
Expand Down Expand Up @@ -141,7 +135,6 @@ function getDefaultGridInputs(): TestGridInputs {
enableSelection: signal(false),
multi: signal(false),
selectionMode: signal('follow'),
enableRangeSelection: signal(false),
getCell: () => undefined,
focusMode: signal('roving'),
disabled: signal(false),
Expand Down Expand Up @@ -602,7 +595,6 @@ describe('Grid', () => {

it('should select all on Ctrl+A', () => {
(gridInputs.multi as WritableSignalLike<boolean>).set(true);
(gridInputs.enableRangeSelection as WritableSignalLike<boolean>).set(true);
grid.onKeydown(a({control: true}));
expect(
grid
Expand All @@ -614,7 +606,6 @@ describe('Grid', () => {

it('should select row on Shift+Space', () => {
(gridInputs.multi as WritableSignalLike<boolean>).set(true);
(gridInputs.enableRangeSelection as WritableSignalLike<boolean>).set(true);
const gridCells = grid.cells();
grid.gridBehavior.focusBehavior.focusCell(gridCells[0][0]);
grid.onKeydown(space({shift: true}));
Expand All @@ -627,7 +618,6 @@ describe('Grid', () => {

it('should select column on Ctrl+Space', () => {
(gridInputs.multi as WritableSignalLike<boolean>).set(true);
(gridInputs.enableRangeSelection as WritableSignalLike<boolean>).set(true);
const gridCells = grid.cells();
grid.gridBehavior.focusBehavior.focusCell(gridCells[0][0]);
grid.onKeydown(space({control: true}));
Expand All @@ -638,66 +628,6 @@ describe('Grid', () => {
expect(gridCells[1][1].selected()).toBe(false);
});
});

describe('Range Selection Logic', () => {
let grid: GridPattern;

beforeEach(() => {
(gridInputs.enableSelection as WritableSignalLike<boolean>).set(true);
(gridInputs.multi as WritableSignalLike<boolean>).set(true);
(gridInputs.enableRangeSelection as WritableSignalLike<boolean>).set(true);

const data = [{cells: [{}, {}, {}]}, {cells: [{}, {}, {}]}, {cells: [{}, {}, {}]}];
const result = createGrid(data, gridInputs);
grid = result.grid;
grid.setDefaultStateEffect();
});

it('should expand the selection range up on Shift+ArrowUp', () => {
const cells = grid.cells();
grid.gridBehavior.focusBehavior.focusCell(cells[1][1]);
grid.onKeydown(shiftUp());
expect(cells[1][1].selected()).toBe(true);
expect(cells[0][1].selected()).toBe(true);
});

it('should expand the selection range down on Shift+ArrowDown', () => {
const cells = grid.cells();
grid.gridBehavior.focusBehavior.focusCell(cells[1][1]);
grid.onKeydown(shiftDown());
expect(cells[1][1].selected()).toBe(true);
expect(cells[2][1].selected()).toBe(true);
});

it('should expand the selection range left on Shift+ArrowLeft', () => {
const cells = grid.cells();
grid.gridBehavior.focusBehavior.focusCell(cells[1][1]);
grid.onKeydown(shiftLeft());
expect(cells[1][1].selected()).toBe(true);
expect(cells[1][0].selected()).toBe(true);
});

it('should expand the selection range right on Shift+ArrowRight', () => {
const cells = grid.cells();
grid.gridBehavior.focusBehavior.focusCell(cells[1][1]);
grid.onKeydown(shiftRight());
expect(cells[1][1].selected()).toBe(true);
expect(cells[1][2].selected()).toBe(true);
});

it('should support range selection with Shift+Home/End', () => {
const cells = grid.cells();
grid.gridBehavior.focusBehavior.focusCell(cells[0][1]);
grid.onKeydown(shiftHome());
expect(cells[0][0].selected()).toBe(true);
expect(cells[0][1].selected()).toBe(true);

grid.onKeydown(shiftEnd());
expect(cells[0][0].selected()).toBe(false);
expect(cells[0][1].selected()).toBe(true);
expect(cells[0][2].selected()).toBe(true);
});
});
});

describe('Pointer Events', () => {
Expand Down Expand Up @@ -745,42 +675,6 @@ describe('Grid', () => {
expect(cells[0][0].selected()).toBe(true);
expect(cells[0][1].selected()).toBe(true);
});

it('should support range selection with Shift+pointerdown', () => {
(gridInputs.multi as WritableSignalLike<boolean>).set(true);
(gridInputs.enableRangeSelection as WritableSignalLike<boolean>).set(true);
const cells = grid.cells();
grid.onPointerdown(createClickEvent(cells[0][0].element()));
grid.onPointerdown(createClickEvent(cells[1][1].element(), {shift: true}));
expect(cells[0][0].selected()).toBe(true);
expect(cells[0][1].selected()).toBe(true);
expect(cells[1][0].selected()).toBe(true);
expect(cells[1][1].selected()).toBe(true);
});
});

describe('Range Selection Dragging', () => {
beforeEach(() => {
(gridInputs.multi as WritableSignalLike<boolean>).set(true);
(gridInputs.enableRangeSelection as WritableSignalLike<boolean>).set(true);
});

it('should select range on pointermove', () => {
const cells = grid.cells();
grid.onPointerdown(createClickEvent(cells[0][0].element()));
grid.onPointermove(createClickEvent(cells[0][1].element()));
expect(cells[0][0].selected()).toBe(true);
expect(cells[0][1].selected()).toBe(true);
});

it('should stabilize selection on pointerup', () => {
const cell = grid.cells()[0][1];
grid.onPointerdown(createClickEvent(grid.cells()[0][0].element()));
grid.onPointermove(createClickEvent(cell.element()));
expect(grid.dragging()).toBe(true);
grid.onPointerup(createClickEvent(cell.element()));
expect(grid.dragging()).toBe(false);
});
});
});

Expand Down
Loading
Loading