Skip to content

Commit e4abb46

Browse files
authored
refactor(multiple): Clean up afterRenderEffect usage across angular aria (#33054)
* refactor(aria/combobox): clean up afterRenderEffect usage and fix missing id * refactor(aria/grid): use read/write appropariately in afterRenderEffect * refactor(aria/menu): use write in afterRenderEffect when setting focus and use computed for patterns * refactor(multiple): use write in afterRenderEffect for aria DeferredContent updates * refactor(multiple): use write stage for setDefaultStateEffect across all aria directives * refactor(aria/tree): change afterRenderEffect to use read/write stages appropriately * refactor(aria/listbox): update afterRenderEffect to use read/write stage appropriately
1 parent dd12062 commit e4abb46

16 files changed

Lines changed: 159 additions & 132 deletions

File tree

goldens/aria/combobox/index.api.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,12 @@ export class ComboboxDialog {
3636
// (undocumented)
3737
close(): void;
3838
readonly combobox: Combobox<any>;
39-
readonly element: HTMLElement;
39+
readonly element: HTMLDialogElement;
40+
readonly id: _angular_core.InputSignal<string>;
4041
// (undocumented)
4142
readonly _pattern: ComboboxDialogPattern;
4243
// (undocumented)
43-
static ɵdir: _angular_core.ɵɵDirectiveDeclaration<ComboboxDialog, "dialog[ngComboboxDialog]", ["ngComboboxDialog"], {}, {}, never, never, true, [{ directive: typeof ComboboxPopup; inputs: {}; outputs: {}; }]>;
44+
static ɵdir: _angular_core.ɵɵDirectiveDeclaration<ComboboxDialog, "dialog[ngComboboxDialog]", ["ngComboboxDialog"], { "id": { "alias": "id"; "required": false; "isSignal": true; }; }, {}, never, never, true, [{ directive: typeof ComboboxPopup; inputs: {}; outputs: {}; }]>;
4445
// (undocumented)
4546
static ɵfac: _angular_core.ɵɵFactoryDeclaration<ComboboxDialog, never>;
4647
}

src/aria/accordion/accordion-panel.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,10 @@ export class AccordionPanel {
7272

7373
constructor() {
7474
// Connect the panel's hidden state to the DeferredContentAware's visibility.
75-
afterRenderEffect(() => {
76-
this._deferredContentAware.contentVisible.set(this.visible());
75+
afterRenderEffect({
76+
write: () => {
77+
this._deferredContentAware.contentVisible.set(this.visible());
78+
},
7779
});
7880
}
7981

src/aria/combobox/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ ng_project(
1111
deps = [
1212
"//:node_modules/@angular/core",
1313
"//src/aria/private",
14+
"//src/cdk/a11y",
1415
"//src/cdk/bidi",
1516
],
1617
)

src/aria/combobox/combobox-dialog.ts

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import {afterRenderEffect, Directive, ElementRef, inject} from '@angular/core';
9+
import {afterRenderEffect, Directive, ElementRef, inject, input} from '@angular/core';
10+
import {_IdGenerator} from '@angular/cdk/a11y';
1011
import {ComboboxDialogPattern} from '../private';
1112
import {Combobox} from './combobox';
1213
import {ComboboxPopup} from './combobox-popup';
@@ -46,35 +47,34 @@ export class ComboboxDialog {
4647
private readonly _elementRef = inject(ElementRef<HTMLDialogElement>);
4748

4849
/** A reference to the dialog element. */
49-
readonly element = this._elementRef.nativeElement as HTMLElement;
50+
readonly element = this._elementRef.nativeElement as HTMLDialogElement;
5051

5152
/** The combobox that the dialog belongs to. */
5253
readonly combobox = inject(Combobox);
5354

55+
/** The unique identifier for the trigger. */
56+
readonly id = input(inject(_IdGenerator).getId('ng-combobox-dialog-', true));
57+
5458
/** A reference to the parent combobox popup, if one exists. */
5559
private readonly _popup = inject<ComboboxPopup<unknown>>(ComboboxPopup, {
5660
optional: true,
5761
});
5862

59-
readonly _pattern: ComboboxDialogPattern;
63+
readonly _pattern: ComboboxDialogPattern = new ComboboxDialogPattern({
64+
id: this.id,
65+
element: () => this.element,
66+
combobox: this.combobox._pattern,
67+
});
6068

6169
constructor() {
62-
this._pattern = new ComboboxDialogPattern({
63-
id: () => '',
64-
element: () => this._elementRef.nativeElement,
65-
combobox: this.combobox._pattern,
66-
});
67-
6870
if (this._popup) {
6971
this._popup._controls.set(this._pattern);
7072
}
7173

72-
afterRenderEffect(() => {
73-
if (this._elementRef) {
74-
this.combobox._pattern.expanded()
75-
? this._elementRef.nativeElement.showModal()
76-
: this._elementRef.nativeElement.close();
77-
}
74+
afterRenderEffect({
75+
write: () => {
76+
this.combobox._pattern.expanded() ? this.element.showModal() : this.element.close();
77+
},
7878
});
7979
}
8080

src/aria/combobox/combobox-input.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,12 @@ export class ComboboxInput {
8181
}
8282

8383
/** Focuses & selects the first item in the combobox if the user changes the input value. */
84-
afterRenderEffect(() => {
85-
this.value();
86-
controls?.items();
87-
untracked(() => this.combobox._pattern.onFilter());
84+
afterRenderEffect({
85+
write: () => {
86+
this.value();
87+
controls?.items();
88+
untracked(() => this.combobox._pattern.onFilter());
89+
},
8890
});
8991
}
9092
}

src/aria/combobox/combobox.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -140,13 +140,15 @@ export class Combobox<V> {
140140
}
141141
});
142142

143-
afterRenderEffect(() => {
144-
if (
145-
!this._deferredContentAware?.contentVisible() &&
146-
(this._pattern.isFocused() || this.alwaysExpanded())
147-
) {
148-
this._deferredContentAware?.contentVisible.set(true);
149-
}
143+
afterRenderEffect({
144+
write: () => {
145+
if (
146+
!this._deferredContentAware?.contentVisible() &&
147+
(this._pattern.isFocused() || this.alwaysExpanded())
148+
) {
149+
this._deferredContentAware?.contentVisible.set(true);
150+
}
151+
},
150152
});
151153
}
152154

src/aria/grid/grid-cell-widget.ts

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -104,19 +104,23 @@ export class GridCellWidget {
104104
}
105105

106106
constructor() {
107-
afterRenderEffect(() => {
108-
if (this._pattern.isActivated()) {
109-
const activateEvent = this._pattern.lastActivateEvent();
110-
this.activated.emit(activateEvent);
111-
this._pattern.focus();
112-
}
107+
afterRenderEffect({
108+
read: () => {
109+
if (this._pattern.isActivated()) {
110+
const activateEvent = this._pattern.lastActivateEvent();
111+
this.activated.emit(activateEvent);
112+
this._pattern.focus();
113+
}
114+
},
113115
});
114116

115-
afterRenderEffect(() => {
116-
const deactivateEvent = this._pattern.lastDeactivateEvent();
117-
if (deactivateEvent) {
118-
this.deactivated.emit(deactivateEvent);
119-
}
117+
afterRenderEffect({
118+
read: () => {
119+
const deactivateEvent = this._pattern.lastDeactivateEvent();
120+
if (deactivateEvent) {
121+
this.deactivated.emit(deactivateEvent);
122+
}
123+
},
120124
});
121125
}
122126

src/aria/grid/grid.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -129,11 +129,12 @@ export class Grid {
129129
});
130130

131131
constructor() {
132-
afterRenderEffect(() => this._pattern.setDefaultStateEffect());
133-
afterRenderEffect(() => this._pattern.resetStateEffect());
134-
afterRenderEffect(() => this._pattern.resetFocusEffect());
135-
afterRenderEffect(() => this._pattern.restoreFocusEffect());
136-
afterRenderEffect(() => this._pattern.focusEffect());
132+
// Use Write mode for all direct DOM focus management actions.
133+
afterRenderEffect({write: () => this._pattern.setDefaultStateEffect()});
134+
afterRenderEffect({write: () => this._pattern.resetStateEffect()});
135+
afterRenderEffect({write: () => this._pattern.resetFocusEffect()});
136+
afterRenderEffect({write: () => this._pattern.restoreFocusEffect()});
137+
afterRenderEffect({write: () => this._pattern.focusEffect()});
137138
}
138139

139140
/** Gets the cell pattern for a given element. */

src/aria/listbox/listbox.ts

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -158,38 +158,44 @@ export class Listbox<V> {
158158
this._popup._controls.set(this._pattern as ComboboxListboxPattern<V>);
159159
}
160160

161-
afterRenderEffect(() => {
162-
if (typeof ngDevMode === 'undefined' || ngDevMode) {
163-
const violations = this._pattern.validate();
164-
for (const violation of violations) {
165-
console.error(violation);
161+
// Check for any violationns after the DOM has been updated.
162+
afterRenderEffect({
163+
read: () => {
164+
if (typeof ngDevMode === 'undefined' || ngDevMode) {
165+
const violations = this._pattern.validate();
166+
for (const violation of violations) {
167+
console.error(violation);
168+
}
166169
}
167-
}
170+
},
168171
});
169172

170-
afterRenderEffect(() => {
171-
this._pattern.setDefaultStateEffect();
172-
});
173+
afterRenderEffect({write: () => this._pattern.setDefaultStateEffect()});
173174

174175
// Ensure that if the active item is removed from
175176
// the list, the listbox updates it's focus state.
176-
afterRenderEffect(() => {
177-
const items = inputs.items();
178-
const activeItem = untracked(() => inputs.activeItem());
177+
afterRenderEffect({
178+
write: () => {
179+
const items = inputs.items();
180+
const activeItem = untracked(() => inputs.activeItem());
179181

180-
if (!items.some(i => i === activeItem) && activeItem) {
181-
this._pattern.listBehavior.unfocus();
182-
}
182+
if (!items.some(i => i === activeItem) && activeItem) {
183+
this._pattern.listBehavior.unfocus();
184+
}
185+
},
183186
});
184187

185188
// Ensure that the value is always in sync with the available options.
186-
afterRenderEffect(() => {
187-
const items = inputs.items();
188-
const value = untracked(() => this.value());
189-
190-
if (items && value.some(v => !items.some(i => i.value() === v))) {
191-
this.value.set(value.filter(v => items.some(i => i.value() === v)));
192-
}
189+
// This needs to be after the render for the value to always be available.
190+
afterRenderEffect({
191+
write: () => {
192+
const items = inputs.items();
193+
const value = untracked(() => this.value());
194+
195+
if (items && value.some(v => !items.some(i => i.value() === v))) {
196+
this.value.set(value.filter(v => items.some(i => i.value() === v)));
197+
}
198+
},
193199
});
194200
}
195201

src/aria/menu/menu-bar.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ export class MenuBar<V> {
104104
readonly _pattern: MenuBarPattern<V>;
105105

106106
/** The menu items as a writable signal. */
107-
private readonly _itemPatterns = signal<any[]>([]);
107+
private readonly _itemPatterns = computed(() => this._items().map(i => i._pattern));
108108

109109
/** A callback function triggered when a menu item is selected. */
110110
readonly itemSelected = output<V>();
@@ -123,13 +123,7 @@ export class MenuBar<V> {
123123
element: computed(() => this._elementRef.nativeElement),
124124
});
125125

126-
afterRenderEffect(() => {
127-
this._itemPatterns.set(this._items().map(i => i._pattern));
128-
});
129-
130-
afterRenderEffect(() => {
131-
this._pattern.setDefaultStateEffect();
132-
});
126+
afterRenderEffect({write: () => this._pattern.setDefaultStateEffect()});
133127
}
134128

135129
/** Closes the menubar. */

0 commit comments

Comments
 (0)