Skip to content

fix(cdk-experimental/ui-patterns): listbox pointer event handler #30843

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 1 commit into from
Apr 11, 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 @@ -115,29 +115,56 @@ export class ListSelection<T extends ListSelectionItem<V>, V> {
this._selectFromIndex(this.inputs.navigation.prevActiveIndex());
}

/** Selects the items in the list starting at the given index. */
private _selectFromIndex(index: number) {
if (index === -1) {
return;
}

const upper = Math.max(this.inputs.navigation.inputs.activeIndex(), index);
const lower = Math.min(this.inputs.navigation.inputs.activeIndex(), index);

for (let i = lower; i <= upper; i++) {
this.select(this.inputs.items()[i]);
}
}

/** Sets the selection to only the current active item. */
selectOne() {
this.deselectAll();
this.select();
}

/** Toggles the items in the list starting at the last selected item. */
toggleFromPrevSelectedItem() {
const prevIndex = this.inputs.items().findIndex(i => this.previousValue() === i.value());
const currIndex = this.inputs.navigation.inputs.activeIndex();
const currValue = this.inputs.items()[currIndex].value();
const items = this._getItemsFromIndex(prevIndex);

const operation = this.inputs.value().includes(currValue)
? this.deselect.bind(this)
: this.select.bind(this);

for (const item of items) {
operation(item);
}
}

/** Sets the anchor to the current active index. */
private _anchor() {
const item = this.inputs.items()[this.inputs.navigation.inputs.activeIndex()];
this.previousValue.set(item.value());
}

/** Selects the items in the list starting at the given index. */
private _selectFromIndex(index: number) {
const items = this._getItemsFromIndex(index);

for (const item of items) {
this.select(item);
}
}

/** Returns all items from the given index to the current active index. */
private _getItemsFromIndex(index: number) {
if (index === -1) {
return [];
}

const upper = Math.max(this.inputs.navigation.inputs.activeIndex(), index);
const lower = Math.min(this.inputs.navigation.inputs.activeIndex(), index);

const items = [];
for (let i = lower; i <= upper; i++) {
items.push(this.inputs.items()[i]);
}
return items;
}
}
158 changes: 157 additions & 1 deletion src/cdk-experimental/ui-patterns/listbox/listbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,15 @@ describe('Listbox Pattern', () => {

function getOptions(listbox: TestListbox, values: string[]): TestOption[] {
return values.map((value, index) => {
const element = document.createElement('div');
element.role = 'option';
return new OptionPattern({
value: signal(value),
id: signal(`option-${index}`),
disabled: signal(false),
searchTerm: signal(value),
listbox: signal(listbox),
element: signal({focus: () => {}} as HTMLElement),
element: signal(element),
});
});
}
Expand Down Expand Up @@ -439,4 +441,158 @@ describe('Listbox Pattern', () => {
});
});
});

