Skip to content

Commit fd3d9d7

Browse files
committed
refactor(aria/accordion): simplify code by using template references instead of user id to match panels with triggers
1 parent dd09709 commit fd3d9d7

22 files changed

Lines changed: 238 additions & 529 deletions

goldens/aria/accordion/index.api.md

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import * as _angular_cdk_bidi from '@angular/cdk/bidi';
88
import * as _angular_core from '@angular/core';
99
import { OnDestroy } from '@angular/core';
10-
import { WritableSignal } from '@angular/core';
10+
import { OnInit } from '@angular/core';
1111

1212
// @public
1313
export class AccordionContent {
@@ -19,7 +19,6 @@ export class AccordionContent {
1919

2020
// @public
2121
export class AccordionGroup {
22-
constructor();
2322
collapseAll(): void;
2423
readonly disabled: _angular_core.InputSignalWithTransform<boolean, unknown>;
2524
readonly element: HTMLElement;
@@ -30,43 +29,42 @@ export class AccordionGroup {
3029
readonly textDirection: _angular_core.WritableSignal<_angular_cdk_bidi.Direction>;
3130
readonly wrap: _angular_core.InputSignalWithTransform<boolean, unknown>;
3231
// (undocumented)
33-
static ɵdir: _angular_core.ɵɵDirectiveDeclaration<AccordionGroup, "[ngAccordionGroup]", ["ngAccordionGroup"], { "disabled": { "alias": "disabled"; "required": false; "isSignal": true; }; "multiExpandable": { "alias": "multiExpandable"; "required": false; "isSignal": true; }; "softDisabled": { "alias": "softDisabled"; "required": false; "isSignal": true; }; "wrap": { "alias": "wrap"; "required": false; "isSignal": true; }; }, {}, ["_triggers", "_panels"], never, true, never>;
32+
static ɵdir: _angular_core.ɵɵDirectiveDeclaration<AccordionGroup, "[ngAccordionGroup]", ["ngAccordionGroup"], { "disabled": { "alias": "disabled"; "required": false; "isSignal": true; }; "multiExpandable": { "alias": "multiExpandable"; "required": false; "isSignal": true; }; "softDisabled": { "alias": "softDisabled"; "required": false; "isSignal": true; }; "wrap": { "alias": "wrap"; "required": false; "isSignal": true; }; }, {}, ["_triggers"], never, true, never>;
3433
// (undocumented)
3534
static ɵfac: _angular_core.ɵɵFactoryDeclaration<AccordionGroup, never>;
3635
}
3736

3837
// @public
3938
export class AccordionPanel {
4039
constructor();
41-
readonly _accordionTriggerPattern: WritableSignal<AccordionTriggerPattern | undefined>;
4240
collapse(): void;
4341
expand(): void;
4442
readonly id: _angular_core.InputSignal<string>;
45-
readonly panelId: _angular_core.InputSignal<string>;
46-
readonly _pattern: AccordionPanelPattern;
43+
_pattern?: AccordionTriggerPattern;
4744
toggle(): void;
4845
readonly visible: _angular_core.Signal<boolean>;
4946
// (undocumented)
50-
static ɵdir: _angular_core.ɵɵDirectiveDeclaration<AccordionPanel, "[ngAccordionPanel]", ["ngAccordionPanel"], { "id": { "alias": "id"; "required": false; "isSignal": true; }; "panelId": { "alias": "panelId"; "required": true; "isSignal": true; }; }, {}, never, never, true, [{ directive: typeof DeferredContentAware; inputs: { "preserveContent": "preserveContent"; }; outputs: {}; }]>;
47+
static ɵdir: _angular_core.ɵɵDirectiveDeclaration<AccordionPanel, "[ngAccordionPanel]", ["ngAccordionPanel"], { "id": { "alias": "id"; "required": false; "isSignal": true; }; }, {}, never, never, true, [{ directive: typeof DeferredContentAware; inputs: { "preserveContent": "preserveContent"; }; outputs: {}; }]>;
5148
// (undocumented)
5249
static ɵfac: _angular_core.ɵɵFactoryDeclaration<AccordionPanel, never>;
5350
}
5451

5552
// @public
56-
export class AccordionTrigger {
57-
readonly _accordionPanelPattern: WritableSignal<AccordionPanelPattern | undefined>;
53+
export class AccordionTrigger implements OnInit {
5854
readonly active: _angular_core.Signal<boolean>;
5955
collapse(): void;
6056
readonly disabled: _angular_core.InputSignalWithTransform<boolean, unknown>;
6157
readonly element: HTMLElement;
6258
expand(): void;
6359
readonly expanded: _angular_core.ModelSignal<boolean>;
6460
readonly id: _angular_core.InputSignal<string>;
65-
readonly panelId: _angular_core.InputSignal<string>;
66-
readonly _pattern: AccordionTriggerPattern;
61+
// (undocumented)
62+
ngOnInit(): void;
63+
readonly panel: _angular_core.InputSignal<AccordionPanel>;
64+
_pattern: AccordionTriggerPattern;
6765
toggle(): void;
6866
// (undocumented)
69-
static ɵdir: _angular_core.ɵɵDirectiveDeclaration<AccordionTrigger, "[ngAccordionTrigger]", ["ngAccordionTrigger"], { "id": { "alias": "id"; "required": false; "isSignal": true; }; "panelId": { "alias": "panelId"; "required": true; "isSignal": true; }; "disabled": { "alias": "disabled"; "required": false; "isSignal": true; }; "expanded": { "alias": "expanded"; "required": false; "isSignal": true; }; }, { "expanded": "expandedChange"; }, never, never, true, never>;
67+
static ɵdir: _angular_core.ɵɵDirectiveDeclaration<AccordionTrigger, "[ngAccordionTrigger]", ["ngAccordionTrigger"], { "panel": { "alias": "panel"; "required": true; "isSignal": true; }; "id": { "alias": "id"; "required": false; "isSignal": true; }; "disabled": { "alias": "disabled"; "required": false; "isSignal": true; }; "expanded": { "alias": "expanded"; "required": false; "isSignal": true; }; }, { "expanded": "expandedChange"; }, never, never, true, never>;
7068
// (undocumented)
7169
static ɵfac: _angular_core.ɵɵFactoryDeclaration<AccordionTrigger, never>;
7270
}

goldens/aria/private/index.api.md

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,13 @@ import { untracked } from '@angular/core/primitives/signals';
1010

1111
// @public
1212
export interface AccordionGroupInputs extends Omit<ListNavigationInputs<AccordionTriggerPattern> & ListFocusInputs<AccordionTriggerPattern> & Omit<ListExpansionInputs, 'items'>, 'focusMode'> {
13-
getItem: (e: Element | null | undefined) => AccordionTriggerPattern | undefined;
1413
}
1514

1615
// @public
1716
export class AccordionGroupPattern {
1817
constructor(inputs: AccordionGroupInputs);
18+
collapseAll(): void;
19+
expandAll(): void;
1920
readonly expansionBehavior: ListExpansion;
2021
readonly focusBehavior: ListFocus<AccordionTriggerPattern>;
2122
// (undocumented)
@@ -31,43 +32,24 @@ export class AccordionGroupPattern {
3132
toggle(): void;
3233
}
3334

34-
// @public
35-
export interface AccordionPanelInputs {
36-
accordionTrigger: SignalLike<AccordionTriggerPattern | undefined>;
37-
id: SignalLike<string>;
38-
panelId: SignalLike<string>;
39-
}
40-
41-
// @public
42-
export class AccordionPanelPattern {
43-
constructor(inputs: AccordionPanelInputs);
44-
accordionTrigger: SignalLike<AccordionTriggerPattern | undefined>;
45-
hidden: SignalLike<boolean>;
46-
id: SignalLike<string>;
47-
// (undocumented)
48-
readonly inputs: AccordionPanelInputs;
49-
}
50-
5135
// @public
5236
export interface AccordionTriggerInputs extends Omit<ListNavigationItem & ListFocusItem, 'index'>, Omit<ExpansionItem, 'expandable'> {
5337
accordionGroup: SignalLike<AccordionGroupPattern>;
54-
accordionPanel: SignalLike<AccordionPanelPattern | undefined>;
55-
panelId: SignalLike<string>;
38+
accordionPanelId: SignalLike<string>;
5639
}
5740

5841
// @public
5942
export class AccordionTriggerPattern implements ListNavigationItem, ListFocusItem, ExpansionItem {
6043
constructor(inputs: AccordionTriggerInputs);
6144
readonly active: SignalLike<boolean>;
6245
close(): void;
63-
readonly controls: SignalLike<string | undefined>;
46+
readonly controls: SignalLike<string>;
6447
readonly disabled: SignalLike<boolean>;
6548
readonly element: SignalLike<HTMLElement>;
6649
readonly expandable: SignalLike<boolean>;
6750
readonly expanded: WritableSignalLike<boolean>;
6851
readonly hardDisabled: SignalLike<boolean>;
6952
readonly id: SignalLike<string>;
70-
readonly index: SignalLike<number>;
7153
// (undocumented)
7254
readonly inputs: AccordionTriggerInputs;
7355
open(): void;

src/aria/accordion/accordion-group.ts

Lines changed: 13 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,16 @@
88

99
import {
1010
Directive,
11-
input,
1211
ElementRef,
13-
inject,
12+
booleanAttribute,
1413
contentChildren,
15-
afterRenderEffect,
14+
inject,
15+
input,
1616
signal,
17-
booleanAttribute,
18-
computed,
1917
} from '@angular/core';
2018
import {Directionality} from '@angular/cdk/bidi';
21-
import {AccordionGroupPattern, AccordionTriggerPattern} from '../private';
19+
import {AccordionGroupPattern} from '../private';
2220
import {AccordionTrigger} from './accordion-trigger';
23-
import {AccordionPanel} from './accordion-panel';
2421
import {ACCORDION_GROUP} from './accordion-tokens';
2522

2623
/**
@@ -32,22 +29,22 @@ import {ACCORDION_GROUP} from './accordion-tokens';
3229
* It supports both single and multiple expansion modes.
3330
*
3431
* ```html
35-
* <div ngAccordionGroup [multiExpandable]="true" [(expandedPanels)]="expandedItems">
32+
* <div ngAccordionGroup [multiExpandable]="true">
3633
* <div class="accordion-item">
3734
* <h3>
38-
* <button ngAccordionTrigger panelId="item-1">Item 1</button>
35+
* <button ngAccordionTrigger [panel]="panel1">Item 1</button>
3936
* </h3>
40-
* <div ngAccordionPanel panelId="item-1">
37+
* <div ngAccordionPanel #panel1="ngAccordionPanel">
4138
* <ng-template ngAccordionContent>
4239
* <p>Content for Item 1.</p>
4340
* </ng-template>
4441
* </div>
4542
* </div>
4643
* <div class="accordion-item">
4744
* <h3>
48-
* <button ngAccordionTrigger panelId="item-2">Item 2</button>
45+
* <button ngAccordionTrigger [panel]="panel2">Item 2</button>
4946
* </h3>
50-
* <div ngAccordionPanel panelId="item-2">
47+
* <div ngAccordionPanel #panel2="ngAccordionPanel">
5148
* <ng-template ngAccordionContent>
5249
* <p>Content for Item 2.</p>
5350
* </ng-template>
@@ -79,12 +76,6 @@ export class AccordionGroup {
7976
/** The AccordionTriggers nested inside this group. */
8077
private readonly _triggers = contentChildren(AccordionTrigger, {descendants: true});
8178

82-
/** The AccordionTrigger patterns nested inside this group. */
83-
private readonly _triggerPatterns = computed(() => this._triggers().map(t => t._pattern));
84-
85-
/** The AccordionPanels nested inside this group. */
86-
private readonly _panels = contentChildren(AccordionPanel, {descendants: true});
87-
8879
/** The text direction (ltr or rtl). */
8980
readonly textDirection = inject(Directionality).valueSignal;
9081

@@ -106,53 +97,20 @@ export class AccordionGroup {
10697
/** The UI pattern instance for this accordion group. */
10798
readonly _pattern: AccordionGroupPattern = new AccordionGroupPattern({
10899
...this,
100+
element: () => this.element,
109101
activeItem: signal(undefined),
110-
items: this._triggerPatterns,
102+
items: () => this._triggers().map(t => t._pattern),
111103
// TODO(ok7sai): Investigate whether an accordion should support horizontal mode.
112104
orientation: () => 'vertical',
113-
getItem: e => this._getItem(e),
114-
element: () => this.element,
115105
});
116106

117-
constructor() {
118-
// Effect to link triggers with their corresponding panels and update the group's items.
119-
afterRenderEffect(() => {
120-
const triggers = this._triggers();
121-
const panels = this._panels();
122-
123-
for (const trigger of triggers) {
124-
const panel = panels.find(p => p.panelId() === trigger.panelId());
125-
trigger._accordionPanelPattern.set(panel?._pattern);
126-
if (panel) {
127-
panel._accordionTriggerPattern.set(trigger._pattern);
128-
}
129-
}
130-
});
131-
}
132-
133107
/** Expands all accordion panels if multi-expandable. */
134108
expandAll() {
135-
this._pattern.expansionBehavior.openAll();
109+
this._pattern.expandAll();
136110
}
137111

138112
/** Collapses all accordion panels. */
139113
collapseAll() {
140-
this._pattern.expansionBehavior.closeAll();
141-
}
142-
143-
/** Gets the trigger pattern for a given element. */
144-
private _getItem(element: Element | null | undefined): AccordionTriggerPattern | undefined {
145-
let target = element;
146-
147-
while (target) {
148-
const pattern = this._triggerPatterns().find(t => t.element() === target);
149-
if (pattern) {
150-
return pattern;
151-
}
152-
153-
target = target.parentElement?.closest('[ngAccordionTrigger]');
154-
}
155-
156-
return undefined;
114+
this._pattern.collapseAll();
157115
}
158116
}

src/aria/accordion/accordion-panel.ts

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

9-
import {
10-
Directive,
11-
input,
12-
inject,
13-
afterRenderEffect,
14-
signal,
15-
computed,
16-
WritableSignal,
17-
} from '@angular/core';
9+
import {Directive, afterRenderEffect, computed, inject, input} from '@angular/core';
1810
import {_IdGenerator} from '@angular/cdk/a11y';
19-
import {DeferredContentAware, AccordionPanelPattern, AccordionTriggerPattern} from '../private';
11+
import {DeferredContentAware, AccordionTriggerPattern} from '../private';
2012

2113
/**
2214
* The content panel of an accordion item that is conditionally visible.
2315
*
24-
* This directive is a container for the content that is shown or hidden. It requires
25-
* a `panelId` that must match the `panelId` of its corresponding `ngAccordionTrigger`.
16+
* This directive is a container for the content that is shown or hidden. It should
17+
* expose a template reference that will be used by the corresponding `ngAccordionTrigger`.
2618
* The content within the panel should be provided using an `ng-template` with the
2719
* `ngAccordionContent` directive so that the content is not rendered on the page until the trigger
2820
* is expanded. It applies `role="region"` for accessibility and uses the `inert` attribute to hide
2921
* its content from assistive technologies when not visible.
3022
*
3123
* ```html
32-
* <div ngAccordionPanel panelId="unique-id-1">
24+
* <div ngAccordionPanel #panel="ngAccordionPanel">
3325
* <ng-template ngAccordionContent>
3426
* <p>This content is lazily rendered and will be shown when the panel is expanded.</p>
3527
* </ng-template>
@@ -50,8 +42,8 @@ import {DeferredContentAware, AccordionPanelPattern, AccordionTriggerPattern} fr
5042
],
5143
host: {
5244
'role': 'region',
53-
'[attr.id]': '_pattern.id()',
54-
'[attr.aria-labelledby]': '_pattern.accordionTrigger()?.id()',
45+
'[attr.id]': 'id()',
46+
'[attr.aria-labelledby]': '_pattern?.id()',
5547
'[attr.inert]': '!visible() ? true : null',
5648
},
5749
})
@@ -62,22 +54,15 @@ export class AccordionPanel {
6254
/** A global unique identifier for the panel. */
6355
readonly id = input(inject(_IdGenerator).getId('ng-accordion-panel-', true));
6456

65-
/** A local unique identifier for the panel, used to match with its trigger's `panelId`. */
66-
readonly panelId = input.required<string>();
67-
6857
/** Whether the accordion panel is visible. True if the associated trigger is expanded. */
69-
readonly visible = computed(() => !this._pattern.hidden());
70-
71-
/** The parent accordion trigger pattern that controls this panel. This is set by AccordionGroup. */
72-
readonly _accordionTriggerPattern: WritableSignal<AccordionTriggerPattern | undefined> =
73-
signal(undefined);
58+
readonly visible = computed(() => this._pattern?.expanded() === true);
7459

75-
/** The UI pattern instance for this panel. */
76-
readonly _pattern: AccordionPanelPattern = new AccordionPanelPattern({
77-
id: this.id,
78-
panelId: this.panelId,
79-
accordionTrigger: () => this._accordionTriggerPattern(),
80-
});
60+
/**
61+
* The pattern for the accordion trigger that controls this panel.
62+
* This is set by the trigger when it initializes.
63+
* There is no need for a panel pattern, as the trigger has all the necessary logic.
64+
*/
65+
_pattern?: AccordionTriggerPattern;
8166

8267
constructor() {
8368
// Connect the panel's hidden state to the DeferredContentAware's visibility.
@@ -88,16 +73,16 @@ export class AccordionPanel {
8873

8974
/** Expands this item. */
9075
expand() {
91-
this._accordionTriggerPattern()?.open();
76+
this._pattern?.open();
9277
}
9378

9479
/** Collapses this item. */
9580
collapse() {
96-
this._accordionTriggerPattern()?.close();
81+
this._pattern?.close();
9782
}
9883

9984
/** Toggles the expansion state of this item. */
10085
toggle() {
101-
this._accordionTriggerPattern()?.toggle();
86+
this._pattern?.toggle();
10287
}
10388
}

0 commit comments

Comments
 (0)