feat: virtual list item selection

[tomivirkki]

Description

Item selection support for <vaadin-virtual-list>

Part of vaadin/platform#6838

Type of change

Feature

[vursen]

First batch of comments.

[vursen]

Suggested change

	__updateItemModel(model, item, index) {
	model.item = this.items[index];
	model.index = index;
	if (super.__updateItemModel) {
	super.__updateItemModel(model, item, index);
	__getItemModel(index) {
	return {
	item: this.items[index],
	index,
	...super.__updateItemModel ? super.__updateItemModel(index) : {}
	}

[tomivirkki]

You probably meant ...super.__getItemModel ? super.__getItemModel(index) : {}? I used this approach earlier but it kind of fealt awkward to return an incomplete model { selected: this.__isSelected(...) } from a function named __getItemModel

[vursen]

You probably meant ...super.__getItemModel ? super.__getItemModel(index) : {}?

Yes, sorry for the mistake.

I used this approach earlier but it kind of fealt awkward to return an incomplete model { selected: this.__isSelected(...) } from a function named __getItemModel

Yeah, I agree, it would be confusing. This problem seems related to the concern I mentioned in #8318 (comment). The current order in which mixins extend each other is a bit confusing in my opinion. I would expect that there would be a base mixin that defines the default model, with SelectionMixin extending this base mixin to add its own functionality, rather than the other way around.

class BaseMixin {
  getItemModel(index) {
    return { item: this.items[index], index } 
  }

  updateElement(row, index) {
    el.setAttribute('role', 'listitem');
    el.setAttribute('aria-set-size', this.items.length);
  }
}

class SelectionMixin extends BaseMixin {
  getItemModel(index) {
    const item = this.items[index];
    return { ...super.getItemModel(index), selected: this.isSelected(item) }
  }
  
  updateElement(row, index) {
    super.updateElement(row, index);
    
    const model = this.getItemModel(index);
    if (this.isSelectable) {
      el.setAttribute('role', 'option');
      el.setAttribute('aria-selected', this.isSelected(model.item));
      ...
    } else {
      el.removeAttribute('aria-selected');
      ...
    }
  }
}

[tomivirkki]

Done. super.updateElement need to be called at the end of the overridden function to have the element up-to-date by the time the renderer gets called. That's why I also needed to extract __updateElementRole as a separate function.

[web-padawan]

Not sure if related to this PR, but I noticed that clicking on an empty space in the virtual list makes the text of the first item selected which looks a bit weird:

virtual-list-click.mp4

[sissbruecker]

Suggested change

	assertType<boolean | undefined>(model.selected);
	assertType<boolean>(model.selected as boolean);
	assertType<undefined>(model.selected as undefined);

Otherwise this would also work if you removed undefined from the type. There are several checks like these below that could be fixed.

[tomivirkki]

Done

[sissbruecker]

navigating is also set when using the mouse, which is not how grid or listbox work. Should we align it with those components? As a practical concern, how can you show the focus outline only when using keyboard?

[tomivirkki]

Done

[sissbruecker]

This could either provide an example, or just explain what parameters the function receives and what it is supposed to return.

[tomivirkki]

Addressed by #8356

[sissbruecker]

When using multi-select it only announces the number of selected items that are currently rendered. So if three selected items are out of view and you select a new one it will announce that only a single item is selected, while there are actually four. Another issue is that scrolling selected items out of view (so that they are removed from DOM) will announce that they have been removed from the selection, which is not the case.

Only tested this with VoiceOver, but I think this is a general issue with how virtual list works.

[tomivirkki]

How do I make VO announce the number of selected items? While navigating, I only saw announcements related to the currently focused item selection state and its position.

Kapture.2024-12-12.at.11.52.15.mp4

[sissbruecker]

It announces the number when changing the selection.

I also just tested with NVDA, which never announces the current selection, neither for single nor multi listboxes. You only get information about the selection state when going through individual items.

[sissbruecker]

Using single selection, when you switch the selection from one item to another one, VoiceOver doesn't announce a selection change.

[tomivirkki]

Done

[sonarqubecloud]

Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud