Skip to content

Commit 221402f

Browse files
authored
fix(aria/accordion): rename value to panelId for trigger and panel (#32295)
* fix(aria/accordion): rename value to panelId for trigger and panel * refactor: fix tests * refactor: fix tests
1 parent ea08fb0 commit 221402f

File tree

11 files changed

+131
-126
lines changed

11 files changed

+131
-126
lines changed

src/aria/accordion/accordion.spec.ts

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ describe('AccordionGroup', () => {
3232
const endKey = (target: HTMLElement) => keydown(target, 'End');
3333

3434
interface SetupOptions {
35-
initialValue?: string[];
35+
initialExpandedPanels?: string[];
3636
multiExpandable?: boolean;
3737
disabledGroup?: boolean;
3838
disabledItemValues?: string[];
@@ -43,8 +43,8 @@ describe('AccordionGroup', () => {
4343
function configureAccordionComponent(opts: SetupOptions = {}) {
4444
const testComponent = fixture.componentInstance as AccordionGroupExample;
4545

46-
if (opts.initialValue !== undefined) {
47-
testComponent.value.set(opts.initialValue);
46+
if (opts.initialExpandedPanels !== undefined) {
47+
testComponent.expandedPanels.set(opts.initialExpandedPanels);
4848
}
4949
if (opts.multiExpandable !== undefined) {
5050
testComponent.multiExpandable.set(opts.multiExpandable);
@@ -109,7 +109,7 @@ describe('AccordionGroup', () => {
109109
});
110110

111111
it('should have aria-expanded="false" when collapsed', () => {
112-
configureAccordionComponent({initialValue: []});
112+
configureAccordionComponent({initialExpandedPanels: []});
113113
expect(triggerElements[0].getAttribute('aria-expanded')).toBe('false');
114114
expect(triggerElements[1].getAttribute('aria-expanded')).toBe('false');
115115
expect(triggerElements[2].getAttribute('aria-expanded')).toBe('false');
@@ -154,7 +154,7 @@ describe('AccordionGroup', () => {
154154
});
155155

156156
it('should have "inert" attribute when collapsed', () => {
157-
configureAccordionComponent({initialValue: []});
157+
configureAccordionComponent({initialExpandedPanels: []});
158158
expect(panelElements[0].hasAttribute('inert')).toBeTrue();
159159
expect(panelElements[1].hasAttribute('inert')).toBeTrue();
160160
expect(panelElements[2].hasAttribute('inert')).toBeTrue();
@@ -168,36 +168,36 @@ describe('AccordionGroup', () => {
168168
configureAccordionComponent({multiExpandable: false});
169169
});
170170

171-
it('should expand panel on trigger click and update value', () => {
171+
it('should expand panel on trigger click and update expanded panels', () => {
172172
click(triggerElements[0]);
173173
expect(isTriggerExpanded(triggerElements[0])).toBeTrue();
174174
expect(panelElements[0].hasAttribute('inert')).toBeFalse();
175-
expect(groupInstance.value()).toEqual(['item-1']);
175+
expect(groupInstance.expandedPanels()).toEqual(['item-1']);
176176
});
177177

178-
it('should collapes panel on trigger click and update value', () => {
178+
it('should collapes panel on trigger click and update expanded panels', () => {
179179
click(triggerElements[0]);
180180
click(triggerElements[0]); // Collapse
181181
expect(isTriggerExpanded(triggerElements[0])).toBeFalse();
182182
expect(panelElements[0].hasAttribute('inert')).toBeTrue();
183-
expect(groupInstance.value()).toEqual([]);
183+
expect(groupInstance.expandedPanels()).toEqual([]);
184184
});
185185

186186
it('should expand one and collapse others', () => {
187187
click(triggerElements[0]);
188188
expect(isTriggerExpanded(triggerElements[0])).toBeTrue();
189-
expect(groupInstance.value()).toEqual(['item-1']);
189+
expect(groupInstance.expandedPanels()).toEqual(['item-1']);
190190

191191
click(triggerElements[1]);
192192
expect(isTriggerExpanded(triggerElements[0])).toBeFalse();
193193
expect(panelElements[0].hasAttribute('inert')).toBeTrue();
194194
expect(isTriggerExpanded(triggerElements[1])).toBeTrue();
195195
expect(panelElements[1].hasAttribute('inert')).toBeFalse();
196-
expect(groupInstance.value()).toEqual(['item-2']);
196+
expect(groupInstance.expandedPanels()).toEqual(['item-2']);
197197
});
198198

199199
it('should allow setting initial value', () => {
200-
configureAccordionComponent({initialValue: ['item-2'], multiExpandable: false});
200+
configureAccordionComponent({initialExpandedPanels: ['item-2'], multiExpandable: false});
201201
expect(isTriggerExpanded(triggerElements[0])).toBeFalse();
202202
expect(isTriggerExpanded(triggerElements[1])).toBeTrue();
203203
expect(isTriggerExpanded(triggerElements[2])).toBeFalse();
@@ -221,16 +221,21 @@ describe('AccordionGroup', () => {
221221
it('should collapse an item without affecting others', () => {
222222
click(triggerElements[0]);
223223
click(triggerElements[1]);
224-
expect(groupInstance.value()).toEqual(jasmine.arrayWithExactContents(['item-1', 'item-2']));
224+
expect(groupInstance.expandedPanels()).toEqual(
225+
jasmine.arrayWithExactContents(['item-1', 'item-2']),
226+
);
225227

226228
click(triggerElements[0]);
227229
expect(isTriggerExpanded(triggerElements[0])).toBeFalse();
228230
expect(isTriggerExpanded(triggerElements[1])).toBeTrue();
229-
expect(groupInstance.value()).toEqual(['item-2']);
231+
expect(groupInstance.expandedPanels()).toEqual(['item-2']);
230232
});
231233

232234
it('should allow setting initial multiple values', () => {
233-
configureAccordionComponent({initialValue: ['item-1', 'item-3'], multiExpandable: true});
235+
configureAccordionComponent({
236+
initialExpandedPanels: ['item-1', 'item-3'],
237+
multiExpandable: true,
238+
});
234239
expect(isTriggerExpanded(triggerElements[0])).toBeTrue();
235240
expect(isTriggerExpanded(triggerElements[1])).toBeFalse();
236241
expect(isTriggerExpanded(triggerElements[2])).toBeTrue();
@@ -242,15 +247,15 @@ describe('AccordionGroup', () => {
242247
configureAccordionComponent({disabledItemValues: ['item-1']});
243248
click(triggerElements[0]);
244249
expect(isTriggerExpanded(triggerElements[0])).toBeFalse();
245-
expect(groupInstance.value()).toEqual([]);
250+
expect(groupInstance.expandedPanels()).toEqual([]);
246251
expect(triggerElements[0].getAttribute('aria-disabled')).toBe('true');
247252
});
248253

249254
it('should not expand any trigger if group is disabled', () => {
250255
configureAccordionComponent({disabledGroup: true});
251256
click(triggerElements[0]);
252257
expect(isTriggerExpanded(triggerElements[0])).toBeFalse();
253-
expect(groupInstance.value()).toEqual([]);
258+
expect(groupInstance.expandedPanels()).toEqual([]);
254259
click(triggerElements[1]);
255260
expect(isTriggerExpanded(triggerElements[1])).toBeFalse();
256261
});
@@ -382,22 +387,22 @@ describe('AccordionGroup', () => {
382387
template: `
383388
<div
384389
ngAccordionGroup
385-
[(value)]="value"
390+
[(expandedPanels)]="expandedPanels"
386391
[multiExpandable]="multiExpandable()"
387392
[disabled]="disabledGroup()"
388393
[softDisabled]="softDisabled()"
389394
[wrap]="wrap()"
390395
>
391-
@for (item of items(); track item.value) {
396+
@for (item of items(); track item.panelId) {
392397
<div class="item-container">
393398
<button
394399
ngAccordionTrigger
395-
[value]="item.value"
400+
[panelId]="item.panelId"
396401
[disabled]="item.disabled"
397402
>{{ item.header }}</button>
398403
<div
399404
ngAccordionPanel
400-
[value]="item.value"
405+
[panelId]="item.panelId"
401406
>
402407
<ng-template ngAccordionContent>
403408
{{ item.content }}
@@ -411,20 +416,20 @@ describe('AccordionGroup', () => {
411416
})
412417
class AccordionGroupExample {
413418
items = signal([
414-
{value: 'item-1', header: 'Item 1 Header', content: 'Item 1 Content', disabled: false},
415-
{value: 'item-2', header: 'Item 2 Header', content: 'Item 2 Content', disabled: false},
416-
{value: 'item-3', header: 'Item 3 Header', content: 'Item 3 Content', disabled: false},
419+
{panelId: 'item-1', header: 'Item 1 Header', content: 'Item 1 Content', disabled: false},
420+
{panelId: 'item-2', header: 'Item 2 Header', content: 'Item 2 Content', disabled: false},
421+
{panelId: 'item-3', header: 'Item 3 Header', content: 'Item 3 Content', disabled: false},
417422
]);
418423

419-
value = model<string[]>([]);
424+
expandedPanels = model<string[]>([]);
420425
multiExpandable = signal(false);
421426
disabledGroup = signal(false);
422427
softDisabled = signal(true);
423428
wrap = signal(false);
424429

425430
disableItem(itemValue: string, disabled: boolean) {
426431
this.items.update(items =>
427-
items.map(item => (item.value === itemValue ? {...item, disabled} : item)),
432+
items.map(item => (item.panelId === itemValue ? {...item, disabled} : item)),
428433
);
429434
}
430435
}

src/aria/accordion/accordion.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ export class AccordionPanel {
5757
/** A global unique identifier for the panel. */
5858
private readonly _id = inject(_IdGenerator).getId('accordion-trigger-', true);
5959

60-
/** A local unique identifier for the panel, used to match with its trigger's value. */
61-
value = input.required<string>();
60+
/** A local unique identifier for the panel, used to match with its trigger's `panelId`. */
61+
panelId = input.required<string>();
6262

6363
/** Whether the accordion panel is visible. True if the associated trigger is expanded. */
6464
readonly visible = computed(() => !this._pattern.hidden());
@@ -70,7 +70,7 @@ export class AccordionPanel {
7070
/** The UI pattern instance for this panel. */
7171
readonly _pattern: AccordionPanelPattern = new AccordionPanelPattern({
7272
id: () => this._id,
73-
value: this.value,
73+
panelId: this.panelId,
7474
accordionTrigger: () => this.accordionTrigger(),
7575
});
7676

@@ -129,8 +129,8 @@ export class AccordionTrigger {
129129
/** The parent AccordionGroup. */
130130
private readonly _accordionGroup = inject(AccordionGroup);
131131

132-
/** A local unique identifier for the trigger, used to match with its panel's value. */
133-
value = input.required<string>();
132+
/** A local unique identifier for the trigger, used to match with its panel's `panelId`. */
133+
panelId = input.required<string>();
134134

135135
/** Whether the trigger is disabled. */
136136
disabled = input(false, {transform: booleanAttribute});
@@ -154,7 +154,7 @@ export class AccordionTrigger {
154154
/** The UI pattern instance for this trigger. */
155155
readonly _pattern: AccordionTriggerPattern = new AccordionTriggerPattern({
156156
id: () => this._id,
157-
value: this.value,
157+
panelId: this.panelId,
158158
disabled: this.disabled,
159159
element: () => this._elementRef.nativeElement,
160160
accordionGroup: computed(() => this._accordionGroup._pattern),
@@ -207,8 +207,8 @@ export class AccordionGroup {
207207
/** Whether multiple accordion items can be expanded simultaneously. */
208208
multiExpandable = input(true, {transform: booleanAttribute});
209209

210-
/** The values of the current selected/expanded accordions. */
211-
value = model<string[]>([]);
210+
/** The ids of the current expanded accordion panels. */
211+
expandedPanels = model<string[]>([]);
212212

213213
/** Whether to allow disabled items to receive focus. */
214214
softDisabled = input(true, {transform: booleanAttribute});
@@ -223,7 +223,7 @@ export class AccordionGroup {
223223
// `setDefaultState` in the CDK.
224224
activeItem: signal(undefined),
225225
items: computed(() => this._triggers().map(trigger => trigger._pattern)),
226-
expandedIds: this.value,
226+
expandedIds: this.expandedPanels,
227227
// TODO(ok7sai): Investigate whether an accordion should support horizontal mode.
228228
orientation: () => 'vertical',
229229
element: () => this._elementRef.nativeElement,
@@ -236,7 +236,7 @@ export class AccordionGroup {
236236
const panels = this._panels();
237237

238238
for (const trigger of triggers) {
239-
const panel = panels.find(p => p.value() === trigger.value());
239+
const panel = panels.find(p => p.panelId() === trigger.panelId());
240240
trigger.accordionPanel.set(panel?._pattern);
241241
if (panel) {
242242
panel.accordionTrigger.set(trigger._pattern);

src/aria/private/accordion/accordion.spec.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,23 +80,23 @@ describe('Accordion Pattern', () => {
8080
id: signal('trigger-1-id'),
8181
element: signal(createAccordionTriggerElement()),
8282
disabled: signal(false),
83-
value: signal('panel-1'), // Value should match the panel it controls
83+
panelId: signal('panel-1'), // Value should match the panel it controls
8484
},
8585
{
8686
accordionGroup: signal(groupPattern),
8787
accordionPanel: signal(undefined),
8888
id: signal('trigger-2-id'),
8989
element: signal(createAccordionTriggerElement()),
9090
disabled: signal(false),
91-
value: signal('panel-2'),
91+
panelId: signal('panel-2'),
9292
},
9393
{
9494
accordionGroup: signal(groupPattern),
9595
accordionPanel: signal(undefined),
9696
id: signal('trigger-3-id'),
9797
element: signal(createAccordionTriggerElement()),
9898
disabled: signal(false),
99-
value: signal('panel-3'),
99+
panelId: signal('panel-3'),
100100
},
101101
];
102102
triggerPatterns = [
@@ -111,17 +111,17 @@ describe('Accordion Pattern', () => {
111111
panelInputs = [
112112
{
113113
id: signal('panel-1-id'),
114-
value: signal('panel-1'),
114+
panelId: signal('panel-1'),
115115
accordionTrigger: signal(undefined),
116116
},
117117
{
118118
id: signal('panel-2-id'),
119-
value: signal('panel-2'),
119+
panelId: signal('panel-2'),
120120
accordionTrigger: signal(undefined),
121121
},
122122
{
123123
id: signal('panel-3-id'),
124-
value: signal('panel-3'),
124+
panelId: signal('panel-3'),
125125
accordionTrigger: signal(undefined),
126126
},
127127
];

src/aria/private/accordion/accordion.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ export class AccordionGroupPattern {
7272
/** Inputs for the AccordionTriggerPattern. */
7373
export type AccordionTriggerInputs = Omit<ListNavigationItem & ListFocusItem, 'index'> &
7474
Omit<ExpansionItem, 'expansionId' | 'expandable'> & {
75-
/** A local unique identifier for the trigger. */
76-
value: SignalLike<string>;
75+
/** A local unique identifier for the trigger's corresponding panel. */
76+
panelId: SignalLike<string>;
7777

7878
/** The parent accordion group that controls this trigger. */
7979
accordionGroup: SignalLike<AccordionGroupPattern>;
@@ -115,10 +115,10 @@ export class AccordionTriggerPattern {
115115
constructor(readonly inputs: AccordionTriggerInputs) {
116116
this.id = inputs.id;
117117
this.element = inputs.element;
118-
this.value = inputs.value;
118+
this.panelId = inputs.panelId;
119119
this.expansionControl = new ExpansionControl({
120120
...inputs,
121-
expansionId: inputs.value,
121+
expansionId: inputs.panelId,
122122
expandable: () => true,
123123
expansionManager: inputs.accordionGroup().expansionManager,
124124
});
@@ -203,8 +203,8 @@ export interface AccordionPanelInputs {
203203
/** A global unique identifier for the panel. */
204204
id: SignalLike<string>;
205205

206-
/** A local unique identifier for the panel, matching its trigger's value. */
207-
value: SignalLike<string>;
206+
/** A local unique identifier for the panel, matching its trigger's panelId. */
207+
panelId: SignalLike<string>;
208208

209209
/** The parent accordion trigger that controls this panel. */
210210
accordionTrigger: SignalLike<AccordionTriggerPattern | undefined>;
@@ -218,7 +218,7 @@ export class AccordionPanelPattern {
218218

219219
constructor(readonly inputs: AccordionPanelInputs) {
220220
this.id = inputs.id;
221-
this.value = inputs.value;
221+
this.panelId = inputs.panelId;
222222
this.accordionTrigger = inputs.accordionTrigger;
223223
this.hidden = computed(() => inputs.accordionTrigger()?.expanded() === false);
224224
}

0 commit comments

Comments
 (0)