From 4d74d425201d8fbbfaf24f092d4c4d1d53bd632d Mon Sep 17 00:00:00 2001 From: Cheng-Hsuan Tsai Date: Tue, 5 May 2026 08:10:37 +0000 Subject: [PATCH] refactor(aria/grid): consolidate widget focus logic and activation handling --- goldens/aria/grid/index.api.md | 4 +- goldens/aria/private/index.api.md | 5 +- src/aria/grid/grid-cell-widget.ts | 19 ++---- src/aria/grid/grid-cell.ts | 6 -- src/aria/grid/grid.spec.ts | 50 +++++++++++++++ src/aria/private/grid/cell.ts | 9 --- src/aria/private/grid/grid.spec.ts | 8 +-- src/aria/private/grid/widget.ts | 62 ++++++++++++++++--- .../simple-combobox/simple-combobox.spec.ts | 8 +-- .../simple-combobox-grid-example.html | 8 +-- 10 files changed, 125 insertions(+), 54 deletions(-) diff --git a/goldens/aria/grid/index.api.md b/goldens/aria/grid/index.api.md index d304ade5a0d0..3d7839b9a1de 100644 --- a/goldens/aria/grid/index.api.md +++ b/goldens/aria/grid/index.api.md @@ -7,7 +7,6 @@ import * as _angular_cdk_bidi from '@angular/cdk/bidi'; import * as _angular_core from '@angular/core'; import { ElementRef } from '@angular/core'; -import { EventEmitter } from '@angular/core'; import { OnDestroy } from '@angular/core'; import { OnInit } from '@angular/core'; import { Signal } from '@angular/core'; @@ -41,7 +40,6 @@ export class Grid implements OnDestroy { // @public export class GridCell implements OnInit, OnDestroy { constructor(); - readonly activated: EventEmitter; readonly active: Signal; readonly colIndex: _angular_core.InputSignal; readonly colSpan: _angular_core.InputSignal; @@ -62,7 +60,7 @@ export class GridCell implements OnInit, OnDestroy { readonly tabindex: _angular_core.InputSignal; readonly textDirection: _angular_core.WritableSignal<_angular_cdk_bidi.Direction>; // (undocumented) - static ɵdir: _angular_core.ɵɵDirectiveDeclaration; + static ɵdir: _angular_core.ɵɵDirectiveDeclaration; // (undocumented) static ɵfac: _angular_core.ɵɵFactoryDeclaration; } diff --git a/goldens/aria/private/index.api.md b/goldens/aria/private/index.api.md index 6f48bc4ee15e..1a32be956d1d 100644 --- a/goldens/aria/private/index.api.md +++ b/goldens/aria/private/index.api.md @@ -301,7 +301,6 @@ export interface GridCellInputs extends GridCell { colIndex: SignalLike; getWidget: (e: Element | null) => GridCellWidgetPattern | undefined; grid: SignalLike; - onActivate?: (event: KeyboardEvent) => void; row: SignalLike; rowIndex: SignalLike; widget: SignalLike; @@ -341,6 +340,8 @@ export interface GridCellWidgetInputs { disabled: SignalLike; element: SignalLike; focusTarget: SignalLike>; + onActivate?: (event: KeyboardEvent | FocusEvent | undefined) => void; + onDeactivate?: (event: KeyboardEvent | FocusEvent | undefined) => void; widgetType: SignalLike<'simple' | 'complex' | 'editable'>; } @@ -348,8 +349,10 @@ export interface GridCellWidgetInputs { export class GridCellWidgetPattern { constructor(inputs: GridCellWidgetInputs); activate(event?: KeyboardEvent | FocusEvent): void; + activationEffect(): void; readonly active: SignalLike; deactivate(event?: KeyboardEvent | FocusEvent): void; + deactivationEffect(): void; readonly disabled: SignalLike; readonly element: SignalLike; focus(): void; diff --git a/src/aria/grid/grid-cell-widget.ts b/src/aria/grid/grid-cell-widget.ts index 5616fe41a9dd..438840116728 100644 --- a/src/aria/grid/grid-cell-widget.ts +++ b/src/aria/grid/grid-cell-widget.ts @@ -88,7 +88,7 @@ export class GridCellWidget { * If a focus target exists then return -1. Unless an override. */ protected readonly _tabIndex: Signal = computed( - () => this.tabindex() ?? (this.focusTarget() ? -1 : this._pattern.tabIndex()), + () => this.tabindex() ?? this._pattern.tabIndex(), ); /** The UI pattern for the grid cell widget. */ @@ -96,6 +96,8 @@ export class GridCellWidget { ...this, element: () => this.element, cell: () => this._cell._pattern, + onActivate: e => this.activated.emit(e), + onDeactivate: e => this.deactivated.emit(e), }); /** Whether the widget is activated. */ @@ -105,22 +107,11 @@ export class GridCellWidget { constructor() { afterRenderEffect({ - read: () => { - if (this._pattern.isActivated()) { - const activateEvent = this._pattern.lastActivateEvent(); - this.activated.emit(activateEvent); - this._pattern.focus(); - } - }, + write: () => this._pattern.activationEffect(), }); afterRenderEffect({ - read: () => { - const deactivateEvent = this._pattern.lastDeactivateEvent(); - if (deactivateEvent) { - this.deactivated.emit(deactivateEvent); - } - }, + write: () => this._pattern.deactivationEffect(), }); } diff --git a/src/aria/grid/grid-cell.ts b/src/aria/grid/grid-cell.ts index 9cd3dc974592..063624c1742e 100644 --- a/src/aria/grid/grid-cell.ts +++ b/src/aria/grid/grid-cell.ts @@ -14,13 +14,11 @@ import { contentChild, Directive, ElementRef, - EventEmitter, inject, input, model, OnDestroy, OnInit, - Output, Signal, Renderer2, } from '@angular/core'; @@ -56,9 +54,6 @@ export class GridCell implements OnInit, OnDestroy { /** A reference to the host element. */ readonly element = this._elementRef.nativeElement as HTMLElement; - /** Emits when the cell is activated via Enter/Space (simple widgets only). */ - @Output() readonly activated = new EventEmitter(); - /** Whether the cell is currently active (focused). */ readonly active = computed(() => this._pattern.active()); @@ -122,7 +117,6 @@ export class GridCell implements OnInit, OnDestroy { widget: this._widgetPattern, getWidget: e => this._getWidget(e), element: () => this.element, - onActivate: e => this.activated.emit(e), }); constructor() { diff --git a/src/aria/grid/grid.spec.ts b/src/aria/grid/grid.spec.ts index 53d64450769b..cf5828525102 100644 --- a/src/aria/grid/grid.spec.ts +++ b/src/aria/grid/grid.spec.ts @@ -938,6 +938,56 @@ describe('Grid directives', () => { expect(widgetElement.getAttribute('tabindex')).toBe('-1'); }); + it('should emit the activated output on Enter for simple widget', () => { + const gridData = createGridData(); + gridData[0].cells[0].widgets = [{id: 'w1', type: 'simple'}]; + setupGrid({gridData}); + gridInstance._pattern.setDefaultStateEffect(); + fixture.detectChanges(); + + tabIntoGrid(); + + expect(fixture.componentInstance.onActivated).not.toHaveBeenCalled(); + + keydown('Enter'); + expect(fixture.componentInstance.onActivated).toHaveBeenCalled(); + }); + + it('should emit the activated output on Space for simple widget', () => { + const gridData = createGridData(); + gridData[0].cells[0].widgets = [{id: 'w1', type: 'simple'}]; + setupGrid({gridData}); + gridInstance._pattern.setDefaultStateEffect(); + fixture.detectChanges(); + + tabIntoGrid(); + + expect(fixture.componentInstance.onActivated).not.toHaveBeenCalled(); + + keydown(' '); + expect(fixture.componentInstance.onActivated).toHaveBeenCalled(); + }); + + it('should emit the activated output in activedescendant mode when event is dispatched directly to grid', () => { + const gridData = createGridData(); + gridData[0].cells[0].widgets = [{id: 'w1', type: 'simple'}]; + setupGrid({gridData, focusMode: 'activedescendant'}); + gridInstance._pattern.setDefaultStateEffect(); + fixture.detectChanges(); + + expect(fixture.componentInstance.onActivated).not.toHaveBeenCalled(); + + // Verify standard activedescendant behavior by targeting the CONTAINER directly + const event = new KeyboardEvent('keydown', { + key: 'Enter', + bubbles: true, + }); + gridElement.dispatchEvent(event); + fixture.detectChanges(); + + expect(fixture.componentInstance.onActivated).toHaveBeenCalled(); + }); + it('should emit the activated output when the widget becomes active', () => { const gridData = createGridData(); gridData[0].cells[0].widgets = [{id: 'w1', type: 'complex'}]; diff --git a/src/aria/private/grid/cell.ts b/src/aria/private/grid/cell.ts index ba52934f90b5..121295985649 100644 --- a/src/aria/private/grid/cell.ts +++ b/src/aria/private/grid/cell.ts @@ -36,9 +36,6 @@ export interface GridCellInputs extends GridCell { /** A function that returns the cell widget associated with a given element. */ getWidget: (e: Element | null) => GridCellWidgetPattern | undefined; - - /** Callback when the cell is activated via Enter/Space. */ - onActivate?: (event: KeyboardEvent) => void; } /** The UI pattern for a grid cell. */ @@ -120,12 +117,6 @@ export class GridCellPattern implements GridCell { onKeydown(event: KeyboardEvent): void { if (this.disabled()) return; this.widget()?.onKeydown(event); - - if (this.widget()?.inputs.widgetType() === 'simple') { - if (event.key === 'Enter' || event.key === ' ') { - this.inputs.onActivate?.(event); - } - } } /** Handles focusin events for the cell. */ diff --git a/src/aria/private/grid/grid.spec.ts b/src/aria/private/grid/grid.spec.ts index b9ef7416148e..9af2475eeab9 100644 --- a/src/aria/private/grid/grid.spec.ts +++ b/src/aria/private/grid/grid.spec.ts @@ -370,7 +370,7 @@ describe('Grid', () => { const onActivateSpy = jasmine.createSpy('onActivate'); const {grid} = createGrid([{cells: [{widget: {widgetType: 'simple'}}]}], gridInputs); const cell = grid.cells()[0][0]; - (cell.inputs as any).onActivate = onActivateSpy; + cell.inputs.widget()!.inputs.onActivate = onActivateSpy; const event = enter(); cell.onKeydown(event); @@ -381,7 +381,7 @@ describe('Grid', () => { const onActivateSpy = jasmine.createSpy('onActivate'); const {grid} = createGrid([{cells: [{widget: {widgetType: 'simple'}}]}], gridInputs); const cell = grid.cells()[0][0]; - (cell.inputs as any).onActivate = onActivateSpy; + cell.inputs.widget()!.inputs.onActivate = onActivateSpy; const event = space(); cell.onKeydown(event); @@ -392,7 +392,7 @@ describe('Grid', () => { const onActivateSpy = jasmine.createSpy('onActivate'); const {grid} = createGrid([{cells: [{widget: {widgetType: 'complex'}}]}], gridInputs); const cell = grid.cells()[0][0]; - (cell.inputs as any).onActivate = onActivateSpy; + cell.inputs.widget()!.inputs.onActivate = onActivateSpy; const event = enter(); cell.onKeydown(event); @@ -403,7 +403,7 @@ describe('Grid', () => { const onActivateSpy = jasmine.createSpy('onActivate'); const {grid} = createGrid([{cells: [{widget: {widgetType: 'simple'}}]}], gridInputs); const cell = grid.cells()[0][0]; - (cell.inputs as any).onActivate = onActivateSpy; + cell.inputs.widget()!.inputs.onActivate = onActivateSpy; const event = up(); cell.onKeydown(event); diff --git a/src/aria/private/grid/widget.ts b/src/aria/private/grid/widget.ts index 469889af16f9..56480bf55582 100644 --- a/src/aria/private/grid/widget.ts +++ b/src/aria/private/grid/widget.ts @@ -32,6 +32,12 @@ export interface GridCellWidgetInputs { /** The element that will receive focus when the widget is activated. */ focusTarget: SignalLike>; + + /** Callback hook used to notify parents or directives upon interaction. */ + onActivate?: (event: KeyboardEvent | FocusEvent | undefined) => void; + + /** Callback hook used to notify parents or directives upon exit. */ + onDeactivate?: (event: KeyboardEvent | FocusEvent | undefined) => void; } /** The UI pattern for a widget inside a grid cell. */ @@ -49,7 +55,12 @@ export class GridCellWidgetPattern { ); /** The tab index for the widget. */ - readonly tabIndex: SignalLike<-1 | 0> = computed(() => this.inputs.cell().widgetTabIndex()); + readonly tabIndex: SignalLike<-1 | 0> = computed(() => { + if (this.inputs.focusTarget()) { + return -1; + } + return this.inputs.cell().widgetTabIndex(); + }); /** Whether the widget is the active widget in the cell. */ readonly active: SignalLike = computed( @@ -71,18 +82,25 @@ export class GridCellWidgetPattern { readonly keydown = computed(() => { const manager = new KeyboardEventManager(); + // Simple widgets emit notification on interaction without capturing event flow + if (this.inputs.widgetType() === 'simple') { + return manager + .on('Enter', e => this.inputs.onActivate?.(e), { + preventDefault: false, + stopPropagation: false, + }) + .on(' ', e => this.inputs.onActivate?.(e), { + preventDefault: false, + stopPropagation: false, + }); + } + // If a widget is activated, only listen to events that exits activate state. if (this.isActivated()) { - manager.on('Escape', e => { - this.deactivate(e); - this.focus(); - }); + manager.on('Escape', e => this.deactivate(e)); if (this.inputs.widgetType() === 'editable') { - manager.on('Enter', e => { - this.deactivate(e); - this.focus(); - }); + manager.on('Enter', e => this.deactivate(e)); } return manager; @@ -135,6 +153,32 @@ export class GridCellWidgetPattern { this.widgetHost().focus(); } + /** Side-effect executed whenever the widget activates. Runs in the write phase. */ + activationEffect(): void { + if (this.isActivated()) { + const event = this.lastActivateEvent(); + this.inputs.onActivate?.(event); + + // Only automatically redirect focus if explicit configuration was supplied. + if (this.inputs.focusTarget()) { + this.focus(); + } + } + } + + /** Side-effect executed whenever the widget deactivates. Runs in the write phase. */ + deactivationEffect(): void { + const event = this.lastDeactivateEvent(); + if (event) { + this.inputs.onDeactivate?.(event); + + // Only automatically restore focus if the deactivation was triggered by user keyboard interaction. + if (event instanceof KeyboardEvent) { + this.focus(); + } + } + } + /** Activates the widget. */ activate(event?: KeyboardEvent | FocusEvent): void { if (this.isActivated()) return; diff --git a/src/aria/simple-combobox/simple-combobox.spec.ts b/src/aria/simple-combobox/simple-combobox.spec.ts index c0228769aaf3..58db4c1ad846 100644 --- a/src/aria/simple-combobox/simple-combobox.spec.ts +++ b/src/aria/simple-combobox/simple-combobox.spec.ts @@ -1541,13 +1541,13 @@ const states = [
@for (item of filteredItems(); track item; let i = $index) {
-
-
-
-
diff --git a/src/components-examples/aria/simple-combobox/simple-combobox-grid/simple-combobox-grid-example.html b/src/components-examples/aria/simple-combobox/simple-combobox-grid/simple-combobox-grid-example.html index 566e7a36b7ed..f6191d9cead4 100644 --- a/src/components-examples/aria/simple-combobox/simple-combobox-grid/simple-combobox-grid-example.html +++ b/src/components-examples/aria/simple-combobox/simple-combobox-grid/simple-combobox-grid-example.html @@ -21,15 +21,15 @@ @for (item of filteredItems(); track item.label; let i = $index) {
- check
-