Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/aria/accordion/accordion-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
input,
signal,
afterNextRender,
afterRenderEffect,
OnDestroy,
} from '@angular/core';
import {Directionality} from '@angular/cdk/bidi';
Expand Down Expand Up @@ -114,6 +115,22 @@ export class AccordionGroup implements OnDestroy {
afterNextRender(() => {
this._collection.startObserving(this.element);
});

// Check for any violations after the DOM has been updated.
afterRenderEffect({
read: () => {
if (typeof ngDevMode === 'undefined' || ngDevMode) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this outside of afterRenderEffect to avoid creating an effect in production.

if (!this.multiExpandable()) {
const expandedCount = this._collection.orderedItems().filter(t => t.expanded()).length;
if (expandedCount > 1) {
console.error(
'ngAccordionGroup has multiExpandable set to false, but multiple ngAccordionTrigger panels are initially expanded.',
);
}
}
}
},
});
}

ngOnDestroy() {
Expand Down
33 changes: 32 additions & 1 deletion src/aria/accordion/accordion-panel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,18 @@
* found in the LICENSE file at https://angular.dev/license
*/

import {Directive, ElementRef, afterRenderEffect, computed, inject, input} from '@angular/core';
import {
Directive,
ElementRef,
afterRenderEffect,
computed,
contentChild,
inject,
input,
} from '@angular/core';
import {_IdGenerator} from '@angular/cdk/a11y';
import {DeferredContentAware, AccordionTriggerPattern} from '../private';
import {AccordionContent} from './accordion-content';

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

private readonly _accordionContent = contentChild(AccordionContent);

/** A global unique identifier for the panel. */
readonly id = input(inject(_IdGenerator).getId('ng-accordion-panel-', true));

Expand All @@ -77,6 +88,26 @@ export class AccordionPanel {
this._deferredContentAware.contentVisible.set(this.visible());
},
});

// Check for any violations after the DOM has been updated.
afterRenderEffect({
read: () => {
if (typeof ngDevMode === 'undefined' || ngDevMode) {
const violations: string[] = [];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to have violations created within the private UI pattern classes.


if (!this._accordionContent()) {
violations.push('ngAccordionPanel must have an ngAccordionContent to render.');
}
if (!this._pattern) {
violations.push('ngAccordionPanel must have an ngAccordionTrigger to control it.');
}

for (const violation of violations) {
console.error(violation);
}
}
},
});
}

/** Expands this item. */
Expand Down
33 changes: 32 additions & 1 deletion src/aria/accordion/accordion-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
inject,
input,
model,
afterRenderEffect,
} from '@angular/core';
import {_IdGenerator} from '@angular/cdk/a11y';
import {AccordionTriggerPattern} from '../private';
Expand Down Expand Up @@ -85,6 +86,32 @@ export class AccordionTrigger implements OnInit, OnDestroy {
/** The UI pattern instance for this trigger. */
_pattern!: AccordionTriggerPattern;

constructor() {
// Check for any violations after the DOM has been updated.
afterRenderEffect({
read: () => {
if (typeof ngDevMode === 'undefined' || ngDevMode) {
const violations: string[] = [];

if (this.panel() && this.panel().element.contains(this.element)) {
violations.push(
'ngAccordionTrigger must not be nested inside its controlled ngAccordionPanel, otherwise it will become unreachable when collapsed.',
);
}
if (this.panel() && (this.panel() as any)._pattern !== this._pattern) {
violations.push(
'ngAccordionPanel is already controlled by another ngAccordionTrigger.',
);
}

for (const violation of violations) {
console.error(violation);
}
}
},
});
}

ngOnInit() {
this._pattern = new AccordionTriggerPattern({
...this,
Expand All @@ -93,7 +120,11 @@ export class AccordionTrigger implements OnInit, OnDestroy {
accordionPanelId: this.panelId,
});

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

this._accordionGroup._collection.register(this);
}
Expand Down
161 changes: 161 additions & 0 deletions src/aria/accordion/accordion.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,89 @@ describe('AccordionGroup', () => {
});
});
});

