Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 9 additions & 15 deletions goldens/aria/private/index.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ export class ComboboxTreePattern<V> extends TreePattern<V> implements ComboboxTr
setValue: (value: V | undefined) => void;
tabIndex: SignalLike<-1 | 0>;
toggle: (item?: TreeItemPattern<V>) => void;
toggleExpansion: (item?: TreeItemPattern<V>) => void;
unfocus: () => void;
}

Expand Down Expand Up @@ -834,23 +835,21 @@ export class ToolbarWidgetPattern<V> implements ListItem<V> {
}

// @public
export interface TreeInputs<V> extends Omit<ListInputs<TreeItemPattern<V>, V>, 'items'> {
allItems: SignalLike<TreeItemPattern<V>[]>;
export interface TreeInputs<V> extends Omit<TreeInputs$1<TreeItemPattern<V>, V>, 'multiExpandable'> {
currentType: SignalLike<'page' | 'step' | 'location' | 'date' | 'time' | 'true' | 'false'>;
id: SignalLike<string>;
nav: SignalLike<boolean>;
}

// @public
export interface TreeItemInputs<V> extends Omit<ListItem<V>, 'index'>, Omit<ExpansionItem, 'expandable'> {
children: SignalLike<TreeItemPattern<V>[]>;
export interface TreeItemInputs<V> extends Omit<TreeItem<V, TreeItemPattern<V>>, 'index' | 'parent' | 'visible' | 'expandable'> {
hasChildren: SignalLike<boolean>;
parent: SignalLike<TreeItemPattern<V> | TreePattern<V>>;
tree: SignalLike<TreePattern<V>>;
}

// @public
export class TreeItemPattern<V> implements ListItem<V>, ExpansionItem {
export class TreeItemPattern<V> implements TreeItem<V, TreeItemPattern<V>> {
constructor(inputs: TreeItemInputs<V>);
readonly active: SignalLike<boolean>;
readonly children: SignalLike<TreeItemPattern<V>[]>;
Expand All @@ -859,13 +858,12 @@ export class TreeItemPattern<V> implements ListItem<V>, ExpansionItem {
readonly element: SignalLike<HTMLElement>;
readonly expandable: SignalLike<boolean>;
readonly expanded: WritableSignalLike<boolean>;
readonly expansionBehavior: ListExpansion;
readonly id: SignalLike<string>;
readonly index: SignalLike<number>;
// (undocumented)
readonly inputs: TreeItemInputs<V>;
readonly level: SignalLike<number>;
readonly parent: SignalLike<TreeItemPattern<V> | TreePattern<V>>;
readonly parent: SignalLike<TreeItemPattern<V> | undefined>;
readonly posinset: SignalLike<number>;
readonly searchTerm: SignalLike<string>;
readonly selectable: SignalLike<boolean>;
Expand All @@ -882,19 +880,16 @@ export class TreePattern<V> implements TreeInputs<V> {
constructor(inputs: TreeInputs<V>);
readonly activeDescendant: SignalLike<string | undefined>;
readonly activeItem: WritableSignalLike<TreeItemPattern<V> | undefined>;
readonly allItems: SignalLike<TreeItemPattern<V>[]>;
readonly children: SignalLike<TreeItemPattern<V>[]>;
collapse(opts?: SelectOptions): void;
readonly collapseKey: SignalLike<"ArrowUp" | "ArrowRight" | "ArrowLeft">;
_collapseOrParent(opts?: SelectOptions): void;
readonly currentType: SignalLike<'page' | 'step' | 'location' | 'date' | 'time' | 'true' | 'false'>;
readonly disabled: SignalLike<boolean>;
readonly dynamicSpaceKey: SignalLike<"" | " ">;
readonly element: SignalLike<HTMLElement>;
expand(opts?: SelectOptions): void;
readonly expanded: () => boolean;
readonly expandKey: SignalLike<"ArrowRight" | "ArrowLeft" | "ArrowDown">;
expandSiblings(item?: TreeItemPattern<V>): void;
readonly expansionBehavior: ListExpansion;
_expandOrFirstChild(opts?: SelectOptions): void;
readonly focusMode: SignalLike<'roving' | 'activedescendant'>;
readonly followFocus: SignalLike<boolean>;
protected _getItem(event: Event): TreeItemPattern<V> | undefined;
Expand All @@ -903,9 +898,9 @@ export class TreePattern<V> implements TreeInputs<V> {
// (undocumented)
readonly inputs: TreeInputs<V>;
readonly isRtl: SignalLike<boolean>;
readonly items: SignalLike<TreeItemPattern<V>[]>;
readonly keydown: SignalLike<KeyboardEventManager<KeyboardEvent>>;
readonly level: () => number;
readonly listBehavior: List<TreeItemPattern<V>, V>;
readonly multi: SignalLike<boolean>;
readonly nav: SignalLike<boolean>;
readonly nextKey: SignalLike<"ArrowRight" | "ArrowLeft" | "ArrowDown">;
Expand All @@ -919,12 +914,11 @@ export class TreePattern<V> implements TreeInputs<V> {
readonly softDisabled: SignalLike<boolean>;
readonly tabIndex: SignalLike<-1 | 0>;
readonly textDirection: SignalLike<'ltr' | 'rtl'>;
toggleExpansion(item?: TreeItemPattern<V>): void;
readonly treeBehavior: Tree<TreeItemPattern<V>, V>;
readonly typeaheadDelay: SignalLike<number>;
readonly typeaheadRegexp: RegExp;
readonly values: WritableSignalLike<V[]>;
readonly visible: () => boolean;
readonly visibleItems: SignalLike<TreeItemPattern<V>[]>;
readonly wrap: SignalLike<boolean>;
}

Expand Down
7 changes: 7 additions & 0 deletions src/aria/combobox/combobox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,13 @@ describe('Combobox', () => {

it('should select and commit on click', () => {
click(inputElement);

// Iterate to the parent node and expand it so the child is visible
down(); // Winter
down(); // Spring
right(); // Expand Spring
fixture.detectChanges();

const item = getTreeItem('April')!;
click(item);
fixture.detectChanges();
Expand Down
1 change: 1 addition & 0 deletions src/aria/private/behaviors/list-focus/list-focus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export interface ListFocusInputs<T extends ListFocusItem> {
/** Whether disabled items in the list should be focusable. */
softDisabled: SignalLike<boolean>;

/** The html element that should receive focus. */
element: SignalLike<HTMLElement | undefined>;
}

Expand Down
51 changes: 51 additions & 0 deletions src/aria/private/behaviors/list-navigation/list-navigation.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,4 +254,55 @@ describe('List Navigation', () => {
expect(nav.inputs.activeItem()).toBe(nav.inputs.items()[4]);
});
});

describe('with items subset', () => {
it('should navigate only within the provided subset for next/prev', () => {
const nav = getNavigation();
const allItems = nav.inputs.items();
const subset = [allItems[0], allItems[2], allItems[4]];

// Start at 0
expect(nav.inputs.activeItem()).toBe(allItems[0]);

// next(subset) -> 2 (skip 1)
nav.next({focusElement: false, items: subset});
expect(nav.inputs.activeItem()).toBe(allItems[2]);

// next(subset) -> 4 (skip 3)
nav.next({focusElement: false, items: subset});
expect(nav.inputs.activeItem()).toBe(allItems[4]);

// prev(subset) -> 2 (skip 3)
nav.prev({focusElement: false, items: subset});
expect(nav.inputs.activeItem()).toBe(allItems[2]);
});

it('should wrap within the subset', () => {
const nav = getNavigation({wrap: signal(true)});
const allItems = nav.inputs.items();
const subset = [allItems[0], allItems[2], allItems[4]];

nav.goto(allItems[4]);

// next(subset) -> 0 (wrap)
nav.next({focusElement: false, items: subset});
expect(nav.inputs.activeItem()).toBe(allItems[0]);

// prev(subset) -> 4 (wrap)
nav.prev({focusElement: false, items: subset});
expect(nav.inputs.activeItem()).toBe(allItems[4]);
});

it('should find first/last within the subset', () => {
const nav = getNavigation();
const allItems = nav.inputs.items();
const subset = [allItems[1], allItems[2], allItems[3]];

nav.first({focusElement: false, items: subset});
expect(nav.inputs.activeItem()).toBe(allItems[1]);

nav.last({focusElement: false, items: subset});
expect(nav.inputs.activeItem()).toBe(allItems[3]);
});
});
});
58 changes: 40 additions & 18 deletions src/aria/private/behaviors/list-navigation/list-navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,54 +24,71 @@ export interface ListNavigationInputs<T extends ListNavigationItem> extends List
textDirection: SignalLike<'rtl' | 'ltr'>;
}

/** Options for list navigation. */
export interface ListNavigationOpts<T> {
/**
* Whether to focus the item's element.
* Defaults to true.
*/
focusElement?: boolean;

/**
* The list of items to navigate through.
* Defaults to the list of items from the inputs.
*/
items?: T[];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The items override feels a bit weird, because whether an override is provided or not, the moving index is always based on the activeIndex from the original input, and the following case becomes hard to reason out.

  • ListNavigation is initiated with items A, and an active index of A.
  • Navigation happens and a different items B is provided.
  • Some logic internally handles the mismatch between active index of A and the items B.

This last part is implicit and not easy to observe.

This makes me think about maybe ListNavigation behavior should just be a set of utility methods, and each method always take all necessary informations (items, activeIndex, wrap, etc.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think ListNavigation should be a set of utility methods since it would require a lot of args in order to perform its operations. Either way, this is outside the scope of this PR.

This strange behavior can be replicated by changing the inputs.items currently. We could prevent this misuse of the behavior with some strict checks that warn or error if these come up. I think we are over-anticipating future concerns that I'm not convinced we will actually encounter

}

/** Controls navigation for a list of items. */
export class ListNavigation<T extends ListNavigationItem> {
constructor(readonly inputs: ListNavigationInputs<T> & {focusManager: ListFocus<T>}) {}

/** Navigates to the given item. */
goto(item?: T, opts?: {focusElement?: boolean}): boolean {
goto(item?: T, opts?: ListNavigationOpts<T>): boolean {
return item ? this.inputs.focusManager.focus(item, opts) : false;
}

/** Navigates to the next item in the list. */
next(opts?: {focusElement?: boolean}): boolean {
next(opts?: ListNavigationOpts<T>): boolean {
return this._advance(1, opts);
}

/** Peeks the next item in the list. */
peekNext(): T | undefined {
return this._peek(1);
peekNext(opts?: ListNavigationOpts<T>): T | undefined {
return this._peek(1, opts);
}

/** Navigates to the previous item in the list. */
prev(opts?: {focusElement?: boolean}): boolean {
prev(opts?: ListNavigationOpts<T>): boolean {
return this._advance(-1, opts);
}

/** Peeks the previous item in the list. */
peekPrev(): T | undefined {
return this._peek(-1);
peekPrev(opts?: ListNavigationOpts<T>): T | undefined {
return this._peek(-1, opts);
}

/** Navigates to the first item in the list. */
first(opts?: {focusElement?: boolean}): boolean {
const item = this.peekFirst();
first(opts?: ListNavigationOpts<T>): boolean {
const item = this.peekFirst(opts);
return item ? this.goto(item, opts) : false;
}

/** Navigates to the last item in the list. */
last(opts?: {focusElement?: boolean}): boolean {
const item = this.peekLast();
last(opts?: ListNavigationOpts<T>): boolean {
const item = this.peekLast(opts);
return item ? this.goto(item, opts) : false;
}

/** Gets the first focusable item from the given list of items. */
peekFirst(items: T[] = this.inputs.items()): T | undefined {
peekFirst(opts?: ListNavigationOpts<T>): T | undefined {
const items = opts?.items ?? this.inputs.items();
return items.find(i => this.inputs.focusManager.isFocusable(i));
}

/** Gets the last focusable item from the given list of items. */
peekLast(items: T[] = this.inputs.items()): T | undefined {
peekLast(opts?: ListNavigationOpts<T>): T | undefined {
const items = opts?.items ?? this.inputs.items();
for (let i = items.length - 1; i >= 0; i--) {
if (this.inputs.focusManager.isFocusable(items[i])) {
return items[i];
Expand All @@ -81,16 +98,21 @@ export class ListNavigation<T extends ListNavigationItem> {
}

/** Advances to the next or previous focusable item in the list based on the given delta. */
private _advance(delta: 1 | -1, opts?: {focusElement?: boolean}): boolean {
const item = this._peek(delta);
private _advance(delta: 1 | -1, opts?: ListNavigationOpts<T>): boolean {
const item = this._peek(delta, opts);
return item ? this.goto(item, opts) : false;
}

/** Peeks the next or previous focusable item in the list based on the given delta. */
private _peek(delta: 1 | -1): T | undefined {
const items = this.inputs.items();
private _peek(delta: 1 | -1, opts?: ListNavigationOpts<T>): T | undefined {
const items = opts?.items ?? this.inputs.items();
const itemCount = items.length;
const startIndex = this.inputs.focusManager.activeIndex();
const activeItem = this.inputs.focusManager.inputs.activeItem();
const startIndex =
opts?.items && activeItem
? items.indexOf(activeItem)
: this.inputs.focusManager.activeIndex();

const step = (i: number) =>
this.inputs.wrap() ? (i + delta + itemCount) % itemCount : i + delta;

Expand Down
3 changes: 2 additions & 1 deletion src/aria/private/behaviors/list-selection/list-selection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export class ListSelection<T extends ListSelectionItem<V>, V> {
!item ||
item.disabled() ||
!item.selectable() ||
!this.inputs.focusManager.isFocusable(item as T) ||
this.inputs.values().includes(item.value())
) {
return;
Expand Down Expand Up @@ -138,7 +139,7 @@ export class ListSelection<T extends ListSelectionItem<V>, V> {
toggleAll() {
const selectableValues = this.inputs
.items()
.filter(i => !i.disabled() && i.selectable())
.filter(i => !i.disabled() && i.selectable() && this.inputs.focusManager.isFocusable(i))
.map(i => i.value());

selectableValues.every(i => this.inputs.values().includes(i))
Expand Down
33 changes: 33 additions & 0 deletions src/aria/private/behaviors/list/list.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,39 @@ describe('List Behavior', () => {
list.prev();
expect(list.inputs.activeItem()).toBe(list.inputs.items()[0]);
});

describe('with items subset', () => {
it('should navigate next/prev within subset', () => {
const {list, items} = getDefaultPatterns();
const subset = [items[0], items[2], items[4]];

// Start at 0
expect(list.inputs.activeItem()).toBe(items[0]);

// next(subset) -> 2 (skip 1)
list.next({items: subset});
expect(list.inputs.activeItem()).toBe(items[2]);

// next(subset) -> 4 (skip 3)
list.next({items: subset});
expect(list.inputs.activeItem()).toBe(items[4]);

// prev(subset) -> 2 (skip 3)
list.prev({items: subset});
expect(list.inputs.activeItem()).toBe(items[2]);
});

it('should verify first/last within subset', () => {
const {list, items} = getDefaultPatterns();
const subset = [items[1], items[2], items[3]];

list.first({items: subset});
expect(list.inputs.activeItem()).toBe(items[1]);

list.last({items: subset});
expect(list.inputs.activeItem()).toBe(items[3]);
});
});
});

describe('Selection', () => {
Expand Down
13 changes: 7 additions & 6 deletions src/aria/private/behaviors/list/list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,14 @@ import {
} from '../list-typeahead/list-typeahead';

/** The operations that the list can perform after navigation. */
interface NavOptions {
export interface NavOptions<T = any> {
toggle?: boolean;
select?: boolean;
selectOne?: boolean;
selectRange?: boolean;
anchor?: boolean;
focusElement?: boolean;
items?: T[];
}

/** Represents an item in the list. */
Expand Down Expand Up @@ -106,27 +107,27 @@ export class List<T extends ListItem<V>, V> {
}

/** Navigates to the first option in the list. */
first(opts?: NavOptions) {
first(opts?: NavOptions<T>) {
this._navigate(opts, () => this.navigationBehavior.first(opts));
}

/** Navigates to the last option in the list. */
last(opts?: NavOptions) {
last(opts?: NavOptions<T>) {
this._navigate(opts, () => this.navigationBehavior.last(opts));
}

/** Navigates to the next option in the list. */
next(opts?: NavOptions) {
next(opts?: NavOptions<T>) {
this._navigate(opts, () => this.navigationBehavior.next(opts));
}

/** Navigates to the previous option in the list. */
prev(opts?: NavOptions) {
prev(opts?: NavOptions<T>) {
this._navigate(opts, () => this.navigationBehavior.prev(opts));
}

/** Navigates to the given item in the list. */
goto(item: T, opts?: NavOptions) {
goto(item: T, opts?: NavOptions<T>) {
this._navigate(opts, () => this.navigationBehavior.goto(item, opts));
}

Expand Down
Loading
Loading