refactor(aria/menu): use SortedCollection

Author: ok7sai

src/aria/menu/BUILD.bazel

@@ -28,6 +28,7 @@ ng_project(
         "//:node_modules/@angular/core",
         "//:node_modules/@angular/platform-browser",
         "//:node_modules/axe-core",
+        "//src/aria/private/testing",
         "//src/cdk/testing/private",
     ],
 )

src/aria/menu/menu-bar.ts

@@ -7,19 +7,20 @@
  */
 
 import {
+  afterNextRender,
   afterRenderEffect,
   booleanAttribute,
   computed,
-  contentChildren,
   Directive,
   ElementRef,
   inject,
   input,
   model,
+  OnDestroy,
   output,
   signal,
 } from '@angular/core';
-import {SignalLike, MenuBarPattern} from '../private';
+import {SignalLike, MenuBarPattern, SortedCollection} from '../private';
 import {Directionality} from '@angular/cdk/bidi';
 import {MenuItem} from './menu-item';
 import {MENU_COMPONENT} from './menu-tokens';
@@ -69,12 +70,12 @@ import {MENU_COMPONENT} from './menu-tokens';
   },
   providers: [{provide: MENU_COMPONENT, useExisting: MenuBar}],
 })
-export class MenuBar<V> {
-  /** The menu items contained in the menubar. */
-  readonly _allItems = contentChildren<MenuItem<V>>(MenuItem, {descendants: true});
+export class MenuBar<V> implements OnDestroy {
+  /** The collection of menu items. */
+  readonly _collection = new SortedCollection<MenuItem<V>>();
 
   readonly _items: SignalLike<MenuItem<V>[]> = () =>
-    this._allItems().filter(i => i.parent === this);
+    this._collection.orderedItems().filter(i => i.parent === this);
 
   /** A reference to the host element. */
   private readonly _elementRef = inject(ElementRef);
@@ -124,6 +125,14 @@ export class MenuBar<V> {
     });
 
     afterRenderEffect({write: () => this._pattern.setDefaultStateEffect()});
+
+    afterNextRender(() => {
+      this._collection.startObserving(this.element);
+    });
+  }
+
+  ngOnDestroy() {
+    this._collection.stopObserving();
   }
 
   /** Closes the menubar. */

src/aria/menu/menu-item.ts

@@ -6,7 +6,17 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
-import {computed, Directive, effect, ElementRef, inject, input, model} from '@angular/core';
+import {
+  computed,
+  Directive,
+  effect,
+  ElementRef,
+  inject,
+  input,
+  model,
+  OnDestroy,
+  OnInit,
+} from '@angular/core';
 import {MenuItemPattern} from '../private';
 import {_IdGenerator} from '@angular/cdk/a11y';
 import {MENU_COMPONENT} from './menu-tokens';
@@ -46,7 +56,7 @@ import type {MenuBar} from './menu-bar';
     '[attr.aria-controls]': '_pattern.submenu()?.id()',
   },
 })
