refactor(multiple): implement generic child discovery with SortedCollection

Author: ok7sai

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<AccordionTrigger>;
     readonly disabled: _angular_core.InputSignalWithTransform<boolean, unknown>;
     readonly element: HTMLElement;
     expandAll(): void;
     readonly multiExpandable: _angular_core.InputSignalWithTransform<boolean, unknown>;
+    // (undocumented)
+    ngOnDestroy(): void;
     readonly _pattern: AccordionGroupPattern;
-    _registerTrigger(trigger: AccordionTrigger): void;
     readonly softDisabled: _angular_core.InputSignalWithTransform<boolean, unknown>;
     readonly textDirection: _angular_core.WritableSignal<_angular_cdk_bidi.Direction>;
-    _unregisterTrigger(trigger: AccordionTrigger): void;
     readonly wrap: _angular_core.InputSignalWithTransform<boolean, unknown>;
     // (undocumented)
     static ɵdir: _angular_core.ɵɵDirectiveDeclaration<AccordionGroup, "[ngAccordionGroup]", ["ngAccordionGroup"], { "disabled": { "alias": "disabled"; "required": false; "isSignal": true; }; "multiExpandable": { "alias": "multiExpandable"; "required": false; "isSignal": true; }; "softDisabled": { "alias": "softDisabled"; "required": false; "isSignal": true; }; "wrap": { "alias": "wrap"; "required": false; "isSignal": true; }; }, {}, never, never, true, never>;
@@ -61,7 +64,6 @@ export class AccordionTrigger implements OnInit, OnDestroy {
     expand(): void;
     readonly expanded: _angular_core.ModelSignal<boolean>;
     readonly id: _angular_core.InputSignal<string>;
-    readonly index: _angular_core.InputSignal<number | undefined>;
     // (undocumented)
     ngOnDestroy(): void;
     // (undocumented)
@@ -71,7 +73,7 @@ export class AccordionTrigger implements OnInit, OnDestroy {
     _pattern: AccordionTriggerPattern;
     toggle(): void;
     // (undocumented)
-    static ɵdir: _angular_core.ɵɵDirectiveDeclaration<AccordionTrigger, "[ngAccordionTrigger]", ["ngAccordionTrigger"], { "panel": { "alias": "panel"; "required": true; "isSignal": true; }; "id": { "alias": "id"; "required": false; "isSignal": true; }; "disabled": { "alias": "disabled"; "required": false; "isSignal": true; }; "index": { "alias": "index"; "required": false; "isSignal": true; }; "expanded": { "alias": "expanded"; "required": false; "isSignal": true; }; }, { "expanded": "expandedChange"; }, never, never, true, never>;
+    static ɵdir: _angular_core.ɵɵDirectiveDeclaration<AccordionTrigger, "[ngAccordionTrigger]", ["ngAccordionTrigger"], { "panel": { "alias": "panel"; "required": true; "isSignal": true; }; "id": { "alias": "id"; "required": false; "isSignal": true; }; "disabled": { "alias": "disabled"; "required": false; "isSignal": true; }; "expanded": { "alias": "expanded"; "required": false; "isSignal": true; }; }, { "expanded": "expandedChange"; }, never, never, true, never>;
     // (undocumented)
     static ɵfac: _angular_core.ɵɵFactoryDeclaration<AccordionTrigger, never>;
 }

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<V> {
+export class Listbox<V> implements OnDestroy {
     constructor();
+    readonly _collection: SortedCollection<Option_2<V>>;
     readonly disabled: _angular_core.InputSignalWithTransform<boolean, unknown>;
     readonly element: HTMLElement;
     readonly focusMode: _angular_core.InputSignal<"roving" | "activedescendant">;
     gotoFirst(): void;
     readonly id: _angular_core.InputSignal<string>;
-    protected readonly items: _angular_core.Signal<OptionPattern<V>[]>;
     readonly multi: _angular_core.InputSignalWithTransform<boolean, unknown>;
+    // (undocumented)
+    ngOnDestroy(): void;
     readonly orientation: _angular_core.InputSignal<"vertical" | "horizontal">;
     readonly _pattern: ListboxPattern<V>;
     readonly readonly: _angular_core.InputSignalWithTransform<boolean, unknown>;
@@ -29,18 +34,22 @@ export class Listbox<V> {
     readonly value: _angular_core.ModelSignal<V[]>;
     readonly wrap: _angular_core.InputSignalWithTransform<boolean, unknown>;
     // (undocumented)
-    static ɵdir: _angular_core.ɵɵDirectiveDeclaration<Listbox<any>, "[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<Listbox<any>, "[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<Listbox<any>, never>;
 }
 
 // @public
-class Option_2<V> {
+class Option_2<V> implements OnInit, OnDestroy {
     readonly active: _angular_core.Signal<boolean>;
     readonly disabled: _angular_core.InputSignalWithTransform<boolean, unknown>;
     readonly element: HTMLElement;
     readonly id: _angular_core.InputSignal<string>;
     readonly label: _angular_core.InputSignal<string | undefined>;
+    // (undocumented)
+    ngOnDestroy(): void;
+    // (undocumented)
+    ngOnInit(): void;
     readonly _pattern: OptionPattern<V>;
     readonly selected: _angular_core.Signal<boolean | undefined>;
     readonly value: _angular_core.InputSignal<V>;

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",

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<AccordionTrigger>());
-
-  /** 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<AccordionTrigger>();
 
   /** 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()));
-  }
 }

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<AccordionGroup>('ACCORDION_GROUP');
+
+export const ACCORDION_COLLECTION = new InjectionToken<SortedCollection<AccordionTrigger>>(
+  'ACCORDION_COLLECTION',
+);

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<AccordionPanel>();
 
@@ -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<number>();
-
   /** Whether the corresponding panel is expanded. */
   readonly expanded = model<boolean>(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. */

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', () => {
         <div>
           <button
             ngAccordionTrigger
-            [index]="$index"
             [panel]="panel"
             [disabled]="item.disabled()"
             [(expanded)]="item.expanded"

src/aria/listbox/BUILD.bazel

@@ -12,6 +12,7 @@ ng_project(
         "//:node_modules/@angular/core",
         "//src/aria/combobox",
         "//src/aria/private",
+        "//src/aria/private/utils",
         "//src/cdk/a11y",
         "//src/cdk/bidi",
         "//src/cdk/testing",
@@ -30,6 +31,7 @@ ng_project(
         "//:node_modules/@angular/core",
         "//:node_modules/@angular/platform-browser",
         "//:node_modules/axe-core",
+        "//src/aria/private/testing",
         "//src/cdk/testing",
         "//src/cdk/testing/private",
         "//src/cdk/testing/testbed",