Skip to content

Commit be06cbf

Browse files
adolgachevadolgachev
committed
test(aria/accordion): check for incorrect usage of Accordion directives and log violations
1 parent c6e99f1 commit be06cbf

4 files changed

Lines changed: 242 additions & 2 deletions

File tree

‎src/aria/accordion/accordion-group.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
input,
1616
signal,
1717
afterNextRender,
18+
afterRenderEffect,
1819
OnDestroy,
1920
} from '@angular/core';
2021
import {Directionality} from '@angular/cdk/bidi';
@@ -114,6 +115,22 @@ export class AccordionGroup implements OnDestroy {
114115
afterNextRender(() => {
115116
this._collection.startObserving(this.element);
116117
});
118+
119+
// Check for any violations after the DOM has been updated.
120+
afterRenderEffect({
121+
read: () => {
122+
if (typeof ngDevMode === 'undefined' || ngDevMode) {
123+
if (!this.multiExpandable()) {
124+
const expandedCount = this._collection.orderedItems().filter(t => t.expanded()).length;
125+
if (expandedCount > 1) {
126+
console.error(
127+
'ngAccordionGroup has multiExpandable set to false, but multiple ngAccordionTrigger panels are initially expanded.',
128+
);
129+
}
130+
}
131+
}
132+
},
133+
});
117134
}
118135

119136
ngOnDestroy() {

‎src/aria/accordion/accordion-panel.ts‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,18 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import {Directive, ElementRef, afterRenderEffect, computed, inject, input} from '@angular/core';
9+
import {
10+
Directive,
11+
ElementRef,
12+
afterRenderEffect,
13+
computed,
14+
contentChild,
15+
inject,
16+
input,
17+
} from '@angular/core';
1018
import {_IdGenerator} from '@angular/cdk/a11y';
1119
import {DeferredContentAware, AccordionTriggerPattern} from '../private';
20+
import {AccordionContent} from './accordion-content';
1221

1322
/**
1423
* The content panel of an accordion item that is conditionally visible.
@@ -57,6 +66,8 @@ export class AccordionPanel {
5766
/** The DeferredContentAware host directive. */
5867
private readonly _deferredContentAware = inject(DeferredContentAware);
5968

69+
private readonly _accordionContent = contentChild(AccordionContent);
70+
6071
/** A global unique identifier for the panel. */
6172
readonly id = input(inject(_IdGenerator).getId('ng-accordion-panel-', true));
6273

@@ -77,6 +88,26 @@ export class AccordionPanel {
7788
this._deferredContentAware.contentVisible.set(this.visible());
7889
},
7990
});
91+
92+
// Check for any violations after the DOM has been updated.
93+
afterRenderEffect({
94+
read: () => {
95+
if (typeof ngDevMode === 'undefined' || ngDevMode) {
96+
const violations: string[] = [];
97+
98+
if (!this._accordionContent()) {
99+
violations.push('ngAccordionPanel must have an ngAccordionContent to render.');
100+
}
101+
if (!this._pattern) {
102+
violations.push('ngAccordionPanel must have an ngAccordionTrigger to control it.');
103+
}
104+
105+
for (const violation of violations) {
106+
console.error(violation);
107+
}
108+
}
109+
},
110+
});
80111
}
81112

82113
/** Expands this item. */