-export class MenuItem<V> {
+export class MenuItem<V> implements OnInit, OnDestroy {
   /** A reference to the host element. */
   private readonly _elementRef = inject(ElementRef);
 
@@ -95,6 +105,14 @@ export class MenuItem<V> {
     effect(() => this.submenu()?.parent.set(this));
   }
 
+  ngOnInit() {
+    this.parent?._collection.register(this);
+  }
+
+  ngOnDestroy() {
+    this.parent?._collection.unregister(this);
+  }
+
   /** Opens the submenu focusing on the first menu item. */
   open() {
     this._pattern.open({first: true});

src/aria/menu/menu.spec.ts

@@ -1,4 +1,4 @@
-import {Component, DebugElement, ChangeDetectionStrategy} from '@angular/core';
+import {Component, DebugElement, ChangeDetectionStrategy, signal} from '@angular/core';
 import {ComponentFixture, TestBed} from '@angular/core/testing';
 import {By} from '@angular/platform-browser';
 import {provideFakeDirectionality} from '@angular/cdk/testing/private';
@@ -7,6 +7,7 @@ import {MenuBar} from './menu-bar';
 import {MenuContent} from './menu-content';
 import {MenuItem} from './menu-item';
 import {MenuTrigger} from './menu-trigger';
+import {waitForMicrotasks} from '../private/testing/test-helpers';
 
 describe('Standalone Menu Pattern', () => {
   let fixture: ComponentFixture<StandaloneMenuExample>;
@@ -63,6 +64,33 @@ describe('Standalone Menu Pattern', () => {
     return items.find(item => item.textContent?.trim() === text) || null;
   }
 
+  describe('dynamic updates', () => {
+    it('should update item order correctly after items are shuffled', async () => {
+      TestBed.configureTestingModule({imports: [ShuffledMenuExample]});
+      const shuffledFixture = TestBed.createComponent(ShuffledMenuExample);
+      shuffledFixture.detectChanges();
+      const menuDirective = shuffledFixture.debugElement
+        .query(By.directive(Menu))
+        .injector.get(Menu);
+
+      const itemsBefore = menuDirective._pattern.inputs.items();
+      expect(itemsBefore.length).toBe(3);
+      expect(itemsBefore[0].element()?.textContent?.trim()).toBe('Apple');
+
+      // Shuffle items: move first item to the end
+      const items = (shuffledFixture.componentInstance as unknown as ShuffledMenuExample).items();
+      const firstItem = items.shift()!;
+      items.push(firstItem);
+      (shuffledFixture.componentInstance as unknown as ShuffledMenuExample).items.set([...items]);
+      shuffledFixture.detectChanges();
+      await waitForMicrotasks();
+
+      const itemsAfter = menuDirective._pattern.inputs.items();
+      expect(itemsAfter.length).toBe(3);
+      expect(itemsAfter[0].element()?.textContent?.trim()).toBe('Banana');
+    });
+  });
+
   describe('Navigation', () => {
     beforeEach(() => setupMenu());
 
@@ -702,6 +730,37 @@ describe('Menu Bar Pattern', () => {
     return getMenuBarItem(menuBarItemText)?.getAttribute('aria-expanded') === 'true';
   }
 
+  describe('dynamic updates', () => {
+    it('should update item order correctly after items are shuffled', async () => {
+      TestBed.configureTestingModule({imports: [ShuffledMenuBarExample]});
+      const shuffledFixture = TestBed.createComponent(ShuffledMenuBarExample);
+      shuffledFixture.detectChanges();
+      const menuBarDirective = shuffledFixture.debugElement
+        .query(By.directive(MenuBar))
+        .injector.get(MenuBar);
+
+      const itemsBefore = menuBarDirective._pattern.inputs.items();
+      expect(itemsBefore.length).toBe(3);
+      expect(itemsBefore[0].element()?.textContent?.trim()).toBe('File');
+
+      // Shuffle items: move first item to the end
+      const items = (
+        shuffledFixture.componentInstance as unknown as ShuffledMenuBarExample
+      ).items();
+      const firstItem = items.shift()!;
+      items.push(firstItem);
+      (shuffledFixture.componentInstance as unknown as ShuffledMenuBarExample).items.set([
+        ...items,
+      ]);
+      shuffledFixture.detectChanges();
+      await waitForMicrotasks();
+
+      const itemsAfter = menuBarDirective._pattern.inputs.items();
+      expect(itemsAfter.length).toBe(3);
+      expect(itemsAfter[0].element()?.textContent?.trim()).toBe('Edit');
+    });
+  });
+
   describe('Navigation', () => {
     beforeEach(() => setupMenu());
 
@@ -1061,3 +1120,33 @@ class MenuTriggerExample {
   changeDetection: ChangeDetectionStrategy.Eager,
 })
 class MenuBarExample {}
+
+@Component({
+  template: `
+    <div ngMenu>
+      @for (item of items(); track item) {
+        <div ngMenuItem [value]="item.value">{{item.value}}</div>
+      }
+    </div>
+  `,
+  imports: [Menu, MenuItem],
+  changeDetection: ChangeDetectionStrategy.Eager,
+})
+class ShuffledMenuExample {
+  items = signal([{value: 'Apple'}, {value: 'Banana'}, {value: 'Cherry'}]);
+}
+
+@Component({
+  template: `
+    <div ngMenuBar>
+      @for (item of items(); track item) {
+        <div ngMenuItem [value]="item.value">{{item.value}}</div>
+      }
+    </div>
+  `,
+  imports: [MenuBar, MenuItem],
+  changeDetection: ChangeDetectionStrategy.Eager,
+})
+class ShuffledMenuBarExample {
+  items = signal([{value: 'File'}, {value: 'Edit'}, {value: 'View'}]);
+}

src/aria/menu/menu.ts

@@ -7,20 +7,21 @@
  */
 
 import {
+  afterNextRender,
   afterRenderEffect,
   booleanAttribute,
   computed,
-  contentChildren,
   Directive,
   ElementRef,
   inject,
   input,
+  OnDestroy,
   output,
   Signal,
   signal,
   untracked,
 } from '@angular/core';
-import {MenuPattern, DeferredContentAware} from '../private';
+import {MenuPattern, DeferredContentAware, SortedCollection} from '../private';
 import {_IdGenerator} from '@angular/cdk/a11y';
 import {Directionality} from '@angular/cdk/bidi';
 import {MenuTrigger} from './menu-trigger';
@@ -79,16 +80,16 @@ import {MENU_COMPONENT} from './menu-tokens';
   ],
   providers: [{provide: MENU_COMPONENT, useExisting: Menu}],
 })
-export class Menu<V> {
+export class Menu<V> implements OnDestroy {
   /** The DeferredContentAware host directive. */
   private readonly _deferredContentAware = inject(DeferredContentAware, {optional: true});
 
-  /** The menu items contained in the menu. */
-  readonly _allItems = contentChildren<MenuItem<V>>(MenuItem, {descendants: true});
+  /** The collection of menu items. */
+  readonly _collection = new SortedCollection<MenuItem<V>>();
 
   /** The menu items that are direct children of this menu. */
   readonly _items: Signal<MenuItem<V>[]> = computed(() =>
-    this._allItems().filter(i => i.parent === this),
+    this._collection.orderedItems().filter(i => i.parent === this),
   );
 
   /** A reference to the host element. */
@@ -188,6 +189,14 @@ export class Menu<V> {
     });
 
     afterRenderEffect({write: () => this._pattern.setDefaultStateEffect()});
+
+    afterNextRender(() => {
+      this._collection.startObserving(this.element);
+    });
+  }
+
+  ngOnDestroy() {
+    this._collection.stopObserving();
   }
 
   /** Closes the menu. */