refactor(aria/toolbar): use SortedCollection

Author: ok7sai

goldens/aria/toolbar/index.api.md

@@ -8,21 +8,21 @@ 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 Toolbar<V> {
+export class Toolbar<V> implements OnDestroy {
     constructor();
+    readonly _collection: SortedCollection<ToolbarWidget<V>>;
     readonly disabled: _angular_core.InputSignalWithTransform<boolean, unknown>;
     readonly element: HTMLElement;
     readonly _itemPatterns: _angular_core.Signal<ToolbarWidgetPattern<V>[]>;
+    // (undocumented)
+    ngOnDestroy(): void;
     readonly orientation: _angular_core.InputSignal<"vertical" | "horizontal">;
     readonly _pattern: ToolbarPattern<V>;
-    // (undocumented)
-    _register(widget: ToolbarWidget<V>): void;
     readonly softDisabled: _angular_core.InputSignalWithTransform<boolean, unknown>;
     readonly textDirection: _angular_core.WritableSignal<_angular_cdk_bidi.Direction>;
-    // (undocumented)
-    _unregister(widget: ToolbarWidget<V>): void;
     readonly value: _angular_core.ModelSignal<V[]>;
     readonly wrap: _angular_core.InputSignalWithTransform<boolean, unknown>;
     // (undocumented)

src/aria/toolbar/BUILD.bazel

@@ -26,6 +26,7 @@ ts_project(
         ":toolbar",
         "//:node_modules/@angular/core",
         "//:node_modules/@angular/platform-browser",
+        "//src/aria/private/testing",
         "//src/cdk/testing/private",
     ],
 )

src/aria/toolbar/toolbar-widget.ts

@@ -107,10 +107,10 @@ export class ToolbarWidget<V> implements OnInit, OnDestroy {
   });
 
   ngOnInit() {
-    this._toolbar._register(this);
+    this._toolbar._collection.register(this);
   }
 
   ngOnDestroy() {
-    this._toolbar._unregister(this);
+    this._toolbar._collection.unregister(this);
   }
 }

src/aria/toolbar/toolbar.spec.ts

@@ -8,6 +8,7 @@ import {
 } from '@angular/core';
 import {ComponentFixture, TestBed} from '@angular/core/testing';
 import {By} from '@angular/platform-browser';
+import {waitForMicrotasks} from '../private/testing/test-helpers';
 import {provideFakeDirectionality, runAccessibilityChecks} from '@angular/cdk/testing/private';
 import {Toolbar} from './toolbar';
 import {ToolbarWidgetGroup} from './toolbar-widget-group';
@@ -98,6 +99,33 @@ describe('Toolbar', () => {
 
   afterEach(async () => await runAccessibilityChecks(fixture.nativeElement));
 
+  describe('dynamic updates', () => {
+    it('should update widget order correctly after widgets are shuffled', async () => {
+      TestBed.configureTestingModule({imports: [ShuffledToolbarExample]});
+      fixture = TestBed.createComponent(
+        ShuffledToolbarExample,
+      ) as unknown as ComponentFixture<ToolbarExample>;
+      fixture.detectChanges();
+      const shuffledToolbarDebugEl = fixture.debugElement.query(By.directive(Toolbar));
+      const shuffledToolbarInstance = shuffledToolbarDebugEl.injector.get(Toolbar);
+
+      const widgetsBefore = shuffledToolbarInstance._itemPatterns();
+      expect(widgetsBefore.length).toBe(3);
+      expect(widgetsBefore[0].element()?.textContent?.trim()).toBe('item 0');
+
+      const items = (fixture.componentInstance as unknown as ShuffledToolbarExample).items();
+      const firstItem = items.shift()!;
+      items.push(firstItem);
+      (fixture.componentInstance as unknown as ShuffledToolbarExample).items.set([...items]);
+      fixture.detectChanges();
+      await waitForMicrotasks();
+
+      const widgetsAfter = shuffledToolbarInstance._itemPatterns();
+      expect(widgetsAfter.length).toBe(3);
+      expect(widgetsAfter[0].element()?.textContent?.trim()).toBe('item 1');
+    });
+  });
+
   describe('Navigation', () => {
     describe('with horizontal orientation', () => {
       it('should navigate on click (horizontal)', () => {
@@ -724,3 +752,18 @@ export class SimpleToolbarButton {
   changeDetection: ChangeDetectionStrategy.Eager,
 })
 class WrappedToolbarExample {}
+
+@Component({
+  template: `
+    <div ngToolbar>
+      @for (item of items(); track item) {
+        <button ngToolbarWidget [value]="item.value">{{item.value}}</button>
+      }
+    </div>
+  `,
+  imports: [Toolbar, ToolbarWidget],
+  changeDetection: ChangeDetectionStrategy.Eager,
+})
+class ShuffledToolbarExample {
+  items = signal([{value: 'item 0'}, {value: 'item 1'}, {value: 'item 2'}]);
+}

src/aria/toolbar/toolbar.ts

@@ -7,17 +7,19 @@
  */
 
 import {
+  afterNextRender,
   afterRenderEffect,
+  booleanAttribute,
+  computed,
   Directive,
   ElementRef,
   inject,
-  computed,
   input,
-  booleanAttribute,
-  signal,
   model,
+  OnDestroy,
+  signal,
 } from '@angular/core';
-import {ToolbarPattern, ToolbarWidgetPattern, sortDirectives} from '../private';
+import {ToolbarPattern, ToolbarWidgetPattern, SortedCollection} from '../private';
 import {Directionality} from '@angular/cdk/bidi';
 import type {ToolbarWidget} from './toolbar-widget';
 
@@ -57,22 +59,22 @@ import type {ToolbarWidget} from './toolbar-widget';
     '(focusin)': '_pattern.onFocusIn()',
   },
 })
-export class Toolbar<V> {
+export class Toolbar<V> implements OnDestroy {
   /** A reference to the host element. */
   private readonly _elementRef = inject(ElementRef);
 
   /** A reference to the host element. */
   readonly element = this._elementRef.nativeElement as HTMLElement;
 
-  /** The TabList nested inside of the container. */
-  private readonly _widgets = signal(new Set<ToolbarWidget<V>>());
+  /** The collection of widgets in the toolbar. */
+  readonly _collection = new SortedCollection<ToolbarWidget<V>>();
 
   /** Text direction. */
   readonly textDirection = inject(Directionality).valueSignal;
 
   /** Sorted UIPatterns of the child widgets */
   readonly _itemPatterns = computed<ToolbarWidgetPattern<V>[]>(() =>
-    [...this._widgets()].sort(sortDirectives).map(widget => widget._pattern),
+    this._collection.orderedItems().map(widget => widget._pattern),
   );
 
   /** Whether the toolbar is vertically or horizontally oriented. */
@@ -106,21 +108,14 @@ export class Toolbar<V> {
 
   constructor() {
     afterRenderEffect({write: () => this._pattern.setDefaultStateEffect()});
-  }
 
-  _register(widget: ToolbarWidget<V>) {
-    const widgets = this._widgets();
-    if (!widgets.has(widget)) {
-      widgets.add(widget);
-      this._widgets.set(new Set(widgets));
-    }
+    afterNextRender(() => {
+      this._collection.startObserving(this.element);
+    });
   }
 
-  _unregister(widget: ToolbarWidget<V>) {
-    const widgets = this._widgets();
-    if (widgets.delete(widget)) {
-      this._widgets.set(new Set(widgets));
-    }
+  ngOnDestroy() {
+    this._collection.stopObserving();
   }
 
   /** Finds the toolbar item associated with a given element. */