‎src/aria/accordion/accordion-trigger.ts‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
inject,
1717
input,
1818
model,
19+
afterRenderEffect,
1920
} from '@angular/core';
2021
import {_IdGenerator} from '@angular/cdk/a11y';
2122
import {AccordionTriggerPattern} from '../private';
@@ -85,6 +86,32 @@ export class AccordionTrigger implements OnInit, OnDestroy {
8586
/** The UI pattern instance for this trigger. */
8687
_pattern!: AccordionTriggerPattern;
8788

89+
constructor() {
90+
// Check for any violations after the DOM has been updated.
91+
afterRenderEffect({
92+
read: () => {
93+
if (typeof ngDevMode === 'undefined' || ngDevMode) {
94+
const violations: string[] = [];
95+
96+
if (this.panel() && this.panel().element.contains(this.element)) {
97+
violations.push(
98+
'ngAccordionTrigger must not be nested inside its controlled ngAccordionPanel, otherwise it will become unreachable when collapsed.',
99+
);
100+
}
101+
if (this.panel() && (this.panel() as any)._pattern !== this._pattern) {
102+
violations.push(
103+
'ngAccordionPanel is already controlled by another ngAccordionTrigger.',
104+
);
105+
}
106+
107+
for (const violation of violations) {
108+
console.error(violation);
109+
}
110+
}
111+
},
112+
});
113+
}
114+
88115
ngOnInit() {
89116
this._pattern = new AccordionTriggerPattern({
90117
...this,
@@ -93,7 +120,11 @@ export class AccordionTrigger implements OnInit, OnDestroy {
93120
accordionPanelId: this.panelId,
94121
});
95122

96-
this.panel()._pattern = this._pattern;
123+
// Only bind panel pattern if it wasn't already claimed, otherwise keep the original
124+
// to let the violation checker detect it at render time.
125+
if (this.panel() && !(this.panel() as any)._pattern) {
126+
this.panel()._pattern = this._pattern;
127+
}
97128

98129
this._accordionGroup._collection.register(this);
99130
}

‎src/aria/accordion/accordion.spec.ts‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,89 @@ describe('AccordionGroup', () => {
480480
});
481481
});
482482
});
483+
484+
describe('structural validations', () => {
485+
let consoleSpy: jasmine.Spy;
486+
487+
beforeEach(() => {
488+
consoleSpy = spyOn(console, 'error');
489+
});
490+
491+
afterEach(() => {
492+
TestBed.resetTestingModule();
493+
TestBed.configureTestingModule({
494+
imports: [AccordionGroupWithLoop],
495+
providers: [provideFakeDirectionality('ltr'), _IdGenerator],
496+
});
497+
fixture = TestBed.createComponent(AccordionGroupWithLoop);
498+
setupAccordionGroup();
499+
});
500+
501+
it('should warn when multiple triggers control the same panel', () => {
502+
TestBed.resetTestingModule();
503+
TestBed.configureTestingModule({
504+
imports: [AccordionWithDuplicateTriggers],
505+
});
506+
const duplicateFixture = TestBed.createComponent(AccordionWithDuplicateTriggers);
507+
duplicateFixture.detectChanges();
508+
509+
expect(consoleSpy).toHaveBeenCalledWith(
510+
'ngAccordionPanel is already controlled by another ngAccordionTrigger.',
511+
);
512+
});
513+
514+
it('should warn when trigger is nested inside its controlled panel', () => {
515+
TestBed.resetTestingModule();
516+
TestBed.configureTestingModule({
517+
imports: [AccordionWithNestedTrigger],
518+
});
519+
const nestedFixture = TestBed.createComponent(AccordionWithNestedTrigger);
520+
nestedFixture.detectChanges();
521+
522+
expect(consoleSpy).toHaveBeenCalledWith(
523+
'ngAccordionTrigger must not be nested inside its controlled ngAccordionPanel, otherwise it will become unreachable when collapsed.',
524+
);
525+
});
526+
527+
it('should warn when ngAccordionPanel is missing ngAccordionContent', () => {
528+
TestBed.resetTestingModule();
529+
TestBed.configureTestingModule({
530+
imports: [AccordionPanelWithoutContent],
531+
});
532+
const noContentFixture = TestBed.createComponent(AccordionPanelWithoutContent);
533+
noContentFixture.detectChanges();
534+
535+
expect(consoleSpy).toHaveBeenCalledWith(
536+
'ngAccordionPanel must have an ngAccordionContent to render.',
537+
);
538+
});
539+
540+
it('should warn when ngAccordionPanel is missing controlling trigger', () => {
541+
TestBed.resetTestingModule();
542+
TestBed.configureTestingModule({
543+
imports: [AccordionPanelWithoutTrigger],
544+
});
545+
const noTriggerFixture = TestBed.createComponent(AccordionPanelWithoutTrigger);
546+
noTriggerFixture.detectChanges();
547+
548+
expect(consoleSpy).toHaveBeenCalledWith(
549+
'ngAccordionPanel must have an ngAccordionTrigger to control it.',
550+
);
551+
});
552+
553+
it('should warn when multiple items are expanded in single-expand mode', () => {
554+
TestBed.resetTestingModule();
555+
TestBed.configureTestingModule({
556+
imports: [AccordionWithMultipleExpandedItems],
557+
});
558+
const multipleExpandedFixture = TestBed.createComponent(AccordionWithMultipleExpandedItems);
559+
multipleExpandedFixture.detectChanges();
560+
561+
expect(consoleSpy).toHaveBeenCalledWith(
562+
'ngAccordionGroup has multiExpandable set to false, but multiple ngAccordionTrigger panels are initially expanded.',
563+
);
564+
});
565+
});
483566
});
484567

