Skip to content

fix(cdk-experimental/listbox): change shift+nav behavior #30854

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 17, 2025
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export enum ModifierKey {
Shift = 0b10,
Alt = 0b100,
Meta = 0b1000,
Any = 'Any',
}

export type ModifierInputs = ModifierKey | ModifierKey[];
Expand Down Expand Up @@ -99,5 +100,10 @@ export function getModifiers(event: EventWithModifiers): number {
export function hasModifiers(event: EventWithModifiers, modifiers: ModifierInputs): boolean {
const eventModifiers = getModifiers(event);
const modifiersList = Array.isArray(modifiers) ? modifiers : [modifiers];

if (modifiersList.includes(ModifierKey.Any)) {
return true;
}

return modifiersList.some(modifiers => eventModifiers === modifiers);
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.dev/license
*/

import {signal} from '@angular/core';
import {computed, signal} from '@angular/core';
import {SignalLike, WritableSignalLike} from '../signal-like/signal-like';

/** Represents an item in a collection, such as a listbox option, than can be navigated to. */
Expand Down Expand Up @@ -41,45 +41,47 @@ export class ListNavigation<T extends ListNavigationItem> {
/** The last index that was active. */
prevActiveIndex = signal(0);

/** The current active item. */
activeItem = computed(() => this.inputs.items()[this.inputs.activeIndex()]);

constructor(readonly inputs: ListNavigationInputs<T>) {}

/** Navigates to the given item. */
goto(item: T) {
if (this.isFocusable(item)) {
goto(item?: T): boolean {
if (item && this.isFocusable(item)) {
this.prevActiveIndex.set(this.inputs.activeIndex());
const index = this.inputs.items().indexOf(item);
this.inputs.activeIndex.set(index);
return true;
}
return false;
}

/** Navigates to the next item in the list. */
next() {
this._advance(1);
next(): boolean {
return this._advance(1);
}

/** Navigates to the previous item in the list. */
prev() {
this._advance(-1);
prev(): boolean {
return this._advance(-1);
}

/** Navigates to the first item in the list. */
first() {
first(): boolean {
const item = this.inputs.items().find(i => this.isFocusable(i));

if (item) {
this.goto(item);
}
return item ? this.goto(item) : false;
}

/** Navigates to the last item in the list. */
last() {
last(): boolean {
const items = this.inputs.items();
for (let i = items.length - 1; i >= 0; i--) {
if (this.isFocusable(items[i])) {
this.goto(items[i]);
return;
return this.goto(items[i]);
}
}
return false;
}

/** Returns true if the given item can be navigated to. */
Expand All @@ -88,7 +90,7 @@ 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) {
private _advance(delta: 1 | -1): boolean {
const items = this.inputs.items();
const itemCount = items.length;
const startIndex = this.inputs.activeIndex();
Expand All @@ -100,9 +102,10 @@ export class ListNavigation<T extends ListNavigationItem> {
// when the index goes out of bounds.
for (let i = step(startIndex); i !== startIndex && i < itemCount && i >= 0; i = step(i)) {
if (this.isFocusable(items[i])) {
this.goto(items[i]);
return;
return this.goto(items[i]);
}
}

return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,36 @@ describe('List Selection', () => {
});
});

describe('#toggleOne', () => {
it('should select an unselected item', () => {
const items = getItems([0, 1, 2, 3, 4]);
const nav = getNavigation(items);
const selection = getSelection(items, nav);

selection.toggleOne(); // [0]
expect(selection.inputs.value()).toEqual([0]);
});

it('should deselect a selected item', () => {
const items = getItems([0, 1, 2, 3, 4]);
const nav = getNavigation(items);
const selection = getSelection(items, nav);
selection.select(); // [0]
selection.toggleOne(); // []
expect(selection.inputs.value().length).toBe(0);
});

it('should only leave one item selected', () => {
const items = getItems([0, 1, 2, 3, 4]);
const nav = getNavigation(items);
const selection = getSelection(items, nav);
selection.select(); // [0]
nav.next();
selection.toggleOne(); // [1]
expect(selection.inputs.value()).toEqual([1]);
});
});

describe('#selectAll', () => {
it('should select all items', () => {
const items = getItems([0, 1, 2, 3, 4]);
Expand Down Expand Up @@ -185,7 +215,71 @@ describe('List Selection', () => {
});
});

describe('#selectFromAnchor', () => {
describe('#toggleAll', () => {
it('should select all items', () => {
const items = getItems([0, 1, 2, 3, 4]);
const nav = getNavigation(items);
const selection = getSelection(items, nav);
selection.toggleAll();
expect(selection.inputs.value()).toEqual([0, 1, 2, 3, 4]);
});

it('should deselect all if all items are selected', () => {
const items = getItems([0, 1, 2, 3, 4]);
const nav = getNavigation(items);
const selection = getSelection(items, nav);
selection.selectAll();
selection.toggleAll();
expect(selection.inputs.value()).toEqual([]);
});

it('should ignore disabled items when determining if all items are selected', () => {
const items = getItems([0, 1, 2, 3, 4]);
const nav = getNavigation(items);
const selection = getSelection(items, nav);
items()[0].disabled.set(true);
selection.toggleAll();
expect(selection.inputs.value()).toEqual([1, 2, 3, 4]);
selection.toggleAll();
expect(selection.inputs.value()).toEqual([]);
});
});

describe('#selectOne', () => {
it('should select a single item', () => {
const items = getItems([0, 1, 2, 3, 4]);
const nav = getNavigation(items);
const selection = getSelection(items, nav);

selection.selectOne(); // [0]
nav.next();
selection.selectOne(); // [1]
expect(selection.inputs.value()).toEqual([1]);
});

it('should not select disabled items', () => {
const items = getItems([0, 1, 2, 3, 4]);
const nav = getNavigation(items);
const selection = getSelection(items, nav);
items()[0].disabled.set(true);

selection.select(); // []
expect(selection.inputs.value()).toEqual([]);
});

it('should do nothing to already selected items', () => {
const items = getItems([0, 1, 2, 3, 4]);
const nav = getNavigation(items);
const selection = getSelection(items, nav);

selection.selectOne(); // [0]
selection.selectOne(); // [0]

expect(selection.inputs.value()).toEqual([0]);
});
});

describe('#selectRange', () => {
it('should select all items from an anchor at a lower index', () => {
const items = getItems([0, 1, 2, 3, 4]);
const nav = getNavigation(items);
Expand All @@ -194,7 +288,7 @@ describe('List Selection', () => {
selection.select(); // [0]
nav.next();
nav.next();
selection.selectFromPrevSelectedItem(); // [0, 1, 2]
selection.selectRange(); // [0, 1, 2]

expect(selection.inputs.value()).toEqual([0, 1, 2]);
});
Expand All @@ -209,10 +303,98 @@ describe('List Selection', () => {
selection.select(); // [3]
nav.prev();
nav.prev();
selection.selectFromPrevSelectedItem(); // [3, 1, 2]
selection.selectRange(); // [3, 2, 1]

expect(selection.inputs.value()).toEqual([3, 2, 1]);
});

it('should deselect items within the range when the range is changed', () => {
const items = getItems([0, 1, 2, 3, 4]);
const nav = getNavigation(items);
const selection = getSelection(items, nav);

nav.next();
nav.next();
selection.select(); // [2]
expect(selection.inputs.value()).toEqual([2]);

// TODO(wagnermaciel): Order the values when inserting them.
expect(selection.inputs.value()).toEqual([3, 1, 2]);
nav.next();
nav.next();
selection.selectRange(); // [2, 3, 4]
expect(selection.inputs.value()).toEqual([2, 3, 4]);

nav.first();
selection.selectRange(); // [2, 1, 0]
expect(selection.inputs.value()).toEqual([2, 1, 0]);
});

it('should not select a disabled item', () => {
const items = getItems([0, 1, 2, 3, 4]);
const nav = getNavigation(items);
const selection = getSelection(items, nav);
items()[1].disabled.set(true);

selection.select(); // [0]
expect(selection.inputs.value()).toEqual([0]);

nav.next();
selection.selectRange(); // [0]
expect(selection.inputs.value()).toEqual([0]);

nav.next();
selection.selectRange(); // [0, 2]
expect(selection.inputs.value()).toEqual([0, 2]);
});

it('should not deselect a disabled item', () => {
const items = getItems([0, 1, 2, 3, 4]);
const nav = getNavigation(items);
const selection = getSelection(items, nav);

selection.select(items()[1]);
items()[1].disabled.set(true);

selection.select(); // [0]
expect(selection.inputs.value()).toEqual([1, 0]);

nav.next();
nav.next();
selection.selectRange(); // [0, 1, 2]
expect(selection.inputs.value()).toEqual([1, 0, 2]);

nav.prev();
nav.prev();
selection.selectRange(); // [0]
expect(selection.inputs.value()).toEqual([1, 0]);
});
});

describe('#beginRangeSelection', () => {
it('should set where a range is starting from', () => {
const items = getItems([0, 1, 2, 3, 4]);
const nav = getNavigation(items);
const selection = getSelection(items, nav);

nav.next();
nav.next();
selection.beginRangeSelection();
expect(selection.inputs.value()).toEqual([]);
nav.next();
nav.next();
selection.selectRange(); // [2, 3, 4]
expect(selection.inputs.value()).toEqual([2, 3, 4]);
});

it('should be able to select a range starting on a disabled item', () => {
const items = getItems([0, 1, 2, 3, 4]);
const nav = getNavigation(items);
const selection = getSelection(items, nav);
items()[0].disabled.set(true);
selection.beginRangeSelection(0);
nav.next();
nav.next();
selection.selectRange();
expect(selection.inputs.value()).toEqual([1, 2]);
});
});
});
Loading
Loading