refactor(aria/accordion): Replace ContentChildren with manual regsitration of triggers

Author: adolgachev

goldens/aria/accordion/index.api.md

@@ -25,11 +25,13 @@ export class AccordionGroup {
     expandAll(): void;
     readonly multiExpandable: _angular_core.InputSignalWithTransform<boolean, unknown>;
     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; }; }, {}, ["_triggers"], never, true, never>;
+    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>;
     // (undocumented)
     static ɵfac: _angular_core.ɵɵFactoryDeclaration<AccordionGroup, never>;
 }
@@ -58,14 +60,17 @@ export class AccordionTrigger implements OnInit {
     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)
     ngOnInit(): void;
     readonly panel: _angular_core.InputSignal<AccordionPanel>;
     readonly panelId: _angular_core.Signal<string>;
     _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; }; "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; }; "index": { "alias": "index"; "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/private/index.api.md

@@ -437,6 +437,12 @@ export class GridRowPattern {
     rowIndex: SignalLike<number | undefined>;
 }
 
+// @public (undocumented)
+export interface HasElement {
+    // (undocumented)
+    element: HTMLElement;
+}
+
 // @public (undocumented)
 export function linkedSignal<T>(sourceFn: () => T): WritableSignalLike<T>;
 
@@ -663,6 +669,9 @@ export function signal<T>(initialValue: T): WritableSignalLike<T>;
 // @public (undocumented)
 export type SignalLike<T> = () => T;
 
+// @public
+export function sortDirectives(a: HasElement, b: HasElement): 1 | -1;
+
 // @public
 export interface TabInputs extends Omit<ListNavigationItem, 'index'>, Omit<ExpansionItem, 'expandable'> {
     tablist: SignalLike<TabListPattern>;

src/aria/accordion/accordion-group.ts

@@ -11,7 +11,6 @@ import {
   ElementRef,
   booleanAttribute,
   computed,
-  contentChildren,
   inject,
   input,
   signal,
@@ -75,10 +74,17 @@ export class AccordionGroup {
   readonly element = this._elementRef.nativeElement as HTMLElement;
 
   /** The AccordionTriggers nested inside this group. */
-  private readonly _triggers = contentChildren(AccordionTrigger, {descendants: true});
+  private readonly _triggers = signal(new Set<AccordionTrigger>());
+
+  /** The AccordionTriggers nested inside this group. */
+  private readonly _sortedTriggers = computed(() => {
+    return [...this._triggers()].sort((a, b) => a.index()! - b.index()!);
+  });
 
   /** The corresponding patterns for the accordion triggers. */
-  private readonly _triggerPatterns = computed(() => this._triggers().map(t => t._pattern));
+  private readonly _triggerPatterns = computed(() => {
+    return this._sortedTriggers().map(t => t._pattern);
+  });
 
   /** The text direction (ltr or rtl). */
   readonly textDirection = inject(Directionality).valueSignal;
@@ -117,4 +123,16 @@ 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-trigger.ts

@@ -9,6 +9,7 @@
 import {
   Directive,
   ElementRef,
+  OnDestroy,
   OnInit,
   booleanAttribute,
   computed,
@@ -53,7 +54,7 @@ import {AccordionPanel} from './accordion-panel';
     '[attr.tabindex]': '_pattern.tabIndex()',
   },
 })
-export class AccordionTrigger implements OnInit {
+export class AccordionTrigger implements OnInit, OnDestroy {
   /** A reference to the trigger element. */
   private readonly _elementRef = inject(ElementRef);
 
@@ -69,12 +70,15 @@ export class AccordionTrigger implements OnInit {
   /** The unique identifier for the trigger. */
   readonly id = input(inject(_IdGenerator).getId('ng-accordion-trigger-', true));
 
-  /** The unique identifier for the correspondingtrigger panel. */
+  /** The unique identifier for the corresponding trigger panel. */
   readonly panelId = computed(() => this.panel().id());
 
   /** 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);
 
@@ -93,6 +97,14 @@ export class AccordionTrigger implements OnInit {
     });
 
     this.panel()._pattern = this._pattern;
+
+    this._accordionGroup._registerTrigger(this);
+  }
+
+  ngOnDestroy() {
+    this.panel()._pattern = undefined;
+
+    this._accordionGroup._unregisterTrigger(this);
   }
 
   /** Expands this item. */

src/aria/accordion/accordion.spec.ts

@@ -10,6 +10,7 @@ import {AccordionGroup} from './accordion-group';
 
 describe('AccordionGroup', () => {
   let fixture: ComponentFixture<AccordionGroupExample>;
+  let testComponent: AccordionGroupExample;
   let triggerDebugElements: DebugElement[];
   let panelDebugElements: DebugElement[];
   let triggerElements: HTMLElement[];
@@ -42,7 +43,7 @@ describe('AccordionGroup', () => {
   }
 
   function configureAccordionComponent(opts: SetupOptions = {}) {
-    const testComponent = fixture.componentInstance as AccordionGroupExample;
+    testComponent = fixture.componentInstance as AccordionGroupExample;
 
     if (opts.multiExpandable !== undefined) {
       testComponent.multiExpandable.set(opts.multiExpandable);
@@ -306,6 +307,16 @@ describe('AccordionGroup', () => {
       expect(isTriggerExpanded(triggerElements[0])).toBeFalse();
     });
 
+    it('should focus on new last trigger with End', () => {
+      const items = testComponent.items().reverse();
+      testComponent.items.set([...items]);
+      fixture.detectChanges();
+
+      // Now reverse, End should move to the former first trigger.
+      endKey(triggerElements[1]);
+      expect(isTriggerActive(triggerElements[0])).toBeTrue();
+    });
+
     describe('wrap behavior', () => {
       it('should wrap to first on ArrowDown from last if wrap=true', () => {
         configureAccordionComponent({wrap: true});
@@ -389,6 +400,7 @@ describe('AccordionGroup', () => {
             ngAccordionTrigger
             [panel]="panel"
             [disabled]="item.disabled"
+            [index]="$index"
             [(expanded)]="item.expanded"
           >{{ item.header }}</button>
           <div