Skip to content

Commit 8a745a6

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

4 files changed

Lines changed: 191 additions & 2 deletions

File tree

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

Lines changed: 1 addition & 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';

‎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: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,76 @@ 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+
});
483553
});
484554

485555
@Component({
@@ -606,3 +676,59 @@ class AccordionGroupWithIfs extends AccordionGroupWithLoop {
606676
includeSecond = signal(true);
607677
includeThird = signal(true);
608678
}
679+
680+
@Component({
681+
template: `
682+
<div ngAccordionGroup>
683+
<button ngAccordionTrigger [panel]="panel1">Trigger 1</button>
684+
<button ngAccordionTrigger [panel]="panel1">Trigger 2</button>
685+
<div ngAccordionPanel #panel1="ngAccordionPanel">
686+
<ng-template ngAccordionContent>Content</ng-template>
687+
</div>
688+
</div>
689+
`,
690+
imports: [AccordionGroup, AccordionTrigger, AccordionPanel, AccordionContent],
691+
changeDetection: ChangeDetectionStrategy.Eager,
692+
})
693+
class AccordionWithDuplicateTriggers {}
694+
695+
@Component({
696+
template: `
697+
<div ngAccordionGroup>
698+
<div ngAccordionPanel #panel1="ngAccordionPanel">
699+
<button ngAccordionTrigger [panel]="panel1">Nested Trigger</button>
700+
<ng-template ngAccordionContent>Content</ng-template>
701+
</div>
702+
</div>
703+
`,
704+
imports: [AccordionGroup, AccordionTrigger, AccordionPanel, AccordionContent],
705+
changeDetection: ChangeDetectionStrategy.Eager,
706+
})
707+
class AccordionWithNestedTrigger {}
708+
709+
@Component({
710+
template: `
711+
<div ngAccordionGroup>
712+
<button ngAccordionTrigger [panel]="panel1">Trigger</button>
713+
<div ngAccordionPanel #panel1="ngAccordionPanel">
714+
Content
715+
</div>
716+
</div>
717+
`,
718+
imports: [AccordionGroup, AccordionTrigger, AccordionPanel],
719+
changeDetection: ChangeDetectionStrategy.Eager,
720+
})
721+
class AccordionPanelWithoutContent {}
722+
723+
@Component({
724+
template: `
725+
<div ngAccordionGroup>
726+
<div ngAccordionPanel>
727+
<ng-template ngAccordionContent>Content</ng-template>
728+
</div>
729+
</div>
730+
`,
731+
imports: [AccordionGroup, AccordionPanel, AccordionContent],
732+
changeDetection: ChangeDetectionStrategy.Eager,
733+
})
734+
class AccordionPanelWithoutTrigger {}

0 commit comments

Comments
 (0)