485568
@Component({
@@ -606,3 +689,81 @@ class AccordionGroupWithIfs extends AccordionGroupWithLoop {
606689
includeSecond = signal(true);
607690
includeThird = signal(true);
608691
}
692+
693+
@Component({
694+
template: `
695+
<div ngAccordionGroup>
696+
<button ngAccordionTrigger [panel]="panel1">Trigger 1</button>
697+
<button ngAccordionTrigger [panel]="panel1">Trigger 2</button>
698+
<div ngAccordionPanel #panel1="ngAccordionPanel">
699+
<ng-template ngAccordionContent>Content</ng-template>
700+
</div>
701+
</div>
702+
`,
703+
imports: [AccordionGroup, AccordionTrigger, AccordionPanel, AccordionContent],
704+
changeDetection: ChangeDetectionStrategy.Eager,
705+
})
706+
class AccordionWithDuplicateTriggers {}
707+
708+
@Component({
709+
template: `
710+
<div ngAccordionGroup>
711+
<div ngAccordionPanel #panel1="ngAccordionPanel">
712+
<button ngAccordionTrigger [panel]="panel1">Nested Trigger</button>
713+
<ng-template ngAccordionContent>Content</ng-template>
714+
</div>
715+
</div>
716+
`,
717+
imports: [AccordionGroup, AccordionTrigger, AccordionPanel, AccordionContent],
718+
changeDetection: ChangeDetectionStrategy.Eager,
719+
})
720+
class AccordionWithNestedTrigger {}
721+
722+
@Component({
723+
template: `
724+
<div ngAccordionGroup>
725+
<button ngAccordionTrigger [panel]="panel1">Trigger</button>
726+
<div ngAccordionPanel #panel1="ngAccordionPanel">
727+
Content
728+
</div>
729+
</div>
730+
`,
731+
imports: [AccordionGroup, AccordionTrigger, AccordionPanel],
732+
changeDetection: ChangeDetectionStrategy.Eager,
733+
})
734+
class AccordionPanelWithoutContent {}
735+
736+
@Component({
737+
template: `
738+
<div ngAccordionGroup>
739+
<div ngAccordionPanel>
740+
<ng-template ngAccordionContent>Content</ng-template>
741+
</div>
742+
</div>
743+
`,
744+
imports: [AccordionGroup, AccordionPanel, AccordionContent],
745+
changeDetection: ChangeDetectionStrategy.Eager,
746+
})
747+
class AccordionPanelWithoutTrigger {}
748+
749+
@Component({
750+
template: `
751+
<div ngAccordionGroup [multiExpandable]="false">
752+
<div>
753+
<button ngAccordionTrigger [panel]="panel1" [expanded]="true">Trigger 1</button>
754+
<div ngAccordionPanel #panel1="ngAccordionPanel">
755+
<ng-template ngAccordionContent>Content 1</ng-template>
756+
</div>
757+
</div>
758+
<div>
759+
<button ngAccordionTrigger [panel]="panel2" [expanded]="true">Trigger 2</button>
760+
<div ngAccordionPanel #panel2="ngAccordionPanel">
761+
<ng-template ngAccordionContent>Content 2</ng-template>
762+
</div>
763+
</div>
764+
</div>
765+
`,
766+
imports: [AccordionGroup, AccordionTrigger, AccordionPanel, AccordionContent],
767+
changeDetection: ChangeDetectionStrategy.Eager,
768+
})
769+
class AccordionWithMultipleExpandedItems {}

0 commit comments

Comments
 (0)