From 845df11752225e29a4a941e1198eadf004c02de4 Mon Sep 17 00:00:00 2001 From: Cheng-Hsuan Tsai Date: Thu, 30 Apr 2026 06:18:16 +0000 Subject: [PATCH] refactor(aria/toolbar): use SortedCollection --- goldens/aria/toolbar/index.api.md | 10 +++---- src/aria/toolbar/BUILD.bazel | 1 + src/aria/toolbar/toolbar-widget.ts | 4 +-- src/aria/toolbar/toolbar.spec.ts | 43 ++++++++++++++++++++++++++++++ src/aria/toolbar/toolbar.ts | 35 +++++++++++------------- 5 files changed, 66 insertions(+), 27 deletions(-) diff --git a/goldens/aria/toolbar/index.api.md b/goldens/aria/toolbar/index.api.md index d7437fc5d51f..81ea71d6883b 100644 --- a/goldens/aria/toolbar/index.api.md +++ b/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 { +export class Toolbar implements OnDestroy { constructor(); + readonly _collection: SortedCollection>; readonly disabled: _angular_core.InputSignalWithTransform; readonly element: HTMLElement; readonly _itemPatterns: _angular_core.Signal[]>; + // (undocumented) + ngOnDestroy(): void; readonly orientation: _angular_core.InputSignal<"vertical" | "horizontal">; readonly _pattern: ToolbarPattern; - // (undocumented) - _register(widget: ToolbarWidget): void; readonly softDisabled: _angular_core.InputSignalWithTransform; readonly textDirection: _angular_core.WritableSignal<_angular_cdk_bidi.Direction>; - // (undocumented) - _unregister(widget: ToolbarWidget): void; readonly value: _angular_core.ModelSignal; readonly wrap: _angular_core.InputSignalWithTransform; // (undocumented) diff --git a/src/aria/toolbar/BUILD.bazel b/src/aria/toolbar/BUILD.bazel index 7516f3def5c0..08d0c64347d6 100644 --- a/src/aria/toolbar/BUILD.bazel +++ b/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", ], ) diff --git a/src/aria/toolbar/toolbar-widget.ts b/src/aria/toolbar/toolbar-widget.ts index d0e41dce5a09..6a327d48794e 100644 --- a/src/aria/toolbar/toolbar-widget.ts +++ b/src/aria/toolbar/toolbar-widget.ts @@ -107,10 +107,10 @@ export class ToolbarWidget implements OnInit, OnDestroy { }); ngOnInit() { - this._toolbar._register(this); + this._toolbar._collection.register(this); } ngOnDestroy() { - this._toolbar._unregister(this); + this._toolbar._collection.unregister(this); } } diff --git a/src/aria/toolbar/toolbar.spec.ts b/src/aria/toolbar/toolbar.spec.ts index 3f6d1994fd4b..67cb124838b1 100644 --- a/src/aria/toolbar/toolbar.spec.ts +++ b/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; + 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: ` +
+ @for (item of items(); track item) { + + } +
+ `, + imports: [Toolbar, ToolbarWidget], + changeDetection: ChangeDetectionStrategy.Eager, +}) +class ShuffledToolbarExample { + items = signal([{value: 'item 0'}, {value: 'item 1'}, {value: 'item 2'}]); +} diff --git a/src/aria/toolbar/toolbar.ts b/src/aria/toolbar/toolbar.ts index 6c3b58d3522f..8bb28c423360 100644 --- a/src/aria/toolbar/toolbar.ts +++ b/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 { +export class Toolbar 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>()); + /** The collection of widgets in the toolbar. */ + readonly _collection = new SortedCollection>(); /** Text direction. */ readonly textDirection = inject(Directionality).valueSignal; /** Sorted UIPatterns of the child widgets */ readonly _itemPatterns = computed[]>(() => - [...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 { constructor() { afterRenderEffect({write: () => this._pattern.setDefaultStateEffect()}); - } - _register(widget: ToolbarWidget) { - 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) { - 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. */