fixup! feat(cdk-experimental/ui-patterns): listbox ui pattern

Author: wagnermaciel

src/cdk-experimental/listbox/BUILD.bazel

@@ -10,6 +10,7 @@ ng_module(
     ),
     deps = [
         "//src/cdk-experimental/ui-patterns",
+        "//src/cdk/a11y",
         "//src/cdk/bidi",
     ],
 )

src/cdk-experimental/listbox/listbox.ts

@@ -19,6 +19,7 @@ import {
 import {ListboxPattern, OptionPattern} from '@angular/cdk-experimental/ui-patterns';
 import {Directionality} from '@angular/cdk/bidi';
 import {toSignal} from '@angular/core/rxjs-interop';
+import {_IdGenerator} from '@angular/cdk/a11y';
 
 /**
  * A listbox container.
@@ -51,10 +52,10 @@ import {toSignal} from '@angular/core/rxjs-interop';
 })
 export class CdkListbox {
   /** The directionality (LTR / RTL) context for the application (or a subtree of it). */
-  private _directionality = inject(Directionality);
+  private readonly _directionality = inject(Directionality);
 
   /** The CdkOptions nested inside of the CdkListbox. */
-  private _cdkOptions = contentChildren(CdkOption, {descendants: true});
+  private readonly _cdkOptions = contentChildren(CdkOption, {descendants: true});
 
   /** A signal wrapper for directionality. */
   protected textDirection = toSignal(this._directionality.change, {
@@ -103,9 +104,6 @@ export class CdkListbox {
   });
 }
 
-// TODO(wagnermaciel): Figure out how we want to generate IDs.
-let count = 0;
-
 /** A selectable option in a CdkListbox. */
 @Directive({
   selector: '[cdkOption]',
@@ -120,14 +118,16 @@ let count = 0;
 })
 export class CdkOption {
   /** A reference to the option element. */
-  private _elementRef = inject(ElementRef);
+  private readonly _elementRef = inject(ElementRef);
 
   /** The parent CdkListbox. */
-  private _cdkListbox = inject(CdkListbox);
+  private readonly _cdkListbox = inject(CdkListbox);
+
+  /** A unique identifier for the option. */
+  private readonly _generatedId = inject(_IdGenerator).getId('cdk-option-');
 
-  // TODO(wagnermaciel): Figure out how we want to generate IDs.
   /** A unique identifier for the option. */
-  protected id = computed(() => `${count++}`);
+  protected id = computed(() => this._generatedId);
 
   // TODO(wagnermaciel): See if we want to change how we handle this since textContent is not
   // reactive. See https://github.com/angular/components/pull/30495#discussion_r1961260216.

src/cdk-experimental/ui-patterns/behaviors/list-focus/list-focus.spec.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
-import {Signal, signal} from '@angular/core';
+import {computed, Signal, signal} from '@angular/core';
 import {ListNavigation, ListNavigationInputs} from '../list-navigation/list-navigation';
 import {ListFocus, ListFocusInputs, ListFocusItem} from './list-focus';
 
@@ -58,7 +58,7 @@ describe('List Focus', () => {
       const items = getItems(5);
       const nav = getNavigation(items);
       const focus = getFocus(nav);
-      const tabindex = focus.getListTabindex();
+      const tabindex = computed(() => focus.getListTabindex());
       expect(tabindex()).toBe(-1);
     });
 
@@ -75,7 +75,7 @@ describe('List Focus', () => {
       const focus = getFocus(nav);
 
       items().forEach(i => {
-        i.tabindex = focus.getItemTabindex(i);
+        i.tabindex = computed(() => focus.getItemTabindex(i));
       });
 
       expect(items()[0].tabindex()).toBe(0);
@@ -91,7 +91,7 @@ describe('List Focus', () => {
       const focus = getFocus(nav);
 
       items().forEach(i => {
-        i.tabindex = focus.getItemTabindex(i);
+        i.tabindex = computed(() => focus.getItemTabindex(i));
       });
 
       nav.next();
@@ -111,7 +111,7 @@ describe('List Focus', () => {
       const focus = getFocus(nav, {
         focusMode: signal('activedescendant'),
       });
-      const tabindex = focus.getListTabindex();
+      const tabindex = computed(() => focus.getListTabindex());
       expect(tabindex()).toBe(0);
     });
 
@@ -132,7 +132,7 @@ describe('List Focus', () => {
       });
 
       items().forEach(i => {
-        i.tabindex = focus.getItemTabindex(i);
+        i.tabindex = computed(() => focus.getItemTabindex(i));
       });
 
       expect(items()[0].tabindex()).toBe(-1);

src/cdk-experimental/ui-patterns/behaviors/list-focus/list-focus.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
-import {computed, Signal} from '@angular/core';
+import {Signal} from '@angular/core';
 import {ListNavigation, ListNavigationItem} from '../list-navigation/list-navigation';
 
 /** Represents an item in a collection, such as a listbox option, than may receive focus. */
@@ -29,32 +29,30 @@ export class ListFocus<T extends ListFocusItem> {
   /** The navigation controller of the parent list. */
   navigation: ListNavigation<ListFocusItem>;
 
+  constructor(readonly inputs: ListFocusInputs<T> & {navigation: ListNavigation<T>}) {
+    this.navigation = inputs.navigation;
+  }
+
   /** The id of the current active item. */
-  getActiveDescendant = computed<string | undefined>(() => {
+  getActiveDescendant(): String | undefined {
     if (this.inputs.focusMode() === 'roving') {
       return undefined;
     }
     return this.navigation.inputs.items()[this.navigation.inputs.activeIndex()].id();
-  });
-
-  constructor(readonly inputs: ListFocusInputs<T> & {navigation: ListNavigation<T>}) {
-    this.navigation = inputs.navigation;
   }
 
-  /** Returns a signal that keeps track of the tabindex for the list. */
-  getListTabindex(): Signal<-1 | 0> {
-    return computed(() => (this.inputs.focusMode() === 'activedescendant' ? 0 : -1));
+  /** The tabindex for the list. */
+  getListTabindex(): -1 | 0 {
+    return this.inputs.focusMode() === 'activedescendant' ? 0 : -1;
   }
 
-  /** Returns a signal that keeps track of the tabindex for the given item. */
-  getItemTabindex(item: T): Signal<-1 | 0> {
-    return computed(() => {
-      if (this.inputs.focusMode() === 'activedescendant') {
-        return -1;
-      }
-      const index = this.navigation.inputs.items().indexOf(item);
-      return this.navigation.inputs.activeIndex() === index ? 0 : -1;
-    });
+  /** Returns the tabindex for the given item. */
+  getItemTabindex(item: T): -1 | 0 {
+    if (this.inputs.focusMode() === 'activedescendant') {
+      return -1;
+    }
+    const index = this.navigation.inputs.items().indexOf(item);
+    return this.navigation.inputs.activeIndex() === index ? 0 : -1;
   }
 
   /** Focuses the current active item. */

src/cdk-experimental/ui-patterns/behaviors/list-navigation/list-navigation.ts

@@ -56,43 +56,30 @@ export class ListNavigation<T extends ListNavigationItem> {
   /** Navigates to the next item in the list. */
   next() {
     const items = this.inputs.items();
+    const itemCount = items.length;
+    const startIndex = this.inputs.activeIndex();
+    const step = (i: number) => this._stepIndex(i, 1);
 
-    for (let i = this.inputs.activeIndex() + 1; i < items.length; i++) {
+    for (let i = step(startIndex); i !== startIndex && i < itemCount; step(i)) {
       if (this.isFocusable(items[i])) {
         this.goto(items[i]);
         return;
       }
     }
-
-    if (this.inputs.wrap()) {
-      for (let i = 0; i <= this.inputs.activeIndex(); i++) {
-        if (this.isFocusable(items[i])) {
-          this.goto(items[i]);
-          return;
-        }
-      }
-    }
   }
 
   /** Navigates to the previous item in the list. */
   prev() {
     const items = this.inputs.items();
+    const startIndex = this.inputs.activeIndex();
+    const step = (i: number) => this._stepIndex(i, -1);
 
-    for (let i = this.inputs.activeIndex() - 1; i >= 0; i--) {
+    for (let i = step(startIndex); i !== startIndex && i >= 0; step(i)) {
       if (this.isFocusable(items[i])) {
         this.goto(items[i]);
         return;
       }
     }
-
-    if (this.inputs.wrap()) {
-      for (let i = items.length - 1; i >= this.inputs.activeIndex(); i--) {
-        if (this.isFocusable(items[i])) {
-          this.goto(items[i]);
-          return;
-        }
-      }
-    }
   }
 
   /** Navigates to the first item in the list. */
@@ -119,4 +106,10 @@ export class ListNavigation<T extends ListNavigationItem> {
   isFocusable(item: T): boolean {
     return !item.disabled() || !this.inputs.skipDisabled();
   }
+
+  private _stepIndex(index: number, step: -1 | 1) {
+    return this.inputs.wrap()
+      ? (index + step + this.inputs.items().length) % this.inputs.items().length
+      : index + step;
+  }
 }

src/cdk-experimental/ui-patterns/behaviors/list-selection/list-selection.spec.ts

@@ -193,7 +193,7 @@ describe('List Selection', () => {
       selection.select(); // [0]
       nav.next();
       nav.next();
-      selection.selectFromLastSelectedItem(); // [0, 1, 2]
+      selection.selectFromPrevSelectedItem(); // [0, 1, 2]
 
       expect(selection.inputs.selectedIds()).toEqual(['0', '1', '2']);
     });
@@ -208,7 +208,7 @@ describe('List Selection', () => {
       selection.select(); // [3]
       nav.prev();
       nav.prev();
-      selection.selectFromLastSelectedItem(); // [3, 1, 2]
+      selection.selectFromPrevSelectedItem(); // [3, 1, 2]
 
       expect(selection.inputs.selectedIds()).toEqual(['3', '1', '2']);
     });

src/cdk-experimental/ui-patterns/behaviors/list-selection/list-selection.ts

@@ -35,8 +35,8 @@ export interface ListSelectionInputs<T extends ListSelectionItem> {
 
 /** Controls selection for a list of items. */
 export class ListSelection<T extends ListSelectionItem> {
-  /** The id of the last selected item. */
-  lastSelectedId = signal<string | undefined>(undefined);
+  /** The id of the most recently selected item. */
+  previousSelectedId = signal<string | undefined>(undefined);
 
   /** The navigation controller of the parent list. */
   navigation: ListNavigation<T>;
@@ -104,9 +104,9 @@ export class ListSelection<T extends ListSelectionItem> {
   }
 
   /** Selects the items in the list starting at the last selected item. */
-  selectFromLastSelectedItem() {
-    const lastSelectedId = this.inputs.items().findIndex(i => this.lastSelectedId() === i.id());
-    this._selectFromIndex(lastSelectedId);
+  selectFromPrevSelectedItem() {
+    const prevSelectedId = this.inputs.items().findIndex(i => this.previousSelectedId() === i.id());
+    this._selectFromIndex(prevSelectedId);
   }
 
   /** Selects the items in the list starting at the last active item. */
@@ -137,6 +137,6 @@ export class ListSelection<T extends ListSelectionItem> {
   /** Sets the anchor to the current active index. */
   private _anchor() {
     const item = this.inputs.items()[this.inputs.navigation.inputs.activeIndex()];
-    this.lastSelectedId.set(item.id());
+    this.previousSelectedId.set(item.id());
   }
 }

src/cdk-experimental/ui-patterns/listbox/listbox.ts

@@ -56,10 +56,10 @@ export class ListboxPattern {
   disabled: Signal<boolean>;
 
   /** The tabindex of the listbox. */
-  tabindex: Signal<-1 | 0>;
+  tabindex = computed(() => this.focusManager.getListTabindex());
 
   /** The id of the current active item. */
-  activedescendant: Signal<string | undefined>;
+  activedescendant = computed(() => this.focusManager.getActiveDescendant());
 
   /** Whether multiple items in the list can be selected at once. */
   multiselectable: Signal<boolean>;
@@ -167,9 +167,6 @@ export class ListboxPattern {
     this.selection = new ListSelection({...inputs, navigation: this.navigation});
     this.typeahead = new ListTypeahead({...inputs, navigation: this.navigation});
     this.focusManager = new ListFocus({...inputs, navigation: this.navigation});
-
-    this.tabindex = this.focusManager.getListTabindex();
-    this.activedescendant = this.focusManager.getActiveDescendant;
   }
 
   /** Handles keydown events for the listbox. */
@@ -249,7 +246,7 @@ export class ListboxPattern {
       this.selection.selectAll();
     }
     if (opts?.selectFromAnchor) {
-      this.selection.selectFromLastSelectedItem();
+      this.selection.selectFromPrevSelectedItem();
     }
     if (opts?.selectFromActive) {
       this.selection.selectFromActive();

src/cdk-experimental/ui-patterns/listbox/option.ts

@@ -57,7 +57,7 @@ export class OptionPattern {
   listbox: Signal<ListboxPattern>;
 
   /** The tabindex of the option. */
-  tabindex: Signal<-1 | 0>;
+  tabindex = computed(() => this.listbox().focusManager.getItemTabindex(this));
 
   /** The html element that should receive focus. */
   element: Signal<HTMLElement>;
@@ -68,6 +68,5 @@ export class OptionPattern {
     this.element = args.element;
     this.disabled = args.disabled;
     this.searchTerm = args.searchTerm;
-    this.tabindex = this.listbox().focusManager.getItemTabindex(this);
   }
 }