describe('structural validations', () => {
let consoleSpy: jasmine.Spy;

beforeEach(() => {
consoleSpy = spyOn(console, 'error');
});

afterEach(() => {
TestBed.resetTestingModule();
TestBed.configureTestingModule({
imports: [AccordionGroupWithLoop],
providers: [provideFakeDirectionality('ltr'), _IdGenerator],
});
fixture = TestBed.createComponent(AccordionGroupWithLoop);
setupAccordionGroup();
});

it('should warn when multiple triggers control the same panel', () => {
TestBed.resetTestingModule();
TestBed.configureTestingModule({
imports: [AccordionWithDuplicateTriggers],
});
const duplicateFixture = TestBed.createComponent(AccordionWithDuplicateTriggers);
duplicateFixture.detectChanges();

expect(consoleSpy).toHaveBeenCalledWith(
'ngAccordionPanel is already controlled by another ngAccordionTrigger.',
);
});

it('should warn when trigger is nested inside its controlled panel', () => {
TestBed.resetTestingModule();
TestBed.configureTestingModule({
imports: [AccordionWithNestedTrigger],
});
const nestedFixture = TestBed.createComponent(AccordionWithNestedTrigger);
nestedFixture.detectChanges();

expect(consoleSpy).toHaveBeenCalledWith(
'ngAccordionTrigger must not be nested inside its controlled ngAccordionPanel, otherwise it will become unreachable when collapsed.',
);
});

it('should warn when ngAccordionPanel is missing ngAccordionContent', () => {
TestBed.resetTestingModule();
TestBed.configureTestingModule({
imports: [AccordionPanelWithoutContent],
});
const noContentFixture = TestBed.createComponent(AccordionPanelWithoutContent);
noContentFixture.detectChanges();

expect(consoleSpy).toHaveBeenCalledWith(
'ngAccordionPanel must have an ngAccordionContent to render.',
);
});

it('should warn when ngAccordionPanel is missing controlling trigger', () => {
TestBed.resetTestingModule();
TestBed.configureTestingModule({
imports: [AccordionPanelWithoutTrigger],
});
const noTriggerFixture = TestBed.createComponent(AccordionPanelWithoutTrigger);
noTriggerFixture.detectChanges();

expect(consoleSpy).toHaveBeenCalledWith(
'ngAccordionPanel must have an ngAccordionTrigger to control it.',
);
});

it('should warn when multiple items are expanded in single-expand mode', () => {
TestBed.resetTestingModule();
TestBed.configureTestingModule({
imports: [AccordionWithMultipleExpandedItems],
});
const multipleExpandedFixture = TestBed.createComponent(AccordionWithMultipleExpandedItems);
multipleExpandedFixture.detectChanges();

expect(consoleSpy).toHaveBeenCalledWith(
'ngAccordionGroup has multiExpandable set to false, but multiple ngAccordionTrigger panels are initially expanded.',
);
});
});
});

