Skip to content

Commit b6c5df6

Browse files
committed
refactor(aria/accordion): simplify code by using template references instead of user id to match panels with triggers (angular#32855)
(cherry picked from commit c2cb1ab)
1 parent 484a61e commit b6c5df6

22 files changed

+247
-528
lines changed

goldens/aria/accordion/index.api.md

Lines changed: 11 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,43 @@ 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+
readonly panelId: _angular_core.Signal<string>;
65+
_pattern: AccordionTriggerPattern;
6766
toggle(): void;
6867
// (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>;
68+
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>;
7069
// (undocumented)
7170
static ɵfac: _angular_core.ɵɵFactoryDeclaration<AccordionTrigger, never>;
7271
}

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: 14 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,17 @@
88

99
import {
1010
Directive,
11-
input,
1211
ElementRef,
13-
inject,
14-
contentChildren,
15-
afterRenderEffect,
16-
signal,
1712
booleanAttribute,
1813
computed,
14+
contentChildren,
15+
inject,
16+
input,
17+
signal,
1918
} from '@angular/core';
2019
import {Directionality} from '@angular/cdk/bidi';
21-
import {AccordionGroupPattern, AccordionTriggerPattern} from '../private';
20+
import {AccordionGroupPattern} from '../private';
2221
import {AccordionTrigger} from './accordion-trigger';
23-
import {AccordionPanel} from './accordion-panel';
2422
import {ACCORDION_GROUP} from './accordion-tokens';
2523

2624
/**
@@ -32,22 +30,22 @@ import {ACCORDION_GROUP} from './accordion-tokens';
3230
* It supports both single and multiple expansion modes.
3331
*
3432
* ```html
35-
* <div ngAccordionGroup [multiExpandable]="true" [(expandedPanels)]="expandedItems">
33+
* <div ngAccordionGroup [multiExpandable]="true">
3634
* <div class="accordion-item">
3735
* <h3>
38-
* <button ngAccordionTrigger panelId="item-1">Item 1</button>
36+
* <button ngAccordionTrigger [panel]="panel1">Item 1</button>
3937
* </h3>
40-
* <div ngAccordionPanel panelId="item-1">
38+
* <div ngAccordionPanel #panel1="ngAccordionPanel">
4139
* <ng-template ngAccordionContent>
4240
* <p>Content for Item 1.</p>
4341
* </ng-template>
4442
* </div>
4543
* </div>
4644
* <div class="accordion-item">
4745
* <h3>
48-
* <button ngAccordionTrigger panelId="item-2">Item 2</button>
46+
* <button ngAccordionTrigger [panel]="panel2">Item 2</button>
4947
* </h3>
50-
* <div ngAccordionPanel panelId="item-2">
48+
* <div ngAccordionPanel #panel2="ngAccordionPanel">
5149
* <ng-template ngAccordionContent>
5250
* <p>Content for Item 2.</p>
5351
* </ng-template>
@@ -79,12 +77,9 @@ export class AccordionGroup {
7977
/** The AccordionTriggers nested inside this group. */
8078
private readonly _triggers = contentChildren(AccordionTrigger, {descendants: true});
8179

82-
/** The AccordionTrigger patterns nested inside this group. */
80+
/** The corresponding patterns for the accordion triggers. */
8381
private readonly _triggerPatterns = computed(() => this._triggers().map(t => t._pattern));
8482

85-
/** The AccordionPanels nested inside this group. */
86-
private readonly _panels = contentChildren(AccordionPanel, {descendants: true});
87-
8883
/** The text direction (ltr or rtl). */
8984
readonly textDirection = inject(Directionality).valueSignal;
9085

@@ -106,53 +101,20 @@ export class AccordionGroup {
106101
/** The UI pattern instance for this accordion group. */
107102
readonly _pattern: AccordionGroupPattern = new AccordionGroupPattern({
108103
...this,
104+
element: () => this.element,
109105
activeItem: signal(undefined),
110106
items: this._triggerPatterns,
111107
// TODO(ok7sai): Investigate whether an accordion should support horizontal mode.
112108
orientation: () => 'vertical',
113-
getItem: e => this._getItem(e),
114-
element: () => this.element,
115109
});
116110

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-
133111
/** Expands all accordion panels if multi-expandable. */
134112
expandAll() {
135-
this._pattern.expansionBehavior.openAll();
113+
this._pattern.expandAll();
136114
}
137115

138116
/** Collapses all accordion panels. */
139117
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;
118+
this._pattern.collapseAll();
157119
}
158120
}

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)