describe('Pointer Events', () => {
function click(options: WritableSignal<TestOption[]>, index: number, mods?: ModifierKeys) {
return {
target: options()[index].element(),
shiftKey: mods?.shift,
ctrlKey: mods?.control,
} as unknown as PointerEvent;
}

describe('follows focus & single select', () => {
it('should select a single option on click', () => {
const {listbox, options} = getDefaultPatterns({
multi: signal(false),
selectionMode: signal('follow'),
});
listbox.onPointerdown(click(options, 0));
expect(listbox.inputs.value()).toEqual(['Apple']);
});
});

describe('explicit focus & single select', () => {
it('should select an unselected option on click', () => {
const {listbox, options} = getDefaultPatterns({
multi: signal(false),
selectionMode: signal('explicit'),
});
listbox.onPointerdown(click(options, 0));
expect(listbox.inputs.value()).toEqual(['Apple']);
});

it('should deselect a selected option on click', () => {
const {listbox, options} = getDefaultPatterns({
multi: signal(false),
value: signal(['Apple']),
selectionMode: signal('explicit'),
});
listbox.onPointerdown(click(options, 0));
expect(listbox.inputs.value()).toEqual([]);
});
});

describe('explicit focus & multi select', () => {
it('should select an unselected option on click', () => {
const {listbox, options} = getDefaultPatterns({
multi: signal(true),
selectionMode: signal('explicit'),
});
listbox.onPointerdown(click(options, 0));
expect(listbox.inputs.value()).toEqual(['Apple']);
});

it('should deselect a selected option on click', () => {
const {listbox, options} = getDefaultPatterns({
multi: signal(true),
value: signal(['Apple']),
selectionMode: signal('explicit'),
});
listbox.onPointerdown(click(options, 0));
expect(listbox.inputs.value()).toEqual([]);
});

it('should select options from anchor on shift + click', () => {
const {listbox, options} = getDefaultPatterns({
multi: signal(true),
selectionMode: signal('explicit'),
});
listbox.onPointerdown(click(options, 2));
listbox.onPointerdown(click(options, 5, {shift: true}));
expect(listbox.inputs.value()).toEqual(['Banana', 'Blackberry', 'Blueberry', 'Cantaloupe']);
});

it('should deselect options from anchor on shift + click', () => {
const {listbox, options} = getDefaultPatterns({
multi: signal(true),
selectionMode: signal('explicit'),
});
listbox.onPointerdown(click(options, 2));
listbox.onPointerdown(click(options, 5));
listbox.onPointerdown(click(options, 2, {shift: true}));
expect(listbox.inputs.value()).toEqual([]);
});
});

describe('follows focus & multi select', () => {
it('should select a single option on click', () => {
const {listbox, options} = getDefaultPatterns({
multi: signal(true),
selectionMode: signal('follow'),
});
listbox.onPointerdown(click(options, 0));
expect(listbox.inputs.value()).toEqual(['Apple']);
listbox.onPointerdown(click(options, 1));
expect(listbox.inputs.value()).toEqual(['Apricot']);
listbox.onPointerdown(click(options, 2));
expect(listbox.inputs.value()).toEqual(['Banana']);
});

it('should select an unselected option on ctrl + click', () => {
const {listbox, options} = getDefaultPatterns({
multi: signal(true),
selectionMode: signal('follow'),
});
listbox.onPointerdown(click(options, 0));
expect(listbox.inputs.value()).toEqual(['Apple']);
listbox.onPointerdown(click(options, 1, {control: true}));
expect(listbox.inputs.value()).toEqual(['Apple', 'Apricot']);
listbox.onPointerdown(click(options, 2, {control: true}));
expect(listbox.inputs.value()).toEqual(['Apple', 'Apricot', 'Banana']);
});

it('should deselect a selected option on ctrl + click', () => {
const {listbox, options} = getDefaultPatterns({
multi: signal(true),
selectionMode: signal('follow'),
});
listbox.onPointerdown(click(options, 0));
expect(listbox.inputs.value()).toEqual(['Apple']);
listbox.onPointerdown(click(options, 0, {control: true}));
expect(listbox.inputs.value()).toEqual([]);
});

it('should select options from anchor on shift + click', () => {
const {listbox, options} = getDefaultPatterns({
multi: signal(true),
selectionMode: signal('follow'),
});
listbox.onPointerdown(click(options, 2));
listbox.onPointerdown(click(options, 5, {shift: true}));
expect(listbox.inputs.value()).toEqual(['Banana', 'Blackberry', 'Blueberry', 'Cantaloupe']);
});

it('should deselect options from anchor on shift + click', () => {
const {listbox, options} = getDefaultPatterns({
multi: signal(true),
selectionMode: signal('follow'),
});
listbox.onPointerdown(click(options, 2));
listbox.onPointerdown(click(options, 5, {control: true}));
listbox.onPointerdown(click(options, 2, {shift: true}));
expect(listbox.inputs.value()).toEqual([]);
});
});

it('should only navigate when readonly', () => {
const {listbox, options} = getDefaultPatterns({readonly: signal(true)});
listbox.onPointerdown(click(options, 0));
expect(listbox.inputs.value()).toEqual([]);
listbox.onPointerdown(click(options, 1));
expect(listbox.inputs.value()).toEqual([]);
listbox.onPointerdown(click(options, 2));
expect(listbox.inputs.value()).toEqual([]);
});
});
});
25 changes: 22 additions & 3 deletions src/cdk-experimental/ui-patterns/listbox/listbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ interface SelectOptions {
selectAll?: boolean;
selectFromAnchor?: boolean;
selectFromActive?: boolean;
toggleFromAnchor?: boolean;
}

/** Represents the required inputs for a listbox. */
Expand Down Expand Up @@ -167,13 +168,28 @@ export class ListboxPattern<V> {
return manager.on(e => this.goto(e));
}

if (this.inputs.multi()) {
if (!this.multi() && this.followFocus()) {
return manager.on(e => this.goto(e, {selectOne: true}));
}

if (!this.multi() && !this.followFocus()) {
return manager.on(e => this.goto(e, {toggle: true}));
}

if (this.multi() && this.followFocus()) {
return manager
.on(e => this.goto(e, {selectOne: true}))
.on(Modifier.Ctrl, e => this.goto(e, {toggle: true}))
.on(Modifier.Shift, e => this.goto(e, {toggleFromAnchor: true}));
}

if (this.multi() && !this.followFocus()) {
return manager
.on(e => this.goto(e, {toggle: true}))
.on(Modifier.Shift, e => this.goto(e, {selectFromActive: true}));
.on(Modifier.Shift, e => this.goto(e, {toggleFromAnchor: true}));
}

return manager.on(e => this.goto(e, {toggleOne: true}));
return manager;
});

constructor(readonly inputs: ListboxInputs<V>) {
Expand Down Expand Up @@ -270,6 +286,9 @@ export class ListboxPattern<V> {
if (opts?.selectFromActive) {
this.selection.selectFromActive();
}
if (opts?.toggleFromAnchor) {
this.selection.toggleFromPrevSelectedItem();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be toggleFromFocusedItem.

If I click an item to select, then press arrow down several times, then shift click, I'd expect selection to be starting from the focused item, not previously selected item.

Copy link
Contributor Author

@wagnermaciel wagnermaciel Apr 10, 2025

Choose a reason for hiding this comment

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

I don't think that's correct. Gmail & React Aria both select from the previous selected

Edit: Also see https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Attributes/multiple

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We prob shouldn't use gmail as a reference, though

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah okay, trying it again I think Im wrong. LGTM!

}
}

private _getItem(e: PointerEvent) {
Expand Down
Loading