From 177661fb137568da664bb98658c8898b4f134c26 Mon Sep 17 00:00:00 2001 From: Cheng-Hsuan Tsai Date: Mon, 27 Apr 2026 05:24:14 +0000 Subject: [PATCH] refactor(multiple): implement generic child discovery with SortedCollection --- goldens/aria/accordion/index.api.md | 12 +- goldens/aria/listbox/index.api.md | 17 ++- goldens/aria/tabs/index.api.md | 29 ++-- src/aria/accordion/BUILD.bazel | 2 + src/aria/accordion/accordion-group.ts | 53 +++---- src/aria/accordion/accordion-tokens.ts | 6 + src/aria/accordion/accordion-trigger.ts | 12 +- src/aria/accordion/accordion.spec.ts | 77 ++-------- src/aria/listbox/BUILD.bazel | 2 + src/aria/listbox/listbox.spec.ts | 29 ++++ src/aria/listbox/listbox.ts | 39 +++-- src/aria/listbox/option.ts | 26 +++- src/aria/listbox/tokens.ts | 6 + src/aria/private/testing/BUILD.bazel | 10 ++ src/aria/private/testing/test-helpers.ts | 15 ++ src/aria/private/utils/BUILD.bazel | 17 ++- src/aria/private/utils/collection.spec.ts | 142 ++++++++++++++++++ src/aria/private/utils/collection.ts | 65 ++++++++ src/aria/tabs/BUILD.bazel | 2 + src/aria/tabs/tab-list.ts | 75 +++++---- src/aria/tabs/tab-panel.ts | 31 ++-- src/aria/tabs/tab-tokens.ts | 11 ++ src/aria/tabs/tab.ts | 28 ++-- src/aria/tabs/tabs.spec.ts | 32 ++++ src/aria/tabs/tabs.ts | 93 +++++++----- .../accordion-configurable-example.html | 1 - 26 files changed, 586 insertions(+), 246 deletions(-) create mode 100644 src/aria/private/testing/BUILD.bazel create mode 100644 src/aria/private/testing/test-helpers.ts create mode 100644 src/aria/private/utils/collection.spec.ts create mode 100644 src/aria/private/utils/collection.ts diff --git a/goldens/aria/accordion/index.api.md b/goldens/aria/accordion/index.api.md index 3378a901b755..ef3ac71b595e 100644 --- a/goldens/aria/accordion/index.api.md +++ b/goldens/aria/accordion/index.api.md @@ -8,6 +8,7 @@ import * as _angular_cdk_bidi from '@angular/cdk/bidi'; import * as _angular_core from '@angular/core'; import { OnDestroy } from '@angular/core'; import { OnInit } from '@angular/core'; +import { Signal } from '@angular/core'; // @public export class AccordionContent { @@ -18,17 +19,19 @@ export class AccordionContent { } // @public -export class AccordionGroup { +export class AccordionGroup implements OnDestroy { + constructor(); collapseAll(): void; + readonly _collection: SortedCollection; readonly disabled: _angular_core.InputSignalWithTransform; readonly element: HTMLElement; expandAll(): void; readonly multiExpandable: _angular_core.InputSignalWithTransform; + // (undocumented) + ngOnDestroy(): void; readonly _pattern: AccordionGroupPattern; - _registerTrigger(trigger: AccordionTrigger): void; readonly softDisabled: _angular_core.InputSignalWithTransform; readonly textDirection: _angular_core.WritableSignal<_angular_cdk_bidi.Direction>; - _unregisterTrigger(trigger: AccordionTrigger): void; readonly wrap: _angular_core.InputSignalWithTransform; // (undocumented) static ɵdir: _angular_core.ɵɵDirectiveDeclaration; @@ -61,7 +64,6 @@ export class AccordionTrigger implements OnInit, OnDestroy { expand(): void; readonly expanded: _angular_core.ModelSignal; readonly id: _angular_core.InputSignal; - readonly index: _angular_core.InputSignal; // (undocumented) ngOnDestroy(): void; // (undocumented) @@ -71,7 +73,7 @@ export class AccordionTrigger implements OnInit, OnDestroy { _pattern: AccordionTriggerPattern; toggle(): void; // (undocumented) - static ɵdir: _angular_core.ɵɵDirectiveDeclaration; + static ɵdir: _angular_core.ɵɵDirectiveDeclaration; // (undocumented) static ɵfac: _angular_core.ɵɵFactoryDeclaration; } diff --git a/goldens/aria/listbox/index.api.md b/goldens/aria/listbox/index.api.md index 66cd7591a273..5084bb01c7d9 100644 --- a/goldens/aria/listbox/index.api.md +++ b/goldens/aria/listbox/index.api.md @@ -6,17 +6,22 @@ import * as _angular_cdk_bidi from '@angular/cdk/bidi'; import * as _angular_core from '@angular/core'; +import { OnDestroy } from '@angular/core'; +import { OnInit } from '@angular/core'; +import { Signal } from '@angular/core'; // @public -export class Listbox { +export class Listbox implements OnDestroy { constructor(); + readonly _collection: SortedCollection>; readonly disabled: _angular_core.InputSignalWithTransform; readonly element: HTMLElement; readonly focusMode: _angular_core.InputSignal<"roving" | "activedescendant">; gotoFirst(): void; readonly id: _angular_core.InputSignal; - protected readonly items: _angular_core.Signal[]>; readonly multi: _angular_core.InputSignalWithTransform; + // (undocumented) + ngOnDestroy(): void; readonly orientation: _angular_core.InputSignal<"vertical" | "horizontal">; readonly _pattern: ListboxPattern; readonly readonly: _angular_core.InputSignalWithTransform; @@ -29,18 +34,22 @@ export class Listbox { readonly value: _angular_core.ModelSignal; readonly wrap: _angular_core.InputSignalWithTransform; // (undocumented) - static ɵdir: _angular_core.ɵɵDirectiveDeclaration, "[ngListbox]", ["ngListbox"], { "id": { "alias": "id"; "required": false; "isSignal": true; }; "orientation": { "alias": "orientation"; "required": false; "isSignal": true; }; "multi": { "alias": "multi"; "required": false; "isSignal": true; }; "wrap": { "alias": "wrap"; "required": false; "isSignal": true; }; "softDisabled": { "alias": "softDisabled"; "required": false; "isSignal": true; }; "focusMode": { "alias": "focusMode"; "required": false; "isSignal": true; }; "selectionMode": { "alias": "selectionMode"; "required": false; "isSignal": true; }; "typeaheadDelay": { "alias": "typeaheadDelay"; "required": false; "isSignal": true; }; "disabled": { "alias": "disabled"; "required": false; "isSignal": true; }; "readonly": { "alias": "readonly"; "required": false; "isSignal": true; }; "value": { "alias": "value"; "required": false; "isSignal": true; }; }, { "value": "valueChange"; }, ["_options"], never, true, [{ directive: typeof ComboboxPopup; inputs: {}; outputs: {}; }]>; + static ɵdir: _angular_core.ɵɵDirectiveDeclaration, "[ngListbox]", ["ngListbox"], { "id": { "alias": "id"; "required": false; "isSignal": true; }; "orientation": { "alias": "orientation"; "required": false; "isSignal": true; }; "multi": { "alias": "multi"; "required": false; "isSignal": true; }; "wrap": { "alias": "wrap"; "required": false; "isSignal": true; }; "softDisabled": { "alias": "softDisabled"; "required": false; "isSignal": true; }; "focusMode": { "alias": "focusMode"; "required": false; "isSignal": true; }; "selectionMode": { "alias": "selectionMode"; "required": false; "isSignal": true; }; "typeaheadDelay": { "alias": "typeaheadDelay"; "required": false; "isSignal": true; }; "disabled": { "alias": "disabled"; "required": false; "isSignal": true; }; "readonly": { "alias": "readonly"; "required": false; "isSignal": true; }; "value": { "alias": "value"; "required": false; "isSignal": true; }; }, { "value": "valueChange"; }, never, never, true, [{ directive: typeof ComboboxPopup; inputs: {}; outputs: {}; }]>; // (undocumented) static ɵfac: _angular_core.ɵɵFactoryDeclaration, never>; } // @public -class Option_2 { +class Option_2 implements OnInit, OnDestroy { readonly active: _angular_core.Signal; readonly disabled: _angular_core.InputSignalWithTransform; readonly element: HTMLElement; readonly id: _angular_core.InputSignal; readonly label: _angular_core.InputSignal; + // (undocumented) + ngOnDestroy(): void; + // (undocumented) + ngOnInit(): void; readonly _pattern: OptionPattern; readonly selected: _angular_core.Signal; readonly value: _angular_core.InputSignal; diff --git a/goldens/aria/tabs/index.api.md b/goldens/aria/tabs/index.api.md index 9cb495531bf1..66824bceea06 100644 --- a/goldens/aria/tabs/index.api.md +++ b/goldens/aria/tabs/index.api.md @@ -8,6 +8,7 @@ import * as _angular_cdk_bidi from '@angular/cdk/bidi'; import * as _angular_core from '@angular/core'; import { OnDestroy } from '@angular/core'; import { OnInit } from '@angular/core'; +import { Signal } from '@angular/core'; import { WritableSignal } from '@angular/core'; // @public @@ -21,7 +22,6 @@ export class Tab implements HasElement, OnInit, OnDestroy { // (undocumented) ngOnInit(): void; open(): void; - readonly panel: _angular_core.Signal; readonly _pattern: TabPattern; readonly selected: _angular_core.Signal; readonly value: _angular_core.InputSignal; @@ -42,6 +42,7 @@ export class TabContent { // @public export class TabList implements OnInit, OnDestroy { constructor(); + readonly _collection: SortedCollection; readonly disabled: _angular_core.InputSignalWithTransform; readonly element: HTMLElement; // (undocumented) @@ -54,15 +55,12 @@ export class TabList implements OnInit, OnDestroy { open(value: string): boolean; readonly orientation: _angular_core.InputSignal<"vertical" | "horizontal">; readonly _pattern: TabListPattern; - // (undocumented) - _registerTab(child: Tab): void; readonly selectedTab: _angular_core.ModelSignal; readonly selectionMode: _angular_core.InputSignal<"follow" | "explicit">; readonly softDisabled: _angular_core.InputSignalWithTransform; - readonly _sortedTabs: _angular_core.Signal; + readonly _tabPatterns: _angular_core.Signal; + readonly _tabsParent: Tabs; readonly textDirection: WritableSignal<_angular_cdk_bidi.Direction>; - // (undocumented) - _unregisterTab(child: Tab): void; readonly wrap: _angular_core.InputSignalWithTransform; // (undocumented) static ɵdir: _angular_core.ɵɵDirectiveDeclaration; @@ -80,7 +78,6 @@ export class TabPanel implements OnInit, OnDestroy { // (undocumented) ngOnInit(): void; readonly _pattern: TabPanelPattern; - readonly _tabPattern: WritableSignal; readonly value: _angular_core.InputSignal; readonly visible: _angular_core.Signal; // (undocumented) @@ -90,19 +87,21 @@ export class TabPanel implements OnInit, OnDestroy { } // @public -export class Tabs { +export class Tabs implements OnDestroy { constructor(); + readonly _collection: SortedCollection; readonly element: HTMLElement; // (undocumented) - findTabPanel(value?: string): TabPanel | undefined; - // (undocumented) - _registerList(list: TabList): void; - // (undocumented) - _registerPanel(panel: TabPanel): void; + ngOnDestroy(): void; + readonly _panelMap: _angular_core.Signal>; // (undocumented) - _unregisterList(list: TabList): void; + _register(child: TabList): void; + readonly _tabList: _angular_core.WritableSignal; + readonly _tabMap: _angular_core.Signal>; + readonly _tabPanelPatterns: _angular_core.Signal; + readonly _tabPatterns: _angular_core.Signal; // (undocumented) - _unregisterPanel(panel: TabPanel): void; + _unregister(): void; // (undocumented) static ɵdir: _angular_core.ɵɵDirectiveDeclaration; // (undocumented) diff --git a/src/aria/accordion/BUILD.bazel b/src/aria/accordion/BUILD.bazel index 59483aa59b55..033b5bf70188 100644 --- a/src/aria/accordion/BUILD.bazel +++ b/src/aria/accordion/BUILD.bazel @@ -11,6 +11,7 @@ ng_project( deps = [ "//:node_modules/@angular/core", "//src/aria/private", + "//src/aria/private/utils", "//src/cdk/a11y", "//src/cdk/bidi", "//src/cdk/testing", @@ -27,6 +28,7 @@ ng_project( ":accordion", "//:node_modules/@angular/core", "//:node_modules/@angular/platform-browser", + "//src/aria/private/testing", "//src/cdk/testing", "//src/cdk/testing/private", "//src/cdk/testing/testbed", diff --git a/src/aria/accordion/accordion-group.ts b/src/aria/accordion/accordion-group.ts index 487936a1f953..8393019f52e0 100644 --- a/src/aria/accordion/accordion-group.ts +++ b/src/aria/accordion/accordion-group.ts @@ -14,11 +14,14 @@ import { inject, input, signal, + afterNextRender, + OnDestroy, } from '@angular/core'; import {Directionality} from '@angular/cdk/bidi'; -import {AccordionGroupPattern, sortDirectives} from '../private'; +import {AccordionGroupPattern} from '../private'; +import {SortedCollection} from '../private/utils/collection'; +import {ACCORDION_GROUP, ACCORDION_COLLECTION} from './accordion-tokens'; import {AccordionTrigger} from './accordion-trigger'; -import {ACCORDION_GROUP} from './accordion-tokens'; /** * A container for a group of accordion items. It manages the overall state and @@ -64,32 +67,24 @@ import {ACCORDION_GROUP} from './accordion-tokens'; '(click)': '_pattern.onClick($event)', '(focusin)': '_pattern.onFocus($event)', }, - providers: [{provide: ACCORDION_GROUP, useExisting: AccordionGroup}], + providers: [ + {provide: ACCORDION_GROUP, useExisting: AccordionGroup}, + {provide: ACCORDION_COLLECTION, useFactory: () => inject(AccordionGroup)._collection}, + ], }) -export class AccordionGroup { +export class AccordionGroup implements OnDestroy { /** A reference to the group element. */ private readonly _elementRef = inject(ElementRef); /** A reference to the group element. */ readonly element = this._elementRef.nativeElement as HTMLElement; - /** The AccordionTriggers nested inside this group. */ - private readonly _triggers = signal(new Set()); - - /** The AccordionTriggers nested inside this group. */ - private readonly _sortedTriggers = computed(() => { - const triggers = [...this._triggers()] as AccordionTrigger[]; - const sortFn = - triggers[0]?.index() === undefined - ? sortDirectives - : (a: AccordionTrigger, b: AccordionTrigger) => a.index()! - b.index()!; - - return triggers.sort(sortFn); - }); + /** The collection of AccordionTriggers. */ + readonly _collection = new SortedCollection(); /** The corresponding patterns for the accordion triggers. */ private readonly _triggerPatterns = computed(() => { - return this._sortedTriggers().map(t => t._pattern); + return this._collection.orderedItems().map(t => t._pattern); }); /** The text direction (ltr or rtl). */ @@ -119,6 +114,16 @@ export class AccordionGroup { orientation: () => 'vertical', }); + constructor() { + afterNextRender(() => { + this._collection.startObserving(this.element); + }); + } + + ngOnDestroy() { + this._collection.stopObserving(); + } + /** Expands all accordion panels if multi-expandable. */ expandAll() { this._pattern.expandAll(); @@ -128,16 +133,4 @@ export class AccordionGroup { collapseAll() { this._pattern.collapseAll(); } - - /** Internal method to register each trigger as we can not use contentChildren. */ - _registerTrigger(trigger: AccordionTrigger) { - this._triggers().add(trigger); - this._triggers.set(new Set(this._triggers())); - } - - /** Internal method to unregister each trigger as we can not use contentChildren. */ - _unregisterTrigger(trigger: AccordionTrigger) { - this._triggers().delete(trigger); - this._triggers.set(new Set(this._triggers())); - } } diff --git a/src/aria/accordion/accordion-tokens.ts b/src/aria/accordion/accordion-tokens.ts index efe8491e8dc6..a9c3ff7094a7 100644 --- a/src/aria/accordion/accordion-tokens.ts +++ b/src/aria/accordion/accordion-tokens.ts @@ -8,6 +8,12 @@ import {InjectionToken} from '@angular/core'; import type {AccordionGroup} from './accordion-group'; +import {SortedCollection} from '../private/utils/collection'; +import type {AccordionTrigger} from './accordion-trigger'; /** Token used to expose the accordion group. */ export const ACCORDION_GROUP = new InjectionToken('ACCORDION_GROUP'); + +export const ACCORDION_COLLECTION = new InjectionToken>( + 'ACCORDION_COLLECTION', +); diff --git a/src/aria/accordion/accordion-trigger.ts b/src/aria/accordion/accordion-trigger.ts index 0d982bd4f1b7..7c6896223a46 100644 --- a/src/aria/accordion/accordion-trigger.ts +++ b/src/aria/accordion/accordion-trigger.ts @@ -19,7 +19,7 @@ import { } from '@angular/core'; import {_IdGenerator} from '@angular/cdk/a11y'; import {AccordionTriggerPattern} from '../private'; -import {ACCORDION_GROUP} from './accordion-tokens'; +import {ACCORDION_GROUP, ACCORDION_COLLECTION} from './accordion-tokens'; import {AccordionPanel} from './accordion-panel'; /** @@ -64,6 +64,9 @@ export class AccordionTrigger implements OnInit, OnDestroy { /** The parent AccordionGroup. */ private readonly _accordionGroup = inject(ACCORDION_GROUP); + /** The parent collection. */ + private readonly _collection = inject(ACCORDION_COLLECTION); + /** The associated AccordionPanel. */ readonly panel = input.required(); @@ -76,9 +79,6 @@ export class AccordionTrigger implements OnInit, OnDestroy { /** Whether the trigger is disabled. */ readonly disabled = input(false, {transform: booleanAttribute}); - /** The index of the trigger within the accordion group. */ - readonly index = input(); - /** Whether the corresponding panel is expanded. */ readonly expanded = model(false); @@ -98,13 +98,13 @@ export class AccordionTrigger implements OnInit, OnDestroy { this.panel()._pattern = this._pattern; - this._accordionGroup._registerTrigger(this); + this._collection.register(this); } ngOnDestroy() { this.panel()._pattern = undefined; - this._accordionGroup._unregisterTrigger(this); + this._collection.unregister(this); } /** Expands this item. */ diff --git a/src/aria/accordion/accordion.spec.ts b/src/aria/accordion/accordion.spec.ts index 9470e340d9e7..4467bbf28be0 100644 --- a/src/aria/accordion/accordion.spec.ts +++ b/src/aria/accordion/accordion.spec.ts @@ -3,6 +3,7 @@ import {ComponentFixture, TestBed} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; import {provideFakeDirectionality, runAccessibilityChecks} from '@angular/cdk/testing/private'; import {_IdGenerator} from '@angular/cdk/a11y'; +import {waitForMicrotasks} from '../private/testing/test-helpers'; import {AccordionPanel} from './accordion-panel'; import {AccordionTrigger} from './accordion-trigger'; import {AccordionContent} from './accordion-content'; @@ -57,7 +58,6 @@ describe('AccordionGroup', () => { const getTriggerAttribute = (index: number, attribute: string) => triggerElements[index].getAttribute(attribute); - const getTriggerText = (index: number) => triggerElements[index].textContent?.trim(); const getPanelAttribute = (index: number, attribute: string) => panelElements[index].getAttribute(attribute); @@ -292,68 +292,24 @@ describe('AccordionGroup', () => { expect(isTriggerExpanded(0)).toBeFalse(); }); - describe('with shuffled items', () => { - it('should focus on new last trigger with End', () => { - const items = testComponent.items().reverse(); - testComponent.items.set([...items]); - fixture.detectChanges(); - - // Now reversed, End should move to the former first trigger. - endKey(); - expect(isTriggerActive(0)).toBeTrue(); - }); - - it('should focus on newly prepended trigger with Begin', () => { - const items = testComponent.items(); - items.unshift({ - panelId: 'item-0', - header: 'Item 0 Header', - content: 'Item 0 Content', - disabled: signal(false), - expanded: signal(false), - }); - testComponent.items.set([...items]); - setupTriggerAndPanels(); - - homeKey(); - expect(isTriggerActive(0)).toBeTrue(); - expect(getTriggerText(0)).toBe('Item 0 Header'); - }); + it('should update collection order when items are shuffled', async () => { + const groupDebugElement = fixture.debugElement.query(By.directive(AccordionGroup)); + const groupDirective = groupDebugElement.injector.get(AccordionGroup); - it('should focus on newly appended trigger with End', () => { - const items = testComponent.items(); - items.push({ - panelId: 'item-4', - header: 'Item 4 Header', - content: 'Item 4 Content', - disabled: signal(false), - expanded: signal(false), - }); - testComponent.items.set([...items]); - setupTriggerAndPanels(); + let orderedItems = groupDirective._collection.orderedItems(); + expect(orderedItems.length).toBe(3); + expect(orderedItems[0].element.textContent?.trim()).toBe('Item 1 Header'); + expect(orderedItems[2].element.textContent?.trim()).toBe('Item 3 Header'); - endKey(); - expect(isTriggerActive(3)).toBeTrue(); - expect(getTriggerText(3)).toBe('Item 4 Header'); - }); - - it('should focus on inserted trigger with navigation', () => { - const items = testComponent.items(); - items.splice(2, 0, { - panelId: 'item-2a', - header: 'Item 2a Header', - content: 'Item 2a Content', - disabled: signal(false), - expanded: signal(false), - }); - testComponent.items.set([...items]); - setupTriggerAndPanels(); + const items = testComponent.items().reverse(); + testComponent.items.set([...items]); + fixture.detectChanges(); + await waitForMicrotasks(); - downArrowKey(); - downArrowKey(); - expect(isTriggerActive(2)).toBeTrue(); - expect(triggerElements[2].textContent?.trim()).toBe('Item 2a Header'); - }); + orderedItems = groupDirective._collection.orderedItems(); + expect(orderedItems.length).toBe(3); + expect(orderedItems[0].element.textContent?.trim()).toBe('Item 3 Header'); + expect(orderedItems[2].element.textContent?.trim()).toBe('Item 1 Header'); }); describe('wrap behavior', () => { @@ -539,7 +495,6 @@ describe('AccordionGroup', () => {