@Component({
Expand Down Expand Up @@ -606,3 +689,81 @@ class AccordionGroupWithIfs extends AccordionGroupWithLoop {
includeSecond = signal(true);
includeThird = signal(true);
}

@Component({
template: `
<div ngAccordionGroup>
<button ngAccordionTrigger [panel]="panel1">Trigger 1</button>
<button ngAccordionTrigger [panel]="panel1">Trigger 2</button>
<div ngAccordionPanel #panel1="ngAccordionPanel">
<ng-template ngAccordionContent>Content</ng-template>
</div>
</div>
`,
imports: [AccordionGroup, AccordionTrigger, AccordionPanel, AccordionContent],
changeDetection: ChangeDetectionStrategy.Eager,
})
class AccordionWithDuplicateTriggers {}

@Component({
template: `
<div ngAccordionGroup>
<div ngAccordionPanel #panel1="ngAccordionPanel">
<button ngAccordionTrigger [panel]="panel1">Nested Trigger</button>
<ng-template ngAccordionContent>Content</ng-template>
</div>
</div>
`,
imports: [AccordionGroup, AccordionTrigger, AccordionPanel, AccordionContent],
changeDetection: ChangeDetectionStrategy.Eager,
})
class AccordionWithNestedTrigger {}

@Component({
template: `
<div ngAccordionGroup>
<button ngAccordionTrigger [panel]="panel1">Trigger</button>
<div ngAccordionPanel #panel1="ngAccordionPanel">
Content
</div>
</div>
`,
imports: [AccordionGroup, AccordionTrigger, AccordionPanel],
changeDetection: ChangeDetectionStrategy.Eager,
})
class AccordionPanelWithoutContent {}

@Component({
template: `
<div ngAccordionGroup>
<div ngAccordionPanel>
<ng-template ngAccordionContent>Content</ng-template>
</div>
</div>
`,
imports: [AccordionGroup, AccordionPanel, AccordionContent],
changeDetection: ChangeDetectionStrategy.Eager,
})
class AccordionPanelWithoutTrigger {}

@Component({
template: `
<div ngAccordionGroup [multiExpandable]="false">
<div>
<button ngAccordionTrigger [panel]="panel1" [expanded]="true">Trigger 1</button>
<div ngAccordionPanel #panel1="ngAccordionPanel">
<ng-template ngAccordionContent>Content 1</ng-template>
</div>
</div>
<div>
<button ngAccordionTrigger [panel]="panel2" [expanded]="true">Trigger 2</button>
<div ngAccordionPanel #panel2="ngAccordionPanel">
<ng-template ngAccordionContent>Content 2</ng-template>
</div>
</div>
</div>
`,
imports: [AccordionGroup, AccordionTrigger, AccordionPanel, AccordionContent],
changeDetection: ChangeDetectionStrategy.Eager,
})
class AccordionWithMultipleExpandedItems {}
13 changes: 13 additions & 0 deletions src/aria/tabs/tab-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,19 @@ export class TabList implements OnInit, OnDestroy {
this.selectedTab.set(tab?.value());
},
});

// Check for any violations after the DOM has been updated.
afterRenderEffect({
read: () => {
if (typeof ngDevMode === 'undefined' || ngDevMode) {
const values = this._collection.orderedItems().map(t => t.value());
const duplicates = values.filter((item, index) => values.indexOf(item) !== index);
if (duplicates.length > 0) {
console.error(`Duplicate value '${duplicates[0]}' detected inside ngTabList.`);
}
}
},
});
}

ngOnInit() {
Expand Down
33 changes: 32 additions & 1 deletion src/aria/tabs/tab-panel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ import {
inject,
input,
afterRenderEffect,
contentChild,
OnInit,
OnDestroy,
} from '@angular/core';
import {TabPanelPattern, DeferredContentAware} from '../private';
import {TABS} from './tab-tokens';
import {TabContent} from './tab-content';

/**
* A TabPanel container for the resources of layered content associated with a tab.
Expand Down Expand Up @@ -89,8 +91,37 @@ export class TabPanel implements OnInit, OnDestroy {
tab: this._tabPattern,
});

private readonly _tabContent = contentChild(TabContent);

constructor() {
afterRenderEffect(() => this._deferredContentAware.contentVisible.set(this.visible()));
// Connect the panel's hidden state to the DeferredContentAware's visibility.
afterRenderEffect({
write: () => {
this._deferredContentAware.contentVisible.set(this.visible());
},
});

// Check for any violations after the DOM has been updated.
afterRenderEffect({
read: () => {
if (typeof ngDevMode === 'undefined' || ngDevMode) {
const violations: string[] = [];

if (!this._tabContent()) {
violations.push('ngTabPanel must have an ngTabContent structural directive to render.');
}
if (!this._tabs._tabMap().has(this.value())) {
violations.push(
`ngTabPanel with value '${this.value()}' does not have a corresponding ngTab.`,
);
}

for (const violation of violations) {
console.error(violation);
}
}
},
});
}

ngOnInit() {
Expand Down
Loading
Loading