From 627689b9c40124c9859a3c96dcbf2d04821dbdfa Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Fri, 22 Nov 2024 16:03:16 +0200 Subject: [PATCH 01/49] feat: virtual list item selection --- dev/virtual-list.html | 15 +- packages/virtual-list/package.json | 1 + .../src/vaadin-lit-virtual-list.js | 2 + .../src/vaadin-virtual-list-mixin.js | 16 +- .../vaadin-virtual-list-selection-mixin.d.ts | 29 ++ .../vaadin-virtual-list-selection-mixin.js | 282 +++++++++++++++++ .../virtual-list/src/vaadin-virtual-list.d.ts | 5 +- .../virtual-list/src/vaadin-virtual-list.js | 2 + .../test/typings/virtual-list.types.ts | 4 + .../test/virtual-list-selection-lit.test.ts | 2 + .../virtual-list-selection-polymer.test.ts | 2 + .../test/virtual-list-selection.common.ts | 294 ++++++++++++++++++ .../theme/lumo/vaadin-lit-virtual-list.js | 1 + .../theme/lumo/vaadin-virtual-list-styles.js | 18 ++ .../theme/lumo/vaadin-virtual-list.js | 1 + 15 files changed, 669 insertions(+), 5 deletions(-) create mode 100644 packages/virtual-list/src/vaadin-virtual-list-selection-mixin.d.ts create mode 100644 packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js create mode 100644 packages/virtual-list/test/virtual-list-selection-lit.test.ts create mode 100644 packages/virtual-list/test/virtual-list-selection-polymer.test.ts create mode 100644 packages/virtual-list/test/virtual-list-selection.common.ts create mode 100644 packages/virtual-list/theme/lumo/vaadin-virtual-list-styles.js diff --git a/dev/virtual-list.html b/dev/virtual-list.html index 06a5134b0d..7aaeab7f1c 100644 --- a/dev/virtual-list.html +++ b/dev/virtual-list.html @@ -1,4 +1,4 @@ - + @@ -9,6 +9,7 @@ + + diff --git a/packages/virtual-list/package.json b/packages/virtual-list/package.json index ac0d218959..b3e2ac85c8 100644 --- a/packages/virtual-list/package.json +++ b/packages/virtual-list/package.json @@ -39,6 +39,7 @@ "dependencies": { "@open-wc/dedupe-mixin": "^1.3.0", "@polymer/polymer": "^3.0.0", + "@vaadin/a11y-base": "24.7.0-alpha1", "@vaadin/component-base": "24.7.0-alpha1", "@vaadin/lit-renderer": "24.7.0-alpha1", "@vaadin/vaadin-lumo-styles": "24.7.0-alpha1", diff --git a/packages/virtual-list/src/vaadin-lit-virtual-list.js b/packages/virtual-list/src/vaadin-lit-virtual-list.js index f5b8bb2db5..1547eb3fce 100644 --- a/packages/virtual-list/src/vaadin-lit-virtual-list.js +++ b/packages/virtual-list/src/vaadin-lit-virtual-list.js @@ -34,6 +34,8 @@ class VirtualList extends VirtualListMixin(ThemableMixin(ElementMixin(PolylitMix
+ + `; } } diff --git a/packages/virtual-list/src/vaadin-virtual-list-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-mixin.js index e5b3d427e2..d966f5d42c 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-mixin.js @@ -7,13 +7,14 @@ import { ControllerMixin } from '@vaadin/component-base/src/controller-mixin.js' import { OverflowController } from '@vaadin/component-base/src/overflow-controller.js'; import { processTemplates } from '@vaadin/component-base/src/templates.js'; import { Virtualizer } from '@vaadin/component-base/src/virtualizer.js'; +import { SelectionMixin } from './vaadin-virtual-list-selection-mixin.js'; /** * @polymerMixin * @mixes ControllerMixin */ export const VirtualListMixin = (superClass) => - class VirtualListMixinClass extends ControllerMixin(superClass) { + class VirtualListMixinClass extends SelectionMixin(ControllerMixin(superClass)) { static get properties() { return { /** @@ -118,16 +119,27 @@ export const VirtualListMixin = (superClass) => /** @private */ __updateElement(el, index) { + if (super.__updateElement) { + super.__updateElement(el, index); + } + if (el.__renderer !== this.renderer) { el.__renderer = this.renderer; this.__clearRenderTargetContent(el); } if (this.renderer) { - this.renderer(el, this, { item: this.items[index], index }); + this.renderer(el, this, this.__getItemModel(this.items[index], index)); } } + /** + * @private + */ + __getItemModel(item, index) { + return { item, index }; + } + /** * Clears the content of a render target. * @private diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.d.ts b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.d.ts new file mode 100644 index 0000000000..1b3d6691cb --- /dev/null +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.d.ts @@ -0,0 +1,29 @@ +/** + * @license + * Copyright (c) 2021 - 2024 Vaadin Ltd. + * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/ + */ +import type { Constructor } from '@open-wc/dedupe-mixin'; +import type { VirtualListDefaultItem } from './vaadin-virtual-list.js'; + +export declare function VirtualListSelectionMixin>( + base: T, +): Constructor & T; + +export declare class VirtualListSelectionMixinClass { + /** + * Selection mode for the virtual list. Available modes are: `single` and `multi`. + */ + selectionMode?: 'single' | 'multi'; + + /** + * Path to an item sub-property that identifies the item. + * @attr {string} item-id-path + */ + itemIdPath: string | null | undefined; + + /** + * An array that contains the selected items. + */ + selectedItems: TItem[]; +} diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js new file mode 100644 index 0000000000..10785059cc --- /dev/null +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -0,0 +1,282 @@ +/** + * @license + * Copyright (c) 2016 - 2024 Vaadin Ltd. + * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/ + */ + +import { getFocusableElements } from '@vaadin/a11y-base'; +import { get } from '@vaadin/component-base/src/path-utils'; + +/** + * @polymerMixin + */ +export const SelectionMixin = (superClass) => + class SelectionMixin extends superClass { + static get properties() { + return { + /** + * Selection mode for the virtual list. Available modes are: `single` and `multi`. + * @type {string} + */ + selectionMode: { + type: String, + observer: '__selectionModeChanged', + }, + + /** + * Path to an item sub-property that identifies the item. + * @attr {string} item-id-path + */ + itemIdPath: { + type: String, + value: null, + sync: true, + }, + + /** + * An array that contains the selected items. + * @type {!Array} + */ + selectedItems: { + type: Object, + notify: true, + value: () => [], + sync: true, + }, + + /** + * Set of selected item ids + * @private + */ + __selectedKeys: { + type: Object, + computed: '__computeSelectedKeys(itemIdPath, selectedItems)', + }, + + /** + * @private + */ + __focusIndex: { + type: Object, + value: 0, + sync: true, + }, + }; + } + + static get observers() { + return ['__selectedItemsChanged(itemIdPath, selectedItems, __focusIndex)']; + } + + constructor() { + super(); + + this.addEventListener('keydown', (e) => this.__onKeyDown(e)); + this.addEventListener('click', (e) => this.__onClick(e)); + this.addEventListener('focusin', (e) => this.__onFocusIn(e)); + this.addEventListener('focusout', (e) => this.__onFocusOut(e)); + } + + /** @private */ + __updateElement(el, index) { + const item = this.items[index]; + el.__item = item; + + el.toggleAttribute('selected', this.__isSelected(item)); + el.tabIndex = this.__isNavigating() && this.selectionMode && this.__focusIndex === index ? 0 : -1; + + el.toggleAttribute( + 'focused', + this.selectionMode && this.__focusIndex === index && el.contains(document.activeElement), + ); + } + + /** @private */ + __selectionModeChanged() { + this.__setNavigating(true); + this.requestContentUpdate(); + } + + /** @private */ + __isSelected(item) { + return this.__selectedKeys.has(this.__getItemId(item)); + } + + /** @private */ + __selectedItemsChanged() { + this.requestContentUpdate(); + } + + /** @private */ + __computeSelectedKeys(_itemIdPath, selectedItems) { + return new Set((selectedItems || []).map((item) => this.__getItemId(item))); + } + + /** @private */ + __itemsEqual(item1, item2) { + return this.__getItemId(item1) === this.__getItemId(item2); + } + + /** @private */ + __getItemId(item) { + return this.itemIdPath ? get(this.itemIdPath, item) : item; + } + + /** @private */ + __getItemFromEvent(e) { + const element = e.composedPath().find((el) => el.parentElement === this); + return element ? element.__item : null; + } + + /** @private */ + __toggleSelection(item) { + if (this.__isSelected(item)) { + this.selectedItems = this.selectedItems.filter((selectedItem) => !this.__itemsEqual(selectedItem, item)); + } else if (this.selectionMode === 'multi') { + this.selectedItems = [...this.selectedItems, item]; + } else { + this.selectedItems = [item]; + } + } + + /** @private */ + __focusElementWithFocusIndex() { + if (!this.__getRenderedFocusIndexElement()) { + this.scrollToIndex(this.__focusIndex); + } + this.__getRenderedFocusIndexElement().focus(); + this.requestContentUpdate(); + } + + /** @private */ + __getRenderedFocusIndexElement() { + return [...this.children].find((el) => this.__getItemIndex(el.__item) === this.__focusIndex); + } + + /** @private */ + __getRootElementWithFocus() { + return [...this.children].find((el) => el.contains(document.activeElement)); + } + + /** @private */ + __isNavigating() { + return this.hasAttribute('navigating'); + } + + /** @private */ + __getItemIndex(item) { + return this.items.indexOf(item); + } + + /** @private */ + __setNavigating(navigating) { + this.tabIndex = this.selectionMode && navigating ? 0 : -1; + this.$.focusexit.hidden = !this.selectionMode || !navigating; + this.toggleAttribute('navigating', navigating); + this.toggleAttribute('interacting', !navigating); + this.requestContentUpdate(); + } + + /** @private */ + __onKeyDown(e) { + if (!this.selectionMode) { + return; + } + + if (this.__isNavigating()) { + if (e.key === 'ArrowDown' || e.key === 'ArrowUp') { + e.preventDefault(); + this.__onNavigationArrowKey(e.key === 'ArrowDown'); + } else if (e.key === 'Enter') { + e.preventDefault(); + this.__onNavigationEnterKey(); + } else if (e.key === ' ') { + e.preventDefault(); + this.__toggleSelection(this.__getItemFromEvent(e)); + } else if (e.key === 'Tab') { + this.__onNavigationTabKey(e.shiftKey); + } + } else if (e.key === 'Escape' && !e.defaultPrevented) { + this.__getRootElementWithFocus().focus(); + } + } + + /** @private */ + __onNavigationArrowKey(down) { + this.__focusIndex = Math.min(Math.max(this.__focusIndex + (down ? 1 : -1), 0), this.items.length - 1); + this.__focusElementWithFocusIndex(); + } + + /** @private */ + __onNavigationTabKey(shift) { + if (shift) { + this.focus(); + } else { + const scrollTop = this.scrollTop; + this.$.focusexit.focus(); + this.scrollTop = scrollTop; + } + } + + /** @private */ + __onNavigationEnterKey() { + // Get the focused item + const focusedItem = this.querySelector('[focused]'); + // First the first focusable element in the focused item and focus it + const focusableElement = getFocusableElements(focusedItem).find((el) => el !== focusedItem); + if (focusableElement) { + focusableElement.focus(); + } + } + + /** @private */ + __onClick(e) { + if (!this.selectionMode || !this.__isNavigating()) { + return; + } + + const item = this.__getItemFromEvent(e); + if (item) { + this.__toggleSelection(item); + } + } + + /** @private */ + __onFocusIn(e) { + if (!this.selectionMode) { + return; + } + + // Set navigating state if one of the root elements, virtual-list or focusexit, is focused + const navigating = [...this.children, this, this.$.focusexit].includes(e.target); + this.__setNavigating(navigating); + + // Update focus index based on the focused item + const targetItem = this.__getItemFromEvent(e); + if (targetItem) { + this.__focusIndex = this.__getItemIndex(targetItem); + } + + // Focus the root element matching focus index if focus came from outside + if (navigating && !this.contains(e.relatedTarget)) { + this.__focusElementWithFocusIndex(); + } + } + + /** @private */ + __onFocusOut() { + if (!this.selectionMode) { + return; + } + if (!this.contains(document.activeElement)) { + // If the focus leaves the virtual list, restore navigating state + this.__setNavigating(true); + } + } + + /** + * Fired when the `selectedItems` property changes. + * + * @event selected-items-changed + */ + }; diff --git a/packages/virtual-list/src/vaadin-virtual-list.d.ts b/packages/virtual-list/src/vaadin-virtual-list.d.ts index 04c88d0f54..6c9d6d7258 100644 --- a/packages/virtual-list/src/vaadin-virtual-list.d.ts +++ b/packages/virtual-list/src/vaadin-virtual-list.d.ts @@ -11,6 +11,7 @@ import type { VirtualListMixinClass, VirtualListRenderer, } from './vaadin-virtual-list-mixin.js'; +import type { VirtualListSelectionMixinClass } from './vaadin-virtual-list-selection-mixin.js'; export { VirtualListDefaultItem, VirtualListItemModel, VirtualListRenderer }; @@ -40,7 +41,9 @@ export { VirtualListDefaultItem, VirtualListItemModel, VirtualListRenderer }; declare class VirtualList extends ThemableMixin(ElementMixin(HTMLElement)) {} // eslint-disable-next-line @typescript-eslint/no-empty-object-type -interface VirtualList extends VirtualListMixinClass {} +interface VirtualList + extends VirtualListMixinClass, + VirtualListSelectionMixinClass {} declare global { interface HTMLElementTagNameMap { diff --git a/packages/virtual-list/src/vaadin-virtual-list.js b/packages/virtual-list/src/vaadin-virtual-list.js index 6da97d3c13..1add5c7987 100644 --- a/packages/virtual-list/src/vaadin-virtual-list.js +++ b/packages/virtual-list/src/vaadin-virtual-list.js @@ -47,6 +47,8 @@ class VirtualList extends ElementMixin(ThemableMixin(VirtualListMixin(PolymerEle
+ + `; } diff --git a/packages/virtual-list/test/typings/virtual-list.types.ts b/packages/virtual-list/test/typings/virtual-list.types.ts index fc99602ab2..7575fcfd7a 100644 --- a/packages/virtual-list/test/typings/virtual-list.types.ts +++ b/packages/virtual-list/test/typings/virtual-list.types.ts @@ -40,3 +40,7 @@ assertType<(index: number) => void>(virtualList.scrollToIndex); assertType(virtualList.firstVisibleIndex); assertType(virtualList.lastVisibleIndex); + +assertType<'single' | 'multi' | undefined>(virtualList.selectionMode); +assertType(virtualList.selectedItems); +assertType(virtualList.itemIdPath); diff --git a/packages/virtual-list/test/virtual-list-selection-lit.test.ts b/packages/virtual-list/test/virtual-list-selection-lit.test.ts new file mode 100644 index 0000000000..b6e469687f --- /dev/null +++ b/packages/virtual-list/test/virtual-list-selection-lit.test.ts @@ -0,0 +1,2 @@ +import '../vaadin-lit-virtual-list.js'; +import './virtual-list-selection.common.js'; diff --git a/packages/virtual-list/test/virtual-list-selection-polymer.test.ts b/packages/virtual-list/test/virtual-list-selection-polymer.test.ts new file mode 100644 index 0000000000..ee6307b507 --- /dev/null +++ b/packages/virtual-list/test/virtual-list-selection-polymer.test.ts @@ -0,0 +1,2 @@ +import '../vaadin-virtual-list.js'; +import './virtual-list-selection.common.js'; diff --git a/packages/virtual-list/test/virtual-list-selection.common.ts b/packages/virtual-list/test/virtual-list-selection.common.ts new file mode 100644 index 0000000000..61054ceab2 --- /dev/null +++ b/packages/virtual-list/test/virtual-list-selection.common.ts @@ -0,0 +1,294 @@ +import { expect } from '@vaadin/chai-plugins'; +import { click as helperClick, fixtureSync, nextFrame } from '@vaadin/testing-helpers'; +import { sendKeys } from '@web/test-runner-commands'; +import sinon from 'sinon'; +import { html, render } from 'lit'; +import type { VirtualList } from '../vaadin-virtual-list'; + +type TestItem = { id: number; name: string }; + +async function click(el: HTMLElement) { + el.focus(); + // TODO: No await? + await helperClick(el); +} + +async function shiftTab() { + await sendKeys({ down: 'Shift' }); + await sendKeys({ press: 'Tab' }); + await sendKeys({ up: 'Shift' }); +} + +describe('selection', () => { + let list: VirtualList; + let beforeButton: HTMLButtonElement; + let afterButton: HTMLButtonElement; + + function getRenderedItem(index: number) { + const childElements = Array.from(list.children) as Array; + return childElements.find((child) => !child.hidden && child.__item.id === index); + } + + beforeEach(async () => { + [beforeButton, list, afterButton] = fixtureSync(` +
+ + + +
+ `).children as any; + + list.style.height = '200px'; + list.items = Array.from({ length: 100 }, (_, i) => ({ id: i, name: `Item ${i}` })); + list.renderer = (root, _, { item }) => { + root.textContent = item.name; + }; + await nextFrame(); + }); + + it('should not be focusable by default', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + expect(document.activeElement).to.equal(afterButton); + }); + + it('should select an item programmatically', async () => { + expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.false; + list.selectedItems = [list.items![0]]; + await nextFrame(); + expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.true; + }); + + describe('selectable', () => { + beforeEach(async () => { + list.selectionMode = 'multi'; + await nextFrame(); + }); + + it('should be focusable', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + expect(list.contains(document.activeElement)).to.be.true; + }); + + it('should be unfocusable forwards', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'Tab' }); + expect(document.activeElement).to.equal(afterButton); + }); + + it('should not scroll when tabbing trough', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'Tab' }); + await nextFrame(); + expect(list.firstVisibleIndex).to.equal(0); + }); + + it('should be unfocusable backwards', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + + await shiftTab(); + expect(document.activeElement).to.equal(beforeButton); + }); + + it('should select an item by clicking', async () => { + expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.false; + await click(getRenderedItem(0)!); + await nextFrame(); + expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.true; + }); + + it('should select an item with keyboard', async () => { + expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.false; + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'Space' }); + expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.true; + }); + + it('should select multiple items with keyboard', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'Space' }); + await sendKeys({ press: 'ArrowDown' }); + await sendKeys({ press: 'Space' }); + await nextFrame(); + expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.true; + }); + + it('should unselect an item by clicking again', async () => { + await click(getRenderedItem(0)!); + await click(getRenderedItem(0)!); + await nextFrame(); + expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.false; + }); + + it('should select multiple items by clicking', async () => { + await click(getRenderedItem(0)!); + await click(getRenderedItem(1)!); + await nextFrame(); + expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.true; + expect(getRenderedItem(1)!.hasAttribute('selected')).to.be.true; + }); + + it('should make the clicked item focusable', async () => { + await click(getRenderedItem(1)!); + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + expect(document.activeElement).to.equal(getRenderedItem(1)); + }); + + it('should select items by identity', async () => { + list.itemIdPath = 'id'; + list.selectedItems = [{ id: 0, name: 'Item 0' }]; + await nextFrame(); + expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.true; + }); + + it('should scroll the focused item into view', async () => { + list.scrollToIndex(100); + expect(list.firstVisibleIndex).not.to.equal(0); + + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + + expect(list.contains(document.activeElement)).to.be.true; + expect(list.firstVisibleIndex).to.equal(0); + }); + + it('should not try to focus an index below 0', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'ArrowUp' }); + expect(document.activeElement).to.equal(getRenderedItem(0)); + }); + + it('should not try to focus an index beyond items.length', async () => { + const lastIndex = list.items!.length - 1; + list.scrollToIndex(lastIndex); + await click(getRenderedItem(lastIndex)!); + await sendKeys({ press: 'ArrowDown' }); + expect(document.activeElement).to.equal(getRenderedItem(lastIndex)); + }); + + it('should unselect an item by clicking another item on single-selection mode', async () => { + list.selectionMode = 'single'; + await click(getRenderedItem(0)!); + await click(getRenderedItem(1)!); + expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.false; + expect(getRenderedItem(1)!.hasAttribute('selected')).to.be.true; + }); + + it('should cancel arrow keys default behavior', async () => { + const spy = sinon.spy(); + list.addEventListener('keydown', spy); + + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'ArrowDown' }); + expect(spy.firstCall.args[0].defaultPrevented).to.be.true; + }); + + it('should cancel space keys default behavior', async () => { + const spy = sinon.spy(); + list.addEventListener('keydown', spy); + + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'Space' }); + expect(spy.firstCall.args[0].defaultPrevented).to.be.true; + }); + + it('should mark element focused', async () => { + expect(getRenderedItem(0)!.hasAttribute('focused')).to.be.false; + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + expect(getRenderedItem(0)!.hasAttribute('focused')).to.be.true; + }); + + describe('focusable children', () => { + beforeEach(async () => { + list.renderer = (root, _, { item }) => { + render(html``, root); + }; + await nextFrame(); + }); + + it('should not focus child elements', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'Tab' }); + expect(document.activeElement).to.equal(afterButton); + }); + + it('should focus focusable child element', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'Enter' }); + expect(document.activeElement!.localName).to.equal('button'); + expect(document.activeElement!.parentElement).to.equal(getRenderedItem(0)); + }); + + it('should focus tab to the next focsuable child', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'Enter' }); + await sendKeys({ press: 'Tab' }); + expect(document.activeElement!.localName).to.equal('button'); + expect(document.activeElement!.parentElement).to.equal(getRenderedItem(1)); + }); + + it('should focus the item element on escape', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'Enter' }); + await sendKeys({ press: 'Escape' }); + expect(document.activeElement).to.equal(getRenderedItem(0)); + }); + + it('should tab backwards from a focusable child', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'Enter' }); + + await shiftTab(); + expect(document.activeElement).to.equal(beforeButton); + }); + + it('should focus tab to the next focusable child after clicking a child', async () => { + await click(getRenderedItem(0)!.querySelector('button')!); + await sendKeys({ press: 'Tab' }); + expect(document.activeElement!.parentElement).to.equal(getRenderedItem(1)); + }); + + it('should focus the first item element after blurring from a focusable child', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'Enter' }); + await shiftTab(); + await sendKeys({ press: 'Tab' }); + + expect(document.activeElement).to.equal(getRenderedItem(0)); + }); + + it('should refocus the focused child element', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'Enter' }); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'Escape' }); + await sendKeys({ press: 'Enter' }); + + expect(document.activeElement?.parentElement).to.equal(getRenderedItem(1)); + }); + + it('should not select an item when clicking a child element', async () => { + await click(getRenderedItem(0)!.querySelector('button')!); + expect(list.selectedItems).to.be.empty; + }); + }); + }); +}); diff --git a/packages/virtual-list/theme/lumo/vaadin-lit-virtual-list.js b/packages/virtual-list/theme/lumo/vaadin-lit-virtual-list.js index 6ef6374f04..76986dfdee 100644 --- a/packages/virtual-list/theme/lumo/vaadin-lit-virtual-list.js +++ b/packages/virtual-list/theme/lumo/vaadin-lit-virtual-list.js @@ -1 +1,2 @@ +import './vaadin-virtual-list-styles.js'; import '../../src/vaadin-lit-virtual-list.js'; diff --git a/packages/virtual-list/theme/lumo/vaadin-virtual-list-styles.js b/packages/virtual-list/theme/lumo/vaadin-virtual-list-styles.js new file mode 100644 index 0000000000..e7288e4f71 --- /dev/null +++ b/packages/virtual-list/theme/lumo/vaadin-virtual-list-styles.js @@ -0,0 +1,18 @@ +import '@vaadin/vaadin-lumo-styles/color.js'; +import { css, registerStyles } from '@vaadin/vaadin-themable-mixin'; + +const virtualListStyles = css` + ::slotted(*) { + outline: none; + } + + :host([navigating]) ::slotted([focused]) { + box-shadow: inset 0 0 0 2px var(--lumo-primary-color-50pct); + } + + ::slotted([selected]) { + background-color: var(--lumo-primary-color-10pct); + } +`; + +registerStyles('vaadin-virtual-list', virtualListStyles, { moduleId: 'lumo-virtual-list' }); diff --git a/packages/virtual-list/theme/lumo/vaadin-virtual-list.js b/packages/virtual-list/theme/lumo/vaadin-virtual-list.js index 1e22fffeb8..845d2b28da 100644 --- a/packages/virtual-list/theme/lumo/vaadin-virtual-list.js +++ b/packages/virtual-list/theme/lumo/vaadin-virtual-list.js @@ -1 +1,2 @@ +import './vaadin-virtual-list-styles.js'; import '../../src/vaadin-virtual-list.js'; From 02e00f07dda41f03eaaf9af72b88173b5ad36746 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Mon, 25 Nov 2024 17:30:22 +0200 Subject: [PATCH 02/49] test: add an integration test --- .../vaadin-virtual-list-selection-mixin.js | 3 ++ test/integration/dialog-virtual-list.test.js | 52 +++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 test/integration/dialog-virtual-list.test.js diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index 10785059cc..0eccaebe45 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -197,6 +197,9 @@ export const SelectionMixin = (superClass) => this.__onNavigationTabKey(e.shiftKey); } } else if (e.key === 'Escape' && !e.defaultPrevented) { + // Prevent closing a dialog etc. when returning to navigation mode on Escape + e.preventDefault(); + e.stopPropagation(); this.__getRootElementWithFocus().focus(); } } diff --git a/test/integration/dialog-virtual-list.test.js b/test/integration/dialog-virtual-list.test.js new file mode 100644 index 0000000000..f81cbdb8c1 --- /dev/null +++ b/test/integration/dialog-virtual-list.test.js @@ -0,0 +1,52 @@ +import { expect } from '@vaadin/chai-plugins'; +import { click, fixtureSync, nextRender, oneEvent } from '@vaadin/testing-helpers'; +import { sendKeys } from '@web/test-runner-commands'; +import '@vaadin/dialog'; +import '@vaadin/virtual-list'; +describe('virtual-list in dialog', () => { + let dialog, overlay, list; + + beforeEach(async () => { + dialog = fixtureSync(``); + dialog.renderer = (root) => { + if (!root.firstChild) { + root.$.overlay.style.width = '700px'; + root.innerHTML = ` + + `; + const list = root.querySelector('vaadin-virtual-list'); + list.items = [{ text: 'Hello 1' }, { text: 'Hello 2' }]; + list.renderer = (root, _list, { item }) => { + if (!root.firstElementChild) { + root.innerHTML = ``; + } + }; + } + }; + await nextRender(); + dialog.opened = true; + overlay = dialog.$.overlay; + await oneEvent(overlay, 'vaadin-overlay-open'); + list = overlay.querySelector('vaadin-virtual-list'); + }); + + it('should close the dialog on esc', async () => { + list.firstElementChild.focus(); + click(list.firstElementChild); + await sendKeys({ press: 'Escape' }); + expect(dialog.opened).to.be.false; + }); + + it('should not close the dialog on esc when returning to navigation mode', async () => { + const button = list.firstElementChild.querySelector('button'); + // Enter interaction mode by clicking a button directly + button.focus(); + click(button); + // Return to navigation mode + await sendKeys({ press: 'Escape' }); + expect(dialog.opened).to.be.true; + // Close the dialog + await sendKeys({ press: 'Escape' }); + expect(dialog.opened).to.be.false; + }); +}); From 1287b6cf06e619c89f00eecd0ae0ad4e0414a845 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Mon, 25 Nov 2024 17:38:01 +0200 Subject: [PATCH 03/49] test: more tests and fixes --- .../vaadin-virtual-list-selection-mixin.js | 12 ++++-- .../test/virtual-list-selection.common.ts | 39 +++++++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index 0eccaebe45..0a9d7f3fbd 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -87,7 +87,7 @@ export const SelectionMixin = (superClass) => el.toggleAttribute( 'focused', - this.selectionMode && this.__focusIndex === index && el.contains(document.activeElement), + !!this.selectionMode && this.__focusIndex === index && el.contains(document.activeElement), ); } @@ -170,7 +170,11 @@ export const SelectionMixin = (superClass) => /** @private */ __setNavigating(navigating) { - this.tabIndex = this.selectionMode && navigating ? 0 : -1; + if (this.selectionMode && navigating) { + this.tabIndex = 0; + } else { + this.removeAttribute('tabindex'); + } this.$.focusexit.hidden = !this.selectionMode || !navigating; this.toggleAttribute('navigating', navigating); this.toggleAttribute('interacting', !navigating); @@ -267,11 +271,11 @@ export const SelectionMixin = (superClass) => } /** @private */ - __onFocusOut() { + __onFocusOut(e) { if (!this.selectionMode) { return; } - if (!this.contains(document.activeElement)) { + if (!this.contains(e.relatedTarget)) { // If the focus leaves the virtual list, restore navigating state this.__setNavigating(true); } diff --git a/packages/virtual-list/test/virtual-list-selection.common.ts b/packages/virtual-list/test/virtual-list-selection.common.ts index 61054ceab2..645b4ca543 100644 --- a/packages/virtual-list/test/virtual-list-selection.common.ts +++ b/packages/virtual-list/test/virtual-list-selection.common.ts @@ -119,6 +119,25 @@ describe('selection', () => { expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.true; }); + it('should update selectedItems array', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'Space' }); + await sendKeys({ press: 'ArrowDown' }); + await sendKeys({ press: 'Space' }); + await nextFrame(); + expect(list.selectedItems).to.deep.equal([list.items![0], list.items![1]]); + }); + + it('should dispatch selected-items-changed event', async () => { + const spy = sinon.spy(); + list.addEventListener('selected-items-changed', spy); + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'Space' }); + expect(spy.calledOnce).to.be.true; + }); + it('should unselect an item by clicking again', async () => { await click(getRenderedItem(0)!); await click(getRenderedItem(0)!); @@ -209,6 +228,12 @@ describe('selection', () => { expect(getRenderedItem(0)!.hasAttribute('focused')).to.be.true; }); + it('should throw on enter when there are no focusable child elements', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'Enter' }); + }); + describe('focusable children', () => { beforeEach(async () => { list.renderer = (root, _, { item }) => { @@ -289,6 +314,20 @@ describe('selection', () => { await click(getRenderedItem(0)!.querySelector('button')!); expect(list.selectedItems).to.be.empty; }); + + it('should tab trough focusable children when selection mode is unset', async () => { + list.selectionMode = undefined; + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + expect(document.activeElement!.parentElement).to.equal(getRenderedItem(0)); + }); + + it('should not have focused items when selection mode is unset', async () => { + list.selectionMode = undefined; + list.scrollToIndex(10); + await nextFrame(); + expect(list.querySelector('[focused]')).to.be.null; + }); }); }); }); From 5273a0a0779b3af193eb3d680562a1d606c12299 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Wed, 27 Nov 2024 09:08:13 +0200 Subject: [PATCH 04/49] fix: do not try to select undefined items --- dev/virtual-list.html | 16 +++++++++++++++ .../vaadin-virtual-list-selection-mixin.js | 20 +++++++++++-------- .../test/virtual-list-selection.common.ts | 16 +++++++++++---- 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/dev/virtual-list.html b/dev/virtual-list.html index 7aaeab7f1c..307f869c65 100644 --- a/dev/virtual-list.html +++ b/dev/virtual-list.html @@ -31,6 +31,16 @@ list.selectionMode = 'multi'; list.selectedItems = [items[0]]; list.itemIdPath = 'label'; + + const selectionMode = document.querySelector('#selection-mode'); + selectionMode.value = list.selectionMode; + selectionMode.addEventListener('change', (e) => { + if (e.target.value === 'none') { + list.selectionMode = null; + } else { + list.selectionMode = e.target.value; + } + }); @@ -38,5 +48,11 @@ + + diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index 0a9d7f3fbd..2c441dbcac 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -81,6 +81,7 @@ export const SelectionMixin = (superClass) => __updateElement(el, index) { const item = this.items[index]; el.__item = item; + el.__index = index; el.toggleAttribute('selected', this.__isSelected(item)); el.tabIndex = this.__isNavigating() && this.selectionMode && this.__focusIndex === index ? 0 : -1; @@ -124,12 +125,20 @@ export const SelectionMixin = (superClass) => /** @private */ __getItemFromEvent(e) { - const element = e.composedPath().find((el) => el.parentElement === this); + const element = this.__getRootElementFromEvent(e); return element ? element.__item : null; } + /** @private */ + __getRootElementFromEvent(e) { + return e.composedPath().find((el) => el.parentElement === this); + } + /** @private */ __toggleSelection(item) { + if (item === undefined) { + return; + } if (this.__isSelected(item)) { this.selectedItems = this.selectedItems.filter((selectedItem) => !this.__itemsEqual(selectedItem, item)); } else if (this.selectionMode === 'multi') { @@ -150,7 +159,7 @@ export const SelectionMixin = (superClass) => /** @private */ __getRenderedFocusIndexElement() { - return [...this.children].find((el) => this.__getItemIndex(el.__item) === this.__focusIndex); + return [...this.children].find((el) => el.__index === this.__focusIndex); } /** @private */ @@ -163,11 +172,6 @@ export const SelectionMixin = (superClass) => return this.hasAttribute('navigating'); } - /** @private */ - __getItemIndex(item) { - return this.items.indexOf(item); - } - /** @private */ __setNavigating(navigating) { if (this.selectionMode && navigating) { @@ -261,7 +265,7 @@ export const SelectionMixin = (superClass) => // Update focus index based on the focused item const targetItem = this.__getItemFromEvent(e); if (targetItem) { - this.__focusIndex = this.__getItemIndex(targetItem); + this.__focusIndex = this.__getRootElementFromEvent(e).__index; } // Focus the root element matching focus index if focus came from outside diff --git a/packages/virtual-list/test/virtual-list-selection.common.ts b/packages/virtual-list/test/virtual-list-selection.common.ts index 645b4ca543..bf13491589 100644 --- a/packages/virtual-list/test/virtual-list-selection.common.ts +++ b/packages/virtual-list/test/virtual-list-selection.common.ts @@ -5,7 +5,7 @@ import sinon from 'sinon'; import { html, render } from 'lit'; import type { VirtualList } from '../vaadin-virtual-list'; -type TestItem = { id: number; name: string }; +type TestItem = { id: number; name: string } | undefined; async function click(el: HTMLElement) { el.focus(); @@ -26,7 +26,7 @@ describe('selection', () => { function getRenderedItem(index: number) { const childElements = Array.from(list.children) as Array; - return childElements.find((child) => !child.hidden && child.__item.id === index); + return childElements.find((child) => !child.hidden && child.__item?.id === index); } beforeEach(async () => { @@ -41,7 +41,7 @@ describe('selection', () => { list.style.height = '200px'; list.items = Array.from({ length: 100 }, (_, i) => ({ id: i, name: `Item ${i}` })); list.renderer = (root, _, { item }) => { - root.textContent = item.name; + root.textContent = item?.name || 'undefined'; }; await nextFrame(); }); @@ -109,6 +109,14 @@ describe('selection', () => { expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.true; }); + it('should ignore selection of undefined items (Flow VirtualList)', async () => { + list.items = [undefined]; + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'Space' }); + expect(list.selectedItems).to.be.empty; + }); + it('should select multiple items with keyboard', async () => { beforeButton.focus(); await sendKeys({ press: 'Tab' }); @@ -237,7 +245,7 @@ describe('selection', () => { describe('focusable children', () => { beforeEach(async () => { list.renderer = (root, _, { item }) => { - render(html``, root); + render(html``, root); }; await nextFrame(); }); From 9892d7a3d409eff602ac145dbc5c052f744d1be6 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Fri, 29 Nov 2024 08:15:04 +0200 Subject: [PATCH 05/49] cleanup --- packages/virtual-list/test/virtual-list-selection.common.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/virtual-list/test/virtual-list-selection.common.ts b/packages/virtual-list/test/virtual-list-selection.common.ts index bf13491589..5d5b7a02f4 100644 --- a/packages/virtual-list/test/virtual-list-selection.common.ts +++ b/packages/virtual-list/test/virtual-list-selection.common.ts @@ -9,8 +9,8 @@ type TestItem = { id: number; name: string } | undefined; async function click(el: HTMLElement) { el.focus(); - // TODO: No await? - await helperClick(el); + helperClick(el); + await nextFrame(); } async function shiftTab() { From 374e6907ac8ddff407794ab3bce390bba6cead84 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Mon, 2 Dec 2024 12:35:30 +0200 Subject: [PATCH 06/49] fix: scroll the focused index to view on space --- .../vaadin-virtual-list-selection-mixin.js | 24 ++++++++++++------- .../test/virtual-list-selection.common.ts | 9 +++++++ 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index 2c441dbcac..08159ba4dc 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -72,7 +72,7 @@ export const SelectionMixin = (superClass) => super(); this.addEventListener('keydown', (e) => this.__onKeyDown(e)); - this.addEventListener('click', (e) => this.__onClick(e)); + this.addEventListener('click', () => this.__onClick()); this.addEventListener('focusin', (e) => this.__onFocusIn(e)); this.addEventListener('focusout', (e) => this.__onFocusOut(e)); } @@ -149,10 +149,15 @@ export const SelectionMixin = (superClass) => } /** @private */ - __focusElementWithFocusIndex() { + __ensureFocusedIndexInView() { if (!this.__getRenderedFocusIndexElement()) { this.scrollToIndex(this.__focusIndex); } + } + + /** @private */ + __focusElementWithFocusIndex() { + this.__ensureFocusedIndexInView(); this.__getRenderedFocusIndexElement().focus(); this.requestContentUpdate(); } @@ -200,7 +205,7 @@ export const SelectionMixin = (superClass) => this.__onNavigationEnterKey(); } else if (e.key === ' ') { e.preventDefault(); - this.__toggleSelection(this.__getItemFromEvent(e)); + this.__onNavigationSpaceKey(); } else if (e.key === 'Tab') { this.__onNavigationTabKey(e.shiftKey); } @@ -229,6 +234,12 @@ export const SelectionMixin = (superClass) => } } + /** @private */ + __onNavigationSpaceKey() { + this.__ensureFocusedIndexInView(); + this.__toggleSelection(this.__getRenderedFocusIndexElement().__item); + } + /** @private */ __onNavigationEnterKey() { // Get the focused item @@ -241,15 +252,12 @@ export const SelectionMixin = (superClass) => } /** @private */ - __onClick(e) { + __onClick() { if (!this.selectionMode || !this.__isNavigating()) { return; } - const item = this.__getItemFromEvent(e); - if (item) { - this.__toggleSelection(item); - } + this.__toggleSelection(this.__getRenderedFocusIndexElement().__item); } /** @private */ diff --git a/packages/virtual-list/test/virtual-list-selection.common.ts b/packages/virtual-list/test/virtual-list-selection.common.ts index 5d5b7a02f4..70b05884e0 100644 --- a/packages/virtual-list/test/virtual-list-selection.common.ts +++ b/packages/virtual-list/test/virtual-list-selection.common.ts @@ -109,6 +109,15 @@ describe('selection', () => { expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.true; }); + it('should scroll back to the focused item when selecting an item with keyboard', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + list.scrollToIndex(100); + await sendKeys({ press: 'Space' }); + expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.true; + expect(list.selectedItems).to.deep.equal([list.items![0]]); + }); + it('should ignore selection of undefined items (Flow VirtualList)', async () => { list.items = [undefined]; beforeButton.focus(); From 57f6dfd876a4aa7b4b278d0ef4a1bca34fa13d9c Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Mon, 2 Dec 2024 12:49:57 +0200 Subject: [PATCH 07/49] refactor: scroll to focused index --- .../vaadin-virtual-list-selection-mixin.js | 9 ++++++- .../test/virtual-list-selection.common.ts | 27 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index 08159ba4dc..3cd9502471 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -152,6 +152,14 @@ export const SelectionMixin = (superClass) => __ensureFocusedIndexInView() { if (!this.__getRenderedFocusIndexElement()) { this.scrollToIndex(this.__focusIndex); + } else { + const listRect = this.getBoundingClientRect(); + const elementRect = this.__getRenderedFocusIndexElement().getBoundingClientRect(); + if (elementRect.top < listRect.top) { + this.scrollTop -= listRect.top - elementRect.top; + } else if (elementRect.bottom > listRect.bottom) { + this.scrollTop += elementRect.bottom - listRect.bottom; + } } } @@ -159,7 +167,6 @@ export const SelectionMixin = (superClass) => __focusElementWithFocusIndex() { this.__ensureFocusedIndexInView(); this.__getRenderedFocusIndexElement().focus(); - this.requestContentUpdate(); } /** @private */ diff --git a/packages/virtual-list/test/virtual-list-selection.common.ts b/packages/virtual-list/test/virtual-list-selection.common.ts index 70b05884e0..0f46d57a6b 100644 --- a/packages/virtual-list/test/virtual-list-selection.common.ts +++ b/packages/virtual-list/test/virtual-list-selection.common.ts @@ -251,6 +251,33 @@ describe('selection', () => { await sendKeys({ press: 'Enter' }); }); + it('should ensure focused index in viewport when navigating down with arrow keys', async () => { + const lastVisibleIndex = list.lastVisibleIndex; + await click(getRenderedItem(lastVisibleIndex)!); + await sendKeys({ press: 'ArrowDown' }); + await nextFrame(); + const newLastVisibleIndex = list.lastVisibleIndex; + expect(newLastVisibleIndex).to.equal(lastVisibleIndex + 1); + + const listRect = list.getBoundingClientRect(); + const lastVisibleItemRect = getRenderedItem(newLastVisibleIndex)!.getBoundingClientRect(); + expect(lastVisibleItemRect.bottom).to.equal(listRect.bottom); + }); + + it('should ensure focused index in viewport when navigating up with arrow keys', async () => { + list.scrollToIndex(list.items!.length - 1); + const firstVisibleIndex = list.firstVisibleIndex; + await click(getRenderedItem(firstVisibleIndex)!); + await sendKeys({ press: 'ArrowUp' }); + await nextFrame(); + const newfirstVisibleIndex = list.firstVisibleIndex; + expect(newfirstVisibleIndex).to.equal(firstVisibleIndex - 1); + + const listRect = list.getBoundingClientRect(); + const firstVisibleItemRect = getRenderedItem(newfirstVisibleIndex)!.getBoundingClientRect(); + expect(firstVisibleItemRect.top).to.equal(listRect.top); + }); + describe('focusable children', () => { beforeEach(async () => { list.renderer = (root, _, { item }) => { From 8033aceb32055a45625e932ec569fd291bb2a0a5 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Mon, 2 Dec 2024 13:05:15 +0200 Subject: [PATCH 08/49] feat: add selected in renderer model --- .../src/vaadin-virtual-list-mixin.d.ts | 1 + .../virtual-list/src/vaadin-virtual-list-mixin.js | 13 ++++++++++--- .../src/vaadin-virtual-list-selection-mixin.js | 10 ++++++++++ .../test/lit-renderer-directives.common.js | 1 + .../test/typings/virtual-list.types.ts | 1 + .../test/virtual-list-selection.common.ts | 14 ++++++++++++-- 6 files changed, 35 insertions(+), 5 deletions(-) diff --git a/packages/virtual-list/src/vaadin-virtual-list-mixin.d.ts b/packages/virtual-list/src/vaadin-virtual-list-mixin.d.ts index 54b7209ff1..1adf7fe99e 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-mixin.d.ts +++ b/packages/virtual-list/src/vaadin-virtual-list-mixin.d.ts @@ -12,6 +12,7 @@ export type VirtualListDefaultItem = any; export interface VirtualListItemModel { index: number; item: TItem; + selected?: boolean; } export type VirtualListRenderer = ( diff --git a/packages/virtual-list/src/vaadin-virtual-list-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-mixin.js index d966f5d42c..62930d4598 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-mixin.js @@ -129,15 +129,22 @@ export const VirtualListMixin = (superClass) => } if (this.renderer) { - this.renderer(el, this, this.__getItemModel(this.items[index], index)); + const model = {}; + this.__updateItemModel(model, this.items[index], index); + + this.renderer(el, this, model); } } /** * @private */ - __getItemModel(item, index) { - return { item, index }; + __updateItemModel(model, item, index) { + model.item = this.items[index]; + model.index = index; + if (super.__updateItemModel) { + super.__updateItemModel(model, item, index); + } } /** diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index 3cd9502471..cc98324108 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -92,6 +92,16 @@ export const SelectionMixin = (superClass) => ); } + /** + * @private + */ + __updateItemModel(model, item, index) { + model.selected = this.__isSelected(item); + if (super.__updateItemModel) { + super.__updateItemModel(model, item, index); + } + } + /** @private */ __selectionModeChanged() { this.__setNavigating(true); diff --git a/packages/virtual-list/test/lit-renderer-directives.common.js b/packages/virtual-list/test/lit-renderer-directives.common.js index 8a919bb713..ec4fe842f4 100644 --- a/packages/virtual-list/test/lit-renderer-directives.common.js +++ b/packages/virtual-list/test/lit-renderer-directives.common.js @@ -71,6 +71,7 @@ describe('lit renderer directive', () => { expect(rendererSpy.firstCall.args[1]).to.deep.equal({ item: 'Item', index: 0, + selected: false, }); }); diff --git a/packages/virtual-list/test/typings/virtual-list.types.ts b/packages/virtual-list/test/typings/virtual-list.types.ts index 7575fcfd7a..d5df36d329 100644 --- a/packages/virtual-list/test/typings/virtual-list.types.ts +++ b/packages/virtual-list/test/typings/virtual-list.types.ts @@ -31,6 +31,7 @@ virtualList.renderer = (root, virtualList, model) => { assertType(virtualList); assertType>(model); assertType(model.index); + assertType(model.selected); assertType(model.item); }; diff --git a/packages/virtual-list/test/virtual-list-selection.common.ts b/packages/virtual-list/test/virtual-list-selection.common.ts index 0f46d57a6b..3e73fb2cbe 100644 --- a/packages/virtual-list/test/virtual-list-selection.common.ts +++ b/packages/virtual-list/test/virtual-list-selection.common.ts @@ -40,8 +40,8 @@ describe('selection', () => { list.style.height = '200px'; list.items = Array.from({ length: 100 }, (_, i) => ({ id: i, name: `Item ${i}` })); - list.renderer = (root, _, { item }) => { - root.textContent = item?.name || 'undefined'; + list.renderer = (root, _, { item, selected }) => { + root.textContent = `${item?.name} ${selected ? 'selected' : ''}`; }; await nextFrame(); }); @@ -278,6 +278,16 @@ describe('selection', () => { expect(firstVisibleItemRect.top).to.equal(listRect.top); }); + it('should re-render items on selection', async () => { + expect(getRenderedItem(0)?.textContent).to.equal('Item 0 '); + list.selectedItems = [list.items![0]]; + await nextFrame(); + expect(getRenderedItem(0)?.textContent).to.equal('Item 0 selected'); + list.selectedItems = []; + await nextFrame(); + expect(getRenderedItem(0)?.textContent).to.equal('Item 0 '); + }); + describe('focusable children', () => { beforeEach(async () => { list.renderer = (root, _, { item }) => { From 2516e3973ef9b20ff8a3c96a9a258cd8ac5004c8 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Mon, 2 Dec 2024 16:50:57 +0200 Subject: [PATCH 09/49] feat: aria semantics --- .../vaadin-virtual-list-selection-mixin.js | 16 ++++++ .../test/virtual-list-selection.common.ts | 56 +++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index cc98324108..00e4a18c83 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -77,6 +77,11 @@ export const SelectionMixin = (superClass) => this.addEventListener('focusout', (e) => this.__onFocusOut(e)); } + ready() { + super.ready(); + this.__updateAria(); + } + /** @private */ __updateElement(el, index) { const item = this.items[index]; @@ -85,6 +90,10 @@ export const SelectionMixin = (superClass) => el.toggleAttribute('selected', this.__isSelected(item)); el.tabIndex = this.__isNavigating() && this.selectionMode && this.__focusIndex === index ? 0 : -1; + el.role = this.selectionMode ? 'option' : 'listitem'; + el.ariaSelected = this.selectionMode ? this.__isSelected(item) : null; + el.ariaSetSize = this.items.length; + el.ariaPosInSet = index + 1; el.toggleAttribute( 'focused', @@ -106,6 +115,13 @@ export const SelectionMixin = (superClass) => __selectionModeChanged() { this.__setNavigating(true); this.requestContentUpdate(); + this.__updateAria(); + } + + /** @private */ + __updateAria() { + this.role = this.selectionMode ? 'listbox' : 'list'; + this.ariaMultiSelectable = this.selectionMode === 'multi' ? 'true' : null; } /** @private */ diff --git a/packages/virtual-list/test/virtual-list-selection.common.ts b/packages/virtual-list/test/virtual-list-selection.common.ts index 3e73fb2cbe..ad3e18610c 100644 --- a/packages/virtual-list/test/virtual-list-selection.common.ts +++ b/packages/virtual-list/test/virtual-list-selection.common.ts @@ -384,4 +384,60 @@ describe('selection', () => { }); }); }); + + describe('a11y', () => { + it('should have role="list"', () => { + expect(list.role).to.equal('list'); + }); + + it('should have items with role="listitem"', () => { + expect(getRenderedItem(0)!.role).to.equal('listitem'); + }); + + it('should have items without aria-selected', () => { + expect(getRenderedItem(0)!.ariaSelected).to.be.null; + }); + + it('should assign aria-setsize and aria-posinset', () => { + list.scrollToIndex(list.items!.length - 1); + const lastVisibleIndex = list.lastVisibleIndex; + expect(getRenderedItem(lastVisibleIndex)!.ariaSetSize).to.equal('100'); + expect(getRenderedItem(lastVisibleIndex)!.ariaPosInSet).to.equal('100'); + }); + + describe('selectable', () => { + beforeEach(async () => { + list.selectionMode = 'multi'; + await nextFrame(); + }); + + it('should have role="listbox"', () => { + expect(list.role).to.equal('listbox'); + }); + + it('should have items with role="option"', () => { + expect(getRenderedItem(0)!.role).to.equal('option'); + }); + + it('should aria-selected="false" on non-selected items', () => { + expect(getRenderedItem(0)!.ariaSelected).to.equal('false'); + }); + + it('should aria-selected="true" on selected items', async () => { + list.selectedItems = [list.items![0]]; + await nextFrame(); + expect(getRenderedItem(0)!.ariaSelected).to.equal('true'); + }); + + it('should have aria-multiselectable="true"', () => { + expect(list.ariaMultiSelectable).to.equal('true'); + }); + + it('should not have aria-multiselectable when selectionMode is "single"', async () => { + list.selectionMode = 'single'; + await nextFrame(); + expect(list.ariaMultiSelectable).to.be.null; + }); + }); + }); }); From e33511e38d794d842a15880d868e587bc017556f Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Tue, 3 Dec 2024 09:41:23 +0200 Subject: [PATCH 10/49] feat: add itemAccessibleNameGenerator --- dev/virtual-list.html | 2 ++ .../vaadin-virtual-list-selection-mixin.d.ts | 5 ++++ .../vaadin-virtual-list-selection-mixin.js | 11 ++++++++- .../test/typings/virtual-list.types.ts | 2 ++ .../test/virtual-list-selection.common.ts | 23 ++++++++++++++----- 5 files changed, 36 insertions(+), 7 deletions(-) diff --git a/dev/virtual-list.html b/dev/virtual-list.html index 307f869c65..f09e60f282 100644 --- a/dev/virtual-list.html +++ b/dev/virtual-list.html @@ -28,6 +28,8 @@ list.items = items; list.renderer = renderer; + list.itemAccessibleNameGenerator = (item) => 'Virtual ' + item.label; + list.selectionMode = 'multi'; list.selectedItems = [items[0]]; list.itemIdPath = 'label'; diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.d.ts b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.d.ts index 1b3d6691cb..49497871a5 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.d.ts +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.d.ts @@ -26,4 +26,9 @@ export declare class VirtualListSelectionMixinClass string; } diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index 00e4a18c83..f335bfee78 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -44,6 +44,14 @@ export const SelectionMixin = (superClass) => sync: true, }, + /** + * A function that generates accessible names for virtual list items. + */ + itemAccessibleNameGenerator: { + type: Function, + sync: true, + }, + /** * Set of selected item ids * @private @@ -65,7 +73,7 @@ export const SelectionMixin = (superClass) => } static get observers() { - return ['__selectedItemsChanged(itemIdPath, selectedItems, __focusIndex)']; + return ['__selectedItemsChanged(itemIdPath, selectedItems, __focusIndex, itemAccessibleNameGenerator)']; } constructor() { @@ -99,6 +107,7 @@ export const SelectionMixin = (superClass) => 'focused', !!this.selectionMode && this.__focusIndex === index && el.contains(document.activeElement), ); + el.ariaLabel = this.itemAccessibleNameGenerator ? this.itemAccessibleNameGenerator(item) : null; } /** diff --git a/packages/virtual-list/test/typings/virtual-list.types.ts b/packages/virtual-list/test/typings/virtual-list.types.ts index d5df36d329..6c6fd95d5a 100644 --- a/packages/virtual-list/test/typings/virtual-list.types.ts +++ b/packages/virtual-list/test/typings/virtual-list.types.ts @@ -45,3 +45,5 @@ assertType(virtualList.lastVisibleIndex); assertType<'single' | 'multi' | undefined>(virtualList.selectionMode); assertType(virtualList.selectedItems); assertType(virtualList.itemIdPath); + +assertType<((item: TestVirtualListItem) => string) | undefined>(virtualList.itemAccessibleNameGenerator); diff --git a/packages/virtual-list/test/virtual-list-selection.common.ts b/packages/virtual-list/test/virtual-list-selection.common.ts index ad3e18610c..d02f3fd95e 100644 --- a/packages/virtual-list/test/virtual-list-selection.common.ts +++ b/packages/virtual-list/test/virtual-list-selection.common.ts @@ -78,7 +78,7 @@ describe('selection', () => { expect(document.activeElement).to.equal(afterButton); }); - it('should not scroll when tabbing trough', async () => { + it('should not scroll when tabbing through', async () => { beforeButton.focus(); await sendKeys({ press: 'Tab' }); await sendKeys({ press: 'Tab' }); @@ -270,11 +270,11 @@ describe('selection', () => { await click(getRenderedItem(firstVisibleIndex)!); await sendKeys({ press: 'ArrowUp' }); await nextFrame(); - const newfirstVisibleIndex = list.firstVisibleIndex; - expect(newfirstVisibleIndex).to.equal(firstVisibleIndex - 1); + const newFirstVisibleIndex = list.firstVisibleIndex; + expect(newFirstVisibleIndex).to.equal(firstVisibleIndex - 1); const listRect = list.getBoundingClientRect(); - const firstVisibleItemRect = getRenderedItem(newfirstVisibleIndex)!.getBoundingClientRect(); + const firstVisibleItemRect = getRenderedItem(newFirstVisibleIndex)!.getBoundingClientRect(); expect(firstVisibleItemRect.top).to.equal(listRect.top); }); @@ -311,7 +311,7 @@ describe('selection', () => { expect(document.activeElement!.parentElement).to.equal(getRenderedItem(0)); }); - it('should focus tab to the next focsuable child', async () => { + it('should focus tab to the next focusable child', async () => { beforeButton.focus(); await sendKeys({ press: 'Tab' }); await sendKeys({ press: 'Enter' }); @@ -369,7 +369,7 @@ describe('selection', () => { expect(list.selectedItems).to.be.empty; }); - it('should tab trough focusable children when selection mode is unset', async () => { + it('should tab through focusable children when selection mode is unset', async () => { list.selectionMode = undefined; beforeButton.focus(); await sendKeys({ press: 'Tab' }); @@ -405,6 +405,17 @@ describe('selection', () => { expect(getRenderedItem(lastVisibleIndex)!.ariaPosInSet).to.equal('100'); }); + it('should generate aria-label to the items', () => { + list.itemAccessibleNameGenerator = (item) => `Accessible ${item?.name}`; + expect(getRenderedItem(0)!.ariaLabel).to.equal('Accessible Item 0'); + }); + + it('should remove aria-label from the items', () => { + list.itemAccessibleNameGenerator = (item) => `Accessible ${item?.name}`; + list.itemAccessibleNameGenerator = undefined; + expect(getRenderedItem(0)!.ariaLabel).to.be.null; + }); + describe('selectable', () => { beforeEach(async () => { list.selectionMode = 'multi'; From a5492070e7e1f093358bb5f9aa5a4b5e820ea993 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Tue, 3 Dec 2024 12:51:46 +0200 Subject: [PATCH 11/49] fix: remove navigating state on selection mode unset --- .../vaadin-virtual-list-selection-mixin.js | 4 +-- .../test/virtual-list-selection.common.ts | 29 +++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index f335bfee78..76c9878651 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -227,8 +227,8 @@ export const SelectionMixin = (superClass) => this.removeAttribute('tabindex'); } this.$.focusexit.hidden = !this.selectionMode || !navigating; - this.toggleAttribute('navigating', navigating); - this.toggleAttribute('interacting', !navigating); + this.toggleAttribute('navigating', this.selectionMode && navigating); + this.toggleAttribute('interacting', this.selectionMode && !navigating); this.requestContentUpdate(); } diff --git a/packages/virtual-list/test/virtual-list-selection.common.ts b/packages/virtual-list/test/virtual-list-selection.common.ts index d02f3fd95e..6882651109 100644 --- a/packages/virtual-list/test/virtual-list-selection.common.ts +++ b/packages/virtual-list/test/virtual-list-selection.common.ts @@ -296,6 +296,12 @@ describe('selection', () => { await nextFrame(); }); + it('should be in navigating state', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + expect(list.hasAttribute('navigating')).to.be.true; + }); + it('should not focus child elements', async () => { beforeButton.focus(); await sendKeys({ press: 'Tab' }); @@ -311,6 +317,13 @@ describe('selection', () => { expect(document.activeElement!.parentElement).to.equal(getRenderedItem(0)); }); + it('should be in interacting state', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'Enter' }); + expect(list.hasAttribute('interacting')).to.be.true; + }); + it('should focus tab to the next focusable child', async () => { beforeButton.focus(); await sendKeys({ press: 'Tab' }); @@ -328,6 +341,14 @@ describe('selection', () => { expect(document.activeElement).to.equal(getRenderedItem(0)); }); + it('should return to navigating state', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'Enter' }); + await sendKeys({ press: 'Escape' }); + expect(list.hasAttribute('navigating')).to.be.true; + }); + it('should tab backwards from a focusable child', async () => { beforeButton.focus(); await sendKeys({ press: 'Tab' }); @@ -382,6 +403,14 @@ describe('selection', () => { await nextFrame(); expect(list.querySelector('[focused]')).to.be.null; }); + + it('should clear navigating state when selection mode is unset', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + list.selectionMode = undefined; + await nextFrame(); + expect(list.hasAttribute('navigating')).to.be.false; + }); }); }); From 734d6e120768d1efee1bcb727abd55b1b9897a6b Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Tue, 3 Dec 2024 14:09:41 +0200 Subject: [PATCH 12/49] refactor: cleanup and test coverage --- dev/virtual-list.html | 13 +++ .../vaadin-virtual-list-selection-mixin.js | 79 ++++++++++++------- .../test/virtual-list-selection.common.ts | 78 ++++++++++++++++++ 3 files changed, 143 insertions(+), 27 deletions(-) diff --git a/dev/virtual-list.html b/dev/virtual-list.html index f09e60f282..5ca1058624 100644 --- a/dev/virtual-list.html +++ b/dev/virtual-list.html @@ -43,6 +43,16 @@ list.selectionMode = e.target.value; } }); + + document.querySelector('#empty').addEventListener('click', () => { + list.items = []; + }); + + document.querySelector('#set-1000-items').addEventListener('click', () => { + list.items = Array.from({ length: 1000 }).map((_, i) => { + return { label: `Item ${i}` }; + }); + }); @@ -56,5 +66,8 @@ + + + diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index 76c9878651..d2af0c6634 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -62,6 +62,7 @@ export const SelectionMixin = (superClass) => }, /** + * The index of the focused item. * @private */ __focusIndex: { @@ -73,7 +74,10 @@ export const SelectionMixin = (superClass) => } static get observers() { - return ['__selectedItemsChanged(itemIdPath, selectedItems, __focusIndex, itemAccessibleNameGenerator)']; + return [ + '__selectionChanged(itemIdPath, selectedItems, __focusIndex, itemAccessibleNameGenerator)', + '__normalizeFocusIndex(items)', + ]; } constructor() { @@ -90,7 +94,10 @@ export const SelectionMixin = (superClass) => this.__updateAria(); } - /** @private */ + /** + * @private + * @override + */ __updateElement(el, index) { const item = this.items[index]; el.__item = item; @@ -112,19 +119,17 @@ export const SelectionMixin = (superClass) => /** * @private + * @override */ - __updateItemModel(model, item, index) { + __updateItemModel(model, item) { + // Include "selected" property in the model passed to the renderer model.selected = this.__isSelected(item); - if (super.__updateItemModel) { - super.__updateItemModel(model, item, index); - } } /** @private */ __selectionModeChanged() { - this.__setNavigating(true); - this.requestContentUpdate(); this.__updateAria(); + this.__setNavigating(true); } /** @private */ @@ -133,13 +138,22 @@ export const SelectionMixin = (superClass) => this.ariaMultiSelectable = this.selectionMode === 'multi' ? 'true' : null; } + /** @private */ + __normalizeFocusIndex() { + // Needs to run in a microtask, otherwise the change to __focusIndex would synchronously invoke + // __updateElement for items that are not yet in sync with virtualizer + queueMicrotask(() => { + this.__focusIndex = Math.min(this.__focusIndex, this.items.length - 1); + }); + } + /** @private */ __isSelected(item) { return this.__selectedKeys.has(this.__getItemId(item)); } /** @private */ - __selectedItemsChanged() { + __selectionChanged() { this.requestContentUpdate(); } @@ -158,27 +172,19 @@ export const SelectionMixin = (superClass) => return this.itemIdPath ? get(this.itemIdPath, item) : item; } - /** @private */ - __getItemFromEvent(e) { - const element = this.__getRootElementFromEvent(e); - return element ? element.__item : null; - } - - /** @private */ - __getRootElementFromEvent(e) { - return e.composedPath().find((el) => el.parentElement === this); - } - /** @private */ __toggleSelection(item) { if (item === undefined) { return; } if (this.__isSelected(item)) { + // Item deselected, remove it from selected items this.selectedItems = this.selectedItems.filter((selectedItem) => !this.__itemsEqual(selectedItem, item)); } else if (this.selectionMode === 'multi') { + // Item selected, add it to selected items this.selectedItems = [...this.selectedItems, item]; } else { + // Single selection mode, replace the selected item this.selectedItems = [item]; } } @@ -186,8 +192,10 @@ export const SelectionMixin = (superClass) => /** @private */ __ensureFocusedIndexInView() { if (!this.__getRenderedFocusIndexElement()) { + // The focused element is not rendered, scroll to the focused index this.scrollToIndex(this.__focusIndex); } else { + // The focused element is rendered. If it's not inside the visible area, scroll to it const listRect = this.getBoundingClientRect(); const elementRect = this.__getRenderedFocusIndexElement().getBoundingClientRect(); if (elementRect.top < listRect.top) { @@ -205,13 +213,24 @@ export const SelectionMixin = (superClass) => } /** @private */ + __getRenderedRootElements() { + return [...this.children].filter((el) => !el.hidden); + } + + /** + * Returns the rendered root element with the current focus index. + * @private + */ __getRenderedFocusIndexElement() { - return [...this.children].find((el) => el.__index === this.__focusIndex); + return this.__getRenderedRootElements().find((el) => el.__index === this.__focusIndex); } - /** @private */ + /** + * Returns the rendered root element which contains focus. + * @private + */ __getRootElementWithFocus() { - return [...this.children].find((el) => el.contains(document.activeElement)); + return this.__getRenderedRootElements().find((el) => el.contains(document.activeElement)); } /** @private */ @@ -243,7 +262,6 @@ export const SelectionMixin = (superClass) => e.preventDefault(); this.__onNavigationArrowKey(e.key === 'ArrowDown'); } else if (e.key === 'Enter') { - e.preventDefault(); this.__onNavigationEnterKey(); } else if (e.key === ' ') { e.preventDefault(); @@ -268,8 +286,13 @@ export const SelectionMixin = (superClass) => /** @private */ __onNavigationTabKey(shift) { if (shift) { + // Focus the virtual list itself when shift-tabbing so the focus actually ends + // up on the previous element in the tab order before the virtual list this.focus(); } else { + // Focus the focus exit element when tabbing so the focus actually ends up on + // the next element in the tab order after the virtual list. + // Focusing the focus exit element causes scroll top to get reset, so we need to save and restore it const scrollTop = this.scrollTop; this.$.focusexit.focus(); this.scrollTop = scrollTop; @@ -278,6 +301,7 @@ export const SelectionMixin = (superClass) => /** @private */ __onNavigationSpaceKey() { + // Ensure the focused item is in view before toggling selection this.__ensureFocusedIndexInView(); this.__toggleSelection(this.__getRenderedFocusIndexElement().__item); } @@ -309,13 +333,14 @@ export const SelectionMixin = (superClass) => } // Set navigating state if one of the root elements, virtual-list or focusexit, is focused + // Set interacting state otherwise (child element is focused) const navigating = [...this.children, this, this.$.focusexit].includes(e.target); this.__setNavigating(navigating); // Update focus index based on the focused item - const targetItem = this.__getItemFromEvent(e); - if (targetItem) { - this.__focusIndex = this.__getRootElementFromEvent(e).__index; + const rootElement = this.__getRootElementWithFocus(); + if (rootElement) { + this.__focusIndex = rootElement.__index; } // Focus the root element matching focus index if focus came from outside diff --git a/packages/virtual-list/test/virtual-list-selection.common.ts b/packages/virtual-list/test/virtual-list-selection.common.ts index 6882651109..34c9f61ed2 100644 --- a/packages/virtual-list/test/virtual-list-selection.common.ts +++ b/packages/virtual-list/test/virtual-list-selection.common.ts @@ -59,6 +59,51 @@ describe('selection', () => { expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.true; }); + it('should not re-render when focused', async () => { + const rendererSpy = sinon.spy(list.renderer!); + list.renderer = rendererSpy; + await nextFrame(); + + rendererSpy.resetHistory(); + const firstItem = getRenderedItem(0)!; + firstItem.tabIndex = 0; + firstItem.focus(); + + expect(rendererSpy.called).to.be.false; + }); + + it('should not re-render when blurred', async () => { + const rendererSpy = sinon.spy(list.renderer!); + list.renderer = rendererSpy; + await nextFrame(); + const firstItem = getRenderedItem(0)!; + firstItem.tabIndex = 0; + firstItem.focus(); + + rendererSpy.resetHistory(); + await sendKeys({ press: 'Tab' }); + + expect(rendererSpy.called).to.be.false; + }); + + it('should not mark element focused', () => { + const firstItem = getRenderedItem(0)!; + firstItem.tabIndex = 0; + firstItem.focus(); + list.requestContentUpdate(); + expect(getRenderedItem(0)!.hasAttribute('focused')).to.be.false; + }); + + it('should not cancel Escape key default behavior', async () => { + const spy = sinon.spy(); + list.addEventListener('keydown', spy); + const firstItem = getRenderedItem(0)!; + firstItem.tabIndex = 0; + firstItem.focus(); + await sendKeys({ press: 'Escape' }); + expect(spy.firstCall.args[0].defaultPrevented).to.be.false; + }); + describe('selectable', () => { beforeEach(async () => { list.selectionMode = 'multi'; @@ -210,6 +255,17 @@ describe('selection', () => { expect(document.activeElement).to.equal(getRenderedItem(lastIndex)); }); + it('should focus the last visible index when items length reduced', async () => { + const lastIndex = list.items!.length - 1; + list.scrollToIndex(lastIndex); + await click(getRenderedItem(lastIndex)!); + + list.items = list.items!.slice(0, -1); + await nextFrame(); + expect(document.activeElement).to.equal(getRenderedItem(lastIndex - 1)); + expect(getRenderedItem(lastIndex - 1)!.hasAttribute('focused')).to.be.true; + }); + it('should unselect an item by clicking another item on single-selection mode', async () => { list.selectionMode = 'single'; await click(getRenderedItem(0)!); @@ -288,6 +344,18 @@ describe('selection', () => { expect(getRenderedItem(0)?.textContent).to.equal('Item 0 '); }); + it('should re-render items once on selection', async () => { + const rendererSpy = sinon.spy(list.renderer!); + list.renderer = rendererSpy; + await nextFrame(); + + rendererSpy.resetHistory(); + list.selectedItems = [list.items![0]]; + await nextFrame(); + + expect(rendererSpy.getCalls().filter((call) => call.args[2].index === 0).length).to.equal(1); + }); + describe('focusable children', () => { beforeEach(async () => { list.renderer = (root, _, { item }) => { @@ -445,6 +513,16 @@ describe('selection', () => { expect(getRenderedItem(0)!.ariaLabel).to.be.null; }); + it('should not invoke itemAccessibleNameGenerator when items are emptied ', async () => { + const spy = sinon.spy((item) => `Accessible ${item?.name}`); + list.itemAccessibleNameGenerator = spy; + await nextFrame(); + + spy.resetHistory(); + list.items = []; + expect(spy.called).to.be.false; + }); + describe('selectable', () => { beforeEach(async () => { list.selectionMode = 'multi'; From 3d78d3f605f74c1bb7e2358719858a1d9aec1d85 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Tue, 3 Dec 2024 16:47:42 +0200 Subject: [PATCH 13/49] fix: bugfixes --- .../vaadin-virtual-list-selection-mixin.js | 39 ++++++++++++----- .../test/virtual-list-selection.common.ts | 43 ++++++++++++++++++- .../virtual-list-date-picker.test.js | 35 +++++++++++++++ 3 files changed, 106 insertions(+), 11 deletions(-) create mode 100644 test/integration/virtual-list-date-picker.test.js diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index d2af0c6634..acfa4ae776 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -76,7 +76,7 @@ export const SelectionMixin = (superClass) => static get observers() { return [ '__selectionChanged(itemIdPath, selectedItems, __focusIndex, itemAccessibleNameGenerator)', - '__normalizeFocusIndex(items)', + '__selectionItemsUpdated(items)', ]; } @@ -84,7 +84,7 @@ export const SelectionMixin = (superClass) => super(); this.addEventListener('keydown', (e) => this.__onKeyDown(e)); - this.addEventListener('click', () => this.__onClick()); + this.addEventListener('click', (e) => this.__onClick(e)); this.addEventListener('focusin', (e) => this.__onFocusIn(e)); this.addEventListener('focusout', (e) => this.__onFocusOut(e)); } @@ -139,11 +139,12 @@ export const SelectionMixin = (superClass) => } /** @private */ - __normalizeFocusIndex() { + __selectionItemsUpdated() { // Needs to run in a microtask, otherwise the change to __focusIndex would synchronously invoke // __updateElement for items that are not yet in sync with virtualizer queueMicrotask(() => { - this.__focusIndex = Math.min(this.__focusIndex, this.items.length - 1); + this.__focusIndex = Math.max(Math.min(this.__focusIndex, this.items.length - 1), 0); + this.__setNavigating(this.__isNavigating()); }); } @@ -233,6 +234,14 @@ export const SelectionMixin = (superClass) => return this.__getRenderedRootElements().find((el) => el.contains(document.activeElement)); } + /** + * Returns the rendered root element containing the given child element. + * @private + */ + __getRootElementByChild(element) { + return this.__getRenderedRootElements().find((el) => el.contains(element)); + } + /** @private */ __isNavigating() { return this.hasAttribute('navigating'); @@ -240,12 +249,12 @@ export const SelectionMixin = (superClass) => /** @private */ __setNavigating(navigating) { - if (this.selectionMode && navigating) { + if (this.selectionMode && navigating && this.items && this.items.length > 0) { this.tabIndex = 0; } else { this.removeAttribute('tabindex'); } - this.$.focusexit.hidden = !this.selectionMode || !navigating; + this.$.focusexit.hidden = !this.selectionMode || !navigating || !this.items || this.items.length === 0; this.toggleAttribute('navigating', this.selectionMode && navigating); this.toggleAttribute('interacting', this.selectionMode && !navigating); this.requestContentUpdate(); @@ -259,8 +268,10 @@ export const SelectionMixin = (superClass) => if (this.__isNavigating()) { if (e.key === 'ArrowDown' || e.key === 'ArrowUp') { - e.preventDefault(); - this.__onNavigationArrowKey(e.key === 'ArrowDown'); + if (!e.defaultPrevented) { + e.preventDefault(); + this.__onNavigationArrowKey(e.key === 'ArrowDown'); + } } else if (e.key === 'Enter') { this.__onNavigationEnterKey(); } else if (e.key === ' ') { @@ -318,12 +329,20 @@ export const SelectionMixin = (superClass) => } /** @private */ - __onClick() { + __onClick(e) { if (!this.selectionMode || !this.__isNavigating()) { return; } - this.__toggleSelection(this.__getRenderedFocusIndexElement().__item); + if (document.activeElement === this) { + // If the virtual list itself is clicked, focus the root element matching focus index + this.__focusElementWithFocusIndex(); + } + + const clickedRootElement = this.__getRootElementByChild(e.target); + if (clickedRootElement) { + this.__toggleSelection(clickedRootElement.__item); + } } /** @private */ diff --git a/packages/virtual-list/test/virtual-list-selection.common.ts b/packages/virtual-list/test/virtual-list-selection.common.ts index 34c9f61ed2..db9d0a6974 100644 --- a/packages/virtual-list/test/virtual-list-selection.common.ts +++ b/packages/virtual-list/test/virtual-list-selection.common.ts @@ -139,6 +139,26 @@ describe('selection', () => { expect(document.activeElement).to.equal(beforeButton); }); + it('should not focus empty list', async () => { + list.items = []; + await nextFrame(); + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + expect(document.activeElement).to.equal(afterButton); + }); + + it('should focus first item after restoring items', async () => { + const items = list.items; + list.items = []; + await nextFrame(); + list.items = items; + await nextFrame(); + + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + expect(document.activeElement).to.equal(getRenderedItem(0)); + }); + it('should select an item by clicking', async () => { expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.false; await click(getRenderedItem(0)!); @@ -356,6 +376,27 @@ describe('selection', () => { expect(rendererSpy.getCalls().filter((call) => call.args[2].index === 0).length).to.equal(1); }); + it('should not throw on click when there are no items', async () => { + list.items = []; + await nextFrame(); + await click(list); + }); + + it('should focus an item on virtual list click', async () => { + list.items = list.items!.slice(0, 5); + await nextFrame(); + click(list); + click(list); + expect(document.activeElement).to.equal(getRenderedItem(0)); + }); + + it('should not select an item on virtual list click', async () => { + list.items = list.items!.slice(0, 5); + await nextFrame(); + click(list); + expect(list.selectedItems).to.be.empty; + }); + describe('focusable children', () => { beforeEach(async () => { list.renderer = (root, _, { item }) => { @@ -513,7 +554,7 @@ describe('selection', () => { expect(getRenderedItem(0)!.ariaLabel).to.be.null; }); - it('should not invoke itemAccessibleNameGenerator when items are emptied ', async () => { + it('should not invoke itemAccessibleNameGenerator when items are emptied', async () => { const spy = sinon.spy((item) => `Accessible ${item?.name}`); list.itemAccessibleNameGenerator = spy; await nextFrame(); diff --git a/test/integration/virtual-list-date-picker.test.js b/test/integration/virtual-list-date-picker.test.js new file mode 100644 index 0000000000..6e10f2a58e --- /dev/null +++ b/test/integration/virtual-list-date-picker.test.js @@ -0,0 +1,35 @@ +import { expect } from '@vaadin/chai-plugins'; +import { fixtureSync, nextRender } from '@vaadin/testing-helpers'; +import { sendKeys } from '@web/test-runner-commands'; +import '@vaadin/date-picker'; +import '@vaadin/virtual-list'; +describe('date-picker in virtual-list', () => { + let list; + + beforeEach(async () => { + list = fixtureSync(``); + list.items = ['foo', 'bar']; + list.renderer = (root, _list) => { + if (!root.firstElementChild) { + root.innerHTML = ``; + } + }; + await nextRender(); + }); + + it('should not navigate the virtual list items on date picker arrow keys', async () => { + const [firstDatePicker] = list.querySelectorAll('vaadin-date-picker'); + firstDatePicker.focus(); + expect(firstDatePicker.parentElement.hasAttribute('focused')).to.be.true; + + // Select a value for the date picker using arrow keys + await sendKeys({ press: 'Space' }); + await sendKeys({ press: 'ArrowDown' }); + await sendKeys({ press: 'Space' }); + await sendKeys({ press: 'Enter' }); + + // Expect the keyboard interaction to not navigate nor select the virtual list items + expect(list.selectedItems).to.be.empty; + expect(firstDatePicker.parentElement.hasAttribute('focused')).to.be.true; + }); +}); From a9fd9b041cd2feeca76052e0e66e354a58bd2446 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Wed, 4 Dec 2024 09:14:27 +0200 Subject: [PATCH 14/49] refactor: cleanup --- dev/virtual-list.html | 38 ++++++++++++++++++- .../vaadin-virtual-list-selection-mixin.js | 21 +++++----- 2 files changed, 46 insertions(+), 13 deletions(-) diff --git a/dev/virtual-list.html b/dev/virtual-list.html index 5ca1058624..8bb0cba453 100644 --- a/dev/virtual-list.html +++ b/dev/virtual-list.html @@ -9,6 +9,12 @@ @@ -69,5 +102,6 @@ + diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index acfa4ae776..6b300a5ab7 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -74,10 +74,7 @@ export const SelectionMixin = (superClass) => } static get observers() { - return [ - '__selectionChanged(itemIdPath, selectedItems, __focusIndex, itemAccessibleNameGenerator)', - '__selectionItemsUpdated(items)', - ]; + return ['__selectionChanged(itemIdPath, selectedItems, __focusIndex, itemAccessibleNameGenerator)']; } constructor() { @@ -92,6 +89,8 @@ export const SelectionMixin = (superClass) => ready() { super.ready(); this.__updateAria(); + + this._createPropertyObserver('items', '__selectionItemsUpdated'); } /** @@ -139,13 +138,13 @@ export const SelectionMixin = (superClass) => } /** @private */ - __selectionItemsUpdated() { - // Needs to run in a microtask, otherwise the change to __focusIndex would synchronously invoke - // __updateElement for items that are not yet in sync with virtualizer - queueMicrotask(() => { - this.__focusIndex = Math.max(Math.min(this.__focusIndex, this.items.length - 1), 0); - this.__setNavigating(this.__isNavigating()); - }); + __selectionItemsUpdated(items) { + if (!this.selectionMode || !this.items) { + return; + } + + this.__focusIndex = Math.max(Math.min(this.__focusIndex, items.length - 1), 0); + this.__setNavigating(this.__isNavigating()); } /** @private */ From 8e7e837e8fa360b3c326344f00a8b13a65e8c448 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Thu, 5 Dec 2024 09:48:06 +0200 Subject: [PATCH 15/49] refactor: cleanup --- .../vaadin-virtual-list-selection-mixin.js | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index 6b300a5ab7..c8b7427d26 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -128,7 +128,7 @@ export const SelectionMixin = (superClass) => /** @private */ __selectionModeChanged() { this.__updateAria(); - this.__setNavigating(true); + this.__updateNavigating(true); } /** @private */ @@ -144,7 +144,7 @@ export const SelectionMixin = (superClass) => } this.__focusIndex = Math.max(Math.min(this.__focusIndex, items.length - 1), 0); - this.__setNavigating(this.__isNavigating()); + this.__updateNavigating(this.__isNavigating()); } /** @private */ @@ -247,15 +247,21 @@ export const SelectionMixin = (superClass) => } /** @private */ - __setNavigating(navigating) { - if (this.selectionMode && navigating && this.items && this.items.length > 0) { + __updateNavigating(navigating) { + const isNavigating = !!(this.selectionMode && navigating); + this.toggleAttribute('navigating', isNavigating); + + const isInteracting = !!(this.selectionMode && !navigating); + this.toggleAttribute('interacting', isInteracting); + + const isFocusable = !!(isNavigating && this.items && this.items.length); + if (isFocusable) { this.tabIndex = 0; } else { this.removeAttribute('tabindex'); } - this.$.focusexit.hidden = !this.selectionMode || !navigating || !this.items || this.items.length === 0; - this.toggleAttribute('navigating', this.selectionMode && navigating); - this.toggleAttribute('interacting', this.selectionMode && !navigating); + this.$.focusexit.hidden = !isFocusable; + this.requestContentUpdate(); } @@ -353,7 +359,7 @@ export const SelectionMixin = (superClass) => // Set navigating state if one of the root elements, virtual-list or focusexit, is focused // Set interacting state otherwise (child element is focused) const navigating = [...this.children, this, this.$.focusexit].includes(e.target); - this.__setNavigating(navigating); + this.__updateNavigating(navigating); // Update focus index based on the focused item const rootElement = this.__getRootElementWithFocus(); @@ -374,7 +380,7 @@ export const SelectionMixin = (superClass) => } if (!this.contains(e.relatedTarget)) { // If the focus leaves the virtual list, restore navigating state - this.__setNavigating(true); + this.__updateNavigating(true); } } From 2acdbf6845c0b35588164f2bcb1a1e99ba9f41ff Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Thu, 5 Dec 2024 09:57:38 +0200 Subject: [PATCH 16/49] fix an issue with single-selection --- .../src/vaadin-virtual-list-selection-mixin.js | 18 ++++++++++-------- .../test/virtual-list-selection.common.ts | 17 +++++++++++++++++ 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index c8b7427d26..257bde2528 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -177,15 +177,17 @@ export const SelectionMixin = (superClass) => if (item === undefined) { return; } - if (this.__isSelected(item)) { - // Item deselected, remove it from selected items - this.selectedItems = this.selectedItems.filter((selectedItem) => !this.__itemsEqual(selectedItem, item)); + + if (this.selectionMode === 'single') { + this.selectedItems = this.__isSelected(item) ? [] : [item]; } else if (this.selectionMode === 'multi') { - // Item selected, add it to selected items - this.selectedItems = [...this.selectedItems, item]; - } else { - // Single selection mode, replace the selected item - this.selectedItems = [item]; + if (this.__isSelected(item)) { + // Item deselected, remove it from selected items + this.selectedItems = this.selectedItems.filter((selectedItem) => !this.__itemsEqual(selectedItem, item)); + } else { + // Item selected, add it to selected items + this.selectedItems = [...this.selectedItems, item]; + } } } diff --git a/packages/virtual-list/test/virtual-list-selection.common.ts b/packages/virtual-list/test/virtual-list-selection.common.ts index db9d0a6974..68a4598e44 100644 --- a/packages/virtual-list/test/virtual-list-selection.common.ts +++ b/packages/virtual-list/test/virtual-list-selection.common.ts @@ -294,6 +294,23 @@ describe('selection', () => { expect(getRenderedItem(1)!.hasAttribute('selected')).to.be.true; }); + it('should deselect all items when deselecting on single-selection mode', async () => { + list.selectionMode = 'single'; + list.selectedItems = [list.items![0], list.items![1]]; + await click(getRenderedItem(0)!); + expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.false; + expect(getRenderedItem(1)!.hasAttribute('selected')).to.be.false; + }); + + it('should deselect other items when selecting on single-selection mode', async () => { + list.selectionMode = 'single'; + list.selectedItems = [list.items![0], list.items![1]]; + await click(getRenderedItem(2)!); + expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.false; + expect(getRenderedItem(1)!.hasAttribute('selected')).to.be.false; + expect(getRenderedItem(2)!.hasAttribute('selected')).to.be.true; + }); + it('should cancel arrow keys default behavior', async () => { const spy = sinon.spy(); list.addEventListener('keydown', spy); From 26af05c531d15637ec0aa454156cede3b411f572 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Thu, 5 Dec 2024 15:03:24 +0200 Subject: [PATCH 17/49] refactor: change selection mode type --- .../vaadin-virtual-list-selection-mixin.d.ts | 4 +-- .../vaadin-virtual-list-selection-mixin.js | 32 +++++++++++-------- .../test/typings/virtual-list.types.ts | 2 +- .../test/virtual-list-selection.common.ts | 6 ++-- 4 files changed, 25 insertions(+), 19 deletions(-) diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.d.ts b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.d.ts index 49497871a5..8121cb1600 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.d.ts +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.d.ts @@ -12,9 +12,9 @@ export declare function VirtualListSelectionMixin { /** - * Selection mode for the virtual list. Available modes are: `single` and `multi`. + * Selection mode for the virtual list. Available modes are: `none`, `single` and `multi`. */ - selectionMode?: 'single' | 'multi'; + selectionMode: 'none' | 'single' | 'multi'; /** * Path to an item sub-property that identifies the item. diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index 257bde2528..719fa3a3ea 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -15,12 +15,13 @@ export const SelectionMixin = (superClass) => static get properties() { return { /** - * Selection mode for the virtual list. Available modes are: `single` and `multi`. + * Selection mode for the virtual list. Available modes are: `none`, `single` and `multi`. * @type {string} */ selectionMode: { type: String, observer: '__selectionModeChanged', + value: 'none', }, /** @@ -93,6 +94,11 @@ export const SelectionMixin = (superClass) => this._createPropertyObserver('items', '__selectionItemsUpdated'); } + /** @private */ + __hasSelectionMode() { + return this.selectionMode !== 'none'; + } + /** * @private * @override @@ -103,15 +109,15 @@ export const SelectionMixin = (superClass) => el.__index = index; el.toggleAttribute('selected', this.__isSelected(item)); - el.tabIndex = this.__isNavigating() && this.selectionMode && this.__focusIndex === index ? 0 : -1; - el.role = this.selectionMode ? 'option' : 'listitem'; - el.ariaSelected = this.selectionMode ? this.__isSelected(item) : null; + el.tabIndex = this.__isNavigating() && this.__hasSelectionMode() && this.__focusIndex === index ? 0 : -1; + el.role = this.__hasSelectionMode() ? 'option' : 'listitem'; + el.ariaSelected = this.__hasSelectionMode() ? this.__isSelected(item) : null; el.ariaSetSize = this.items.length; el.ariaPosInSet = index + 1; el.toggleAttribute( 'focused', - !!this.selectionMode && this.__focusIndex === index && el.contains(document.activeElement), + this.__hasSelectionMode() && this.__focusIndex === index && el.contains(document.activeElement), ); el.ariaLabel = this.itemAccessibleNameGenerator ? this.itemAccessibleNameGenerator(item) : null; } @@ -133,13 +139,13 @@ export const SelectionMixin = (superClass) => /** @private */ __updateAria() { - this.role = this.selectionMode ? 'listbox' : 'list'; + this.role = this.__hasSelectionMode() ? 'listbox' : 'list'; this.ariaMultiSelectable = this.selectionMode === 'multi' ? 'true' : null; } /** @private */ __selectionItemsUpdated(items) { - if (!this.selectionMode || !this.items) { + if (!this.__hasSelectionMode() || !this.items) { return; } @@ -250,10 +256,10 @@ export const SelectionMixin = (superClass) => /** @private */ __updateNavigating(navigating) { - const isNavigating = !!(this.selectionMode && navigating); + const isNavigating = !!(this.__hasSelectionMode() && navigating); this.toggleAttribute('navigating', isNavigating); - const isInteracting = !!(this.selectionMode && !navigating); + const isInteracting = !!(this.__hasSelectionMode() && !navigating); this.toggleAttribute('interacting', isInteracting); const isFocusable = !!(isNavigating && this.items && this.items.length); @@ -269,7 +275,7 @@ export const SelectionMixin = (superClass) => /** @private */ __onKeyDown(e) { - if (!this.selectionMode) { + if (!this.__hasSelectionMode()) { return; } @@ -337,7 +343,7 @@ export const SelectionMixin = (superClass) => /** @private */ __onClick(e) { - if (!this.selectionMode || !this.__isNavigating()) { + if (!this.__hasSelectionMode() || !this.__isNavigating()) { return; } @@ -354,7 +360,7 @@ export const SelectionMixin = (superClass) => /** @private */ __onFocusIn(e) { - if (!this.selectionMode) { + if (!this.__hasSelectionMode()) { return; } @@ -377,7 +383,7 @@ export const SelectionMixin = (superClass) => /** @private */ __onFocusOut(e) { - if (!this.selectionMode) { + if (!this.__hasSelectionMode()) { return; } if (!this.contains(e.relatedTarget)) { diff --git a/packages/virtual-list/test/typings/virtual-list.types.ts b/packages/virtual-list/test/typings/virtual-list.types.ts index 6c6fd95d5a..0fde3541cc 100644 --- a/packages/virtual-list/test/typings/virtual-list.types.ts +++ b/packages/virtual-list/test/typings/virtual-list.types.ts @@ -42,7 +42,7 @@ assertType<(index: number) => void>(virtualList.scrollToIndex); assertType(virtualList.firstVisibleIndex); assertType(virtualList.lastVisibleIndex); -assertType<'single' | 'multi' | undefined>(virtualList.selectionMode); +assertType<'none' | 'single' | 'multi'>(virtualList.selectionMode); assertType(virtualList.selectedItems); assertType(virtualList.itemIdPath); diff --git a/packages/virtual-list/test/virtual-list-selection.common.ts b/packages/virtual-list/test/virtual-list-selection.common.ts index 68a4598e44..21bb7eebc3 100644 --- a/packages/virtual-list/test/virtual-list-selection.common.ts +++ b/packages/virtual-list/test/virtual-list-selection.common.ts @@ -517,14 +517,14 @@ describe('selection', () => { }); it('should tab through focusable children when selection mode is unset', async () => { - list.selectionMode = undefined; + list.selectionMode = 'none'; beforeButton.focus(); await sendKeys({ press: 'Tab' }); expect(document.activeElement!.parentElement).to.equal(getRenderedItem(0)); }); it('should not have focused items when selection mode is unset', async () => { - list.selectionMode = undefined; + list.selectionMode = 'none'; list.scrollToIndex(10); await nextFrame(); expect(list.querySelector('[focused]')).to.be.null; @@ -533,7 +533,7 @@ describe('selection', () => { it('should clear navigating state when selection mode is unset', async () => { beforeButton.focus(); await sendKeys({ press: 'Tab' }); - list.selectionMode = undefined; + list.selectionMode = 'none'; await nextFrame(); expect(list.hasAttribute('navigating')).to.be.false; }); From e408af5008c9fb50725df42b000cf712214a98bb Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Thu, 5 Dec 2024 15:54:34 +0200 Subject: [PATCH 18/49] refactor: cleanup --- .../vaadin-virtual-list-selection-mixin.js | 40 +++++++++---------- .../test/virtual-list-selection.common.ts | 18 +++++++++ 2 files changed, 38 insertions(+), 20 deletions(-) diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index 719fa3a3ea..3a20b55ea1 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -16,7 +16,7 @@ export const SelectionMixin = (superClass) => return { /** * Selection mode for the virtual list. Available modes are: `none`, `single` and `multi`. - * @type {string} + * @attr {string} selection-mode */ selectionMode: { type: String, @@ -75,7 +75,7 @@ export const SelectionMixin = (superClass) => } static get observers() { - return ['__selectionChanged(itemIdPath, selectedItems, __focusIndex, itemAccessibleNameGenerator)']; + return ['__selectionChanged(itemIdPath, selectedItems, itemAccessibleNameGenerator)']; } constructor() { @@ -89,7 +89,6 @@ export const SelectionMixin = (superClass) => ready() { super.ready(); - this.__updateAria(); this._createPropertyObserver('items', '__selectionItemsUpdated'); } @@ -109,16 +108,14 @@ export const SelectionMixin = (superClass) => el.__index = index; el.toggleAttribute('selected', this.__isSelected(item)); - el.tabIndex = this.__isNavigating() && this.__hasSelectionMode() && this.__focusIndex === index ? 0 : -1; + const isFocusable = this.__isNavigating() && this.__hasSelectionMode() && this.__focusIndex === index; + el.tabIndex = isFocusable ? 0 : -1; el.role = this.__hasSelectionMode() ? 'option' : 'listitem'; el.ariaSelected = this.__hasSelectionMode() ? this.__isSelected(item) : null; el.ariaSetSize = this.items.length; el.ariaPosInSet = index + 1; - el.toggleAttribute( - 'focused', - this.__hasSelectionMode() && this.__focusIndex === index && el.contains(document.activeElement), - ); + el.toggleAttribute('focused', this.__hasSelectionMode() && el.contains(document.activeElement)); el.ariaLabel = this.itemAccessibleNameGenerator ? this.itemAccessibleNameGenerator(item) : null; } @@ -144,12 +141,17 @@ export const SelectionMixin = (superClass) => } /** @private */ - __selectionItemsUpdated(items) { - if (!this.__hasSelectionMode() || !this.items) { + __clampIndex(index) { + return Math.max(0, Math.min(index, (this.items || []).length - 1)); + } + + /** @private */ + __selectionItemsUpdated() { + if (!this.__hasSelectionMode()) { return; } - this.__focusIndex = Math.max(Math.min(this.__focusIndex, items.length - 1), 0); + this.__focusIndex = this.__clampIndex(this.__focusIndex); this.__updateNavigating(this.__isNavigating()); } @@ -245,7 +247,7 @@ export const SelectionMixin = (superClass) => * Returns the rendered root element containing the given child element. * @private */ - __getRootElementByChild(element) { + __getRootElementByContent(element) { return this.__getRenderedRootElements().find((el) => el.contains(element)); } @@ -275,16 +277,14 @@ export const SelectionMixin = (superClass) => /** @private */ __onKeyDown(e) { - if (!this.__hasSelectionMode()) { + if (e.defaultPrevented || !this.__hasSelectionMode()) { return; } if (this.__isNavigating()) { if (e.key === 'ArrowDown' || e.key === 'ArrowUp') { - if (!e.defaultPrevented) { - e.preventDefault(); - this.__onNavigationArrowKey(e.key === 'ArrowDown'); - } + e.preventDefault(); + this.__onNavigationArrowKey(e.key === 'ArrowDown'); } else if (e.key === 'Enter') { this.__onNavigationEnterKey(); } else if (e.key === ' ') { @@ -293,7 +293,7 @@ export const SelectionMixin = (superClass) => } else if (e.key === 'Tab') { this.__onNavigationTabKey(e.shiftKey); } - } else if (e.key === 'Escape' && !e.defaultPrevented) { + } else if (e.key === 'Escape') { // Prevent closing a dialog etc. when returning to navigation mode on Escape e.preventDefault(); e.stopPropagation(); @@ -303,7 +303,7 @@ export const SelectionMixin = (superClass) => /** @private */ __onNavigationArrowKey(down) { - this.__focusIndex = Math.min(Math.max(this.__focusIndex + (down ? 1 : -1), 0), this.items.length - 1); + this.__focusIndex = this.__clampIndex(this.__focusIndex + (down ? 1 : -1)); this.__focusElementWithFocusIndex(); } @@ -352,7 +352,7 @@ export const SelectionMixin = (superClass) => this.__focusElementWithFocusIndex(); } - const clickedRootElement = this.__getRootElementByChild(e.target); + const clickedRootElement = this.__getRootElementByContent(e.target); if (clickedRootElement) { this.__toggleSelection(clickedRootElement.__item); } diff --git a/packages/virtual-list/test/virtual-list-selection.common.ts b/packages/virtual-list/test/virtual-list-selection.common.ts index 21bb7eebc3..2cc5e7da4d 100644 --- a/packages/virtual-list/test/virtual-list-selection.common.ts +++ b/packages/virtual-list/test/virtual-list-selection.common.ts @@ -249,6 +249,14 @@ describe('selection', () => { expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.true; }); + it('should select items by identity when identity changed dynamically', async () => { + list.selectedItems = [{ id: 0, name: 'Item 0' }]; + await nextFrame(); + list.itemIdPath = 'id'; + await nextFrame(); + expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.true; + }); + it('should scroll the focused item into view', async () => { list.scrollToIndex(100); expect(list.firstVisibleIndex).not.to.equal(0); @@ -338,6 +346,12 @@ describe('selection', () => { expect(getRenderedItem(0)!.hasAttribute('focused')).to.be.true; }); + it('should not mark unfocused element focused', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + expect(getRenderedItem(1)!.hasAttribute('focused')).to.be.false; + }); + it('should throw on enter when there are no focusable child elements', async () => { beforeButton.focus(); await sendKeys({ press: 'Tab' }); @@ -414,6 +428,10 @@ describe('selection', () => { expect(list.selectedItems).to.be.empty; }); + it('should not throw is items are unset', () => { + list.items = undefined; + }); + describe('focusable children', () => { beforeEach(async () => { list.renderer = (root, _, { item }) => { From a407c4a5a95b46de3a5b8830d78b885a38c5cb13 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Fri, 6 Dec 2024 13:13:35 +0200 Subject: [PATCH 19/49] refactor: cleanup --- .../vaadin-virtual-list-selection-mixin.js | 61 ++++++++++--------- 1 file changed, 31 insertions(+), 30 deletions(-) diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index 3a20b55ea1..34ec7c6886 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -5,7 +5,7 @@ */ import { getFocusableElements } from '@vaadin/a11y-base'; -import { get } from '@vaadin/component-base/src/path-utils'; +import { get } from '@vaadin/component-base/src/path-utils.js'; /** * @polymerMixin @@ -39,7 +39,7 @@ export const SelectionMixin = (superClass) => * @type {!Array} */ selectedItems: { - type: Object, + type: Array, notify: true, value: () => [], sync: true, @@ -67,7 +67,7 @@ export const SelectionMixin = (superClass) => * @private */ __focusIndex: { - type: Object, + type: Number, value: 0, sync: true, }, @@ -94,7 +94,7 @@ export const SelectionMixin = (superClass) => } /** @private */ - __hasSelectionMode() { + __isSelectable() { return this.selectionMode !== 'none'; } @@ -108,14 +108,14 @@ export const SelectionMixin = (superClass) => el.__index = index; el.toggleAttribute('selected', this.__isSelected(item)); - const isFocusable = this.__isNavigating() && this.__hasSelectionMode() && this.__focusIndex === index; + const isFocusable = this.__isNavigating() && this.__isSelectable() && this.__focusIndex === index; el.tabIndex = isFocusable ? 0 : -1; - el.role = this.__hasSelectionMode() ? 'option' : 'listitem'; - el.ariaSelected = this.__hasSelectionMode() ? this.__isSelected(item) : null; + el.role = this.__isSelectable() ? 'option' : 'listitem'; + el.ariaSelected = this.__isSelectable() ? this.__isSelected(item) : null; el.ariaSetSize = this.items.length; el.ariaPosInSet = index + 1; - el.toggleAttribute('focused', this.__hasSelectionMode() && el.contains(document.activeElement)); + el.toggleAttribute('focused', this.__isSelectable() && el.contains(document.activeElement)); el.ariaLabel = this.itemAccessibleNameGenerator ? this.itemAccessibleNameGenerator(item) : null; } @@ -136,7 +136,7 @@ export const SelectionMixin = (superClass) => /** @private */ __updateAria() { - this.role = this.__hasSelectionMode() ? 'listbox' : 'list'; + this.role = this.__isSelectable() ? 'listbox' : 'list'; this.ariaMultiSelectable = this.selectionMode === 'multi' ? 'true' : null; } @@ -147,12 +147,13 @@ export const SelectionMixin = (superClass) => /** @private */ __selectionItemsUpdated() { - if (!this.__hasSelectionMode()) { + if (!this.__isSelectable()) { return; } this.__focusIndex = this.__clampIndex(this.__focusIndex); - this.__updateNavigating(this.__isNavigating()); + // Items may have been emptied, need to update focusability + this.__updateFocusable(); } /** @private */ @@ -189,25 +190,21 @@ export const SelectionMixin = (superClass) => if (this.selectionMode === 'single') { this.selectedItems = this.__isSelected(item) ? [] : [item]; } else if (this.selectionMode === 'multi') { - if (this.__isSelected(item)) { - // Item deselected, remove it from selected items - this.selectedItems = this.selectedItems.filter((selectedItem) => !this.__itemsEqual(selectedItem, item)); - } else { - // Item selected, add it to selected items - this.selectedItems = [...this.selectedItems, item]; - } + this.selectedItems = this.__isSelected(item) + ? this.selectedItems.filter((selectedItem) => !this.__itemsEqual(selectedItem, item)) + : [...this.selectedItems, item]; } } - /** @private */ __ensureFocusedIndexInView() { - if (!this.__getRenderedFocusIndexElement()) { + const focusElement = this.__getRenderedFocusIndexElement(); + if (!focusElement) { // The focused element is not rendered, scroll to the focused index this.scrollToIndex(this.__focusIndex); } else { // The focused element is rendered. If it's not inside the visible area, scroll to it const listRect = this.getBoundingClientRect(); - const elementRect = this.__getRenderedFocusIndexElement().getBoundingClientRect(); + const elementRect = focusElement.getBoundingClientRect(); if (elementRect.top < listRect.top) { this.scrollTop -= listRect.top - elementRect.top; } else if (elementRect.bottom > listRect.bottom) { @@ -258,26 +255,30 @@ export const SelectionMixin = (superClass) => /** @private */ __updateNavigating(navigating) { - const isNavigating = !!(this.__hasSelectionMode() && navigating); + const isNavigating = !!(this.__isSelectable() && navigating); this.toggleAttribute('navigating', isNavigating); - const isInteracting = !!(this.__hasSelectionMode() && !navigating); + const isInteracting = !!(this.__isSelectable() && !navigating); this.toggleAttribute('interacting', isInteracting); - const isFocusable = !!(isNavigating && this.items && this.items.length); + this.__updateFocusable(); + this.requestContentUpdate(); + } + + /** @private */ + __updateFocusable() { + const isFocusable = !!(this.__isNavigating() && this.items && this.items.length); if (isFocusable) { this.tabIndex = 0; } else { this.removeAttribute('tabindex'); } this.$.focusexit.hidden = !isFocusable; - - this.requestContentUpdate(); } /** @private */ __onKeyDown(e) { - if (e.defaultPrevented || !this.__hasSelectionMode()) { + if (e.defaultPrevented || !this.__isSelectable()) { return; } @@ -343,7 +344,7 @@ export const SelectionMixin = (superClass) => /** @private */ __onClick(e) { - if (!this.__hasSelectionMode() || !this.__isNavigating()) { + if (!this.__isSelectable() || !this.__isNavigating()) { return; } @@ -360,7 +361,7 @@ export const SelectionMixin = (superClass) => /** @private */ __onFocusIn(e) { - if (!this.__hasSelectionMode()) { + if (!this.__isSelectable()) { return; } @@ -383,7 +384,7 @@ export const SelectionMixin = (superClass) => /** @private */ __onFocusOut(e) { - if (!this.__hasSelectionMode()) { + if (!this.__isSelectable()) { return; } if (!this.contains(e.relatedTarget)) { From 1854b36161e995bd20a914623ee7687a3767b6f3 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Mon, 9 Dec 2024 09:44:54 +0200 Subject: [PATCH 20/49] fix: do not mark element focused if it does not match focus index --- .../src/vaadin-virtual-list-selection-mixin.js | 7 ++++++- .../virtual-list/test/virtual-list-selection.common.ts | 8 ++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index 34ec7c6886..e13aa292c4 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -110,12 +110,13 @@ export const SelectionMixin = (superClass) => el.toggleAttribute('selected', this.__isSelected(item)); const isFocusable = this.__isNavigating() && this.__isSelectable() && this.__focusIndex === index; el.tabIndex = isFocusable ? 0 : -1; + el.toggleAttribute('focused', isFocusable && el.contains(document.activeElement)); + el.role = this.__isSelectable() ? 'option' : 'listitem'; el.ariaSelected = this.__isSelectable() ? this.__isSelected(item) : null; el.ariaSetSize = this.items.length; el.ariaPosInSet = index + 1; - el.toggleAttribute('focused', this.__isSelectable() && el.contains(document.activeElement)); el.ariaLabel = this.itemAccessibleNameGenerator ? this.itemAccessibleNameGenerator(item) : null; } @@ -151,7 +152,11 @@ export const SelectionMixin = (superClass) => return; } + const oldFocusIndex = this.__focusIndex; this.__focusIndex = this.__clampIndex(this.__focusIndex); + if (oldFocusIndex !== this.__focusIndex) { + this.requestContentUpdate(); + } // Items may have been emptied, need to update focusability this.__updateFocusable(); } diff --git a/packages/virtual-list/test/virtual-list-selection.common.ts b/packages/virtual-list/test/virtual-list-selection.common.ts index 2cc5e7da4d..996c8d7c34 100644 --- a/packages/virtual-list/test/virtual-list-selection.common.ts +++ b/packages/virtual-list/test/virtual-list-selection.common.ts @@ -352,6 +352,14 @@ describe('selection', () => { expect(getRenderedItem(1)!.hasAttribute('focused')).to.be.false; }); + it('should not mark element focused if it does not match virtual focus index', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + list.scrollToIndex(list.items!.length - 1); + + expect([...list.children].some((child) => child.hasAttribute('focused'))).to.be.false; + }); + it('should throw on enter when there are no focusable child elements', async () => { beforeButton.focus(); await sendKeys({ press: 'Tab' }); From c626bcb1320d86572cb9ff99e4071da0b630f2d7 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Mon, 9 Dec 2024 11:55:35 +0200 Subject: [PATCH 21/49] refactor: avoid excess re-renders on click select --- .../vaadin-virtual-list-selection-mixin.js | 19 ++++++++++++++++--- .../test/virtual-list-selection.common.ts | 19 ++++++++++++++++++- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index e13aa292c4..bd437fc194 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -5,6 +5,8 @@ */ import { getFocusableElements } from '@vaadin/a11y-base'; +import { timeOut } from '@vaadin/component-base/src/async'; +import { Debouncer } from '@vaadin/component-base/src/debounce.js'; import { get } from '@vaadin/component-base/src/path-utils.js'; /** @@ -155,7 +157,7 @@ export const SelectionMixin = (superClass) => const oldFocusIndex = this.__focusIndex; this.__focusIndex = this.__clampIndex(this.__focusIndex); if (oldFocusIndex !== this.__focusIndex) { - this.requestContentUpdate(); + this.__scheduleRequestContentUpdate(); } // Items may have been emptied, need to update focusability this.__updateFocusable(); @@ -168,7 +170,7 @@ export const SelectionMixin = (superClass) => /** @private */ __selectionChanged() { - this.requestContentUpdate(); + this.__scheduleRequestContentUpdate(); } /** @private */ @@ -267,7 +269,7 @@ export const SelectionMixin = (superClass) => this.toggleAttribute('interacting', isInteracting); this.__updateFocusable(); - this.requestContentUpdate(); + this.__scheduleRequestContentUpdate(); } /** @private */ @@ -313,6 +315,17 @@ export const SelectionMixin = (superClass) => this.__focusElementWithFocusIndex(); } + /** @private */ + __scheduleRequestContentUpdate() { + this.__debounceRequestContentUpdate = Debouncer.debounce( + this.__debounceRequestContentUpdate, + timeOut.after(0), + () => { + this.requestContentUpdate(); + }, + ); + } + /** @private */ __onNavigationTabKey(shift) { if (shift) { diff --git a/packages/virtual-list/test/virtual-list-selection.common.ts b/packages/virtual-list/test/virtual-list-selection.common.ts index 996c8d7c34..7afc9efd6d 100644 --- a/packages/virtual-list/test/virtual-list-selection.common.ts +++ b/packages/virtual-list/test/virtual-list-selection.common.ts @@ -9,6 +9,9 @@ type TestItem = { id: number; name: string } | undefined; async function click(el: HTMLElement) { el.focus(); + await new Promise((resolve) => { + queueMicrotask(() => resolve()); + }); helperClick(el); await nextFrame(); } @@ -166,6 +169,18 @@ describe('selection', () => { expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.true; }); + it('should re-render items once on click select', async () => { + const rendererSpy = sinon.spy(list.renderer!); + list.renderer = rendererSpy; + await nextFrame(); + + rendererSpy.resetHistory(); + await click(getRenderedItem(0)!); + await nextFrame(); + + expect(rendererSpy.getCalls().filter((call) => call.args[2].index === 0).length).to.equal(1); + }); + it('should select an item with keyboard', async () => { expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.false; beforeButton.focus(); @@ -426,6 +441,7 @@ describe('selection', () => { await nextFrame(); click(list); click(list); + await nextFrame(); expect(document.activeElement).to.equal(getRenderedItem(0)); }); @@ -586,8 +602,9 @@ describe('selection', () => { expect(getRenderedItem(lastVisibleIndex)!.ariaPosInSet).to.equal('100'); }); - it('should generate aria-label to the items', () => { + it('should generate aria-label to the items', async () => { list.itemAccessibleNameGenerator = (item) => `Accessible ${item?.name}`; + await nextFrame(); expect(getRenderedItem(0)!.ariaLabel).to.equal('Accessible Item 0'); }); From 7e2eb5b7dcf5e66e3e5edbcd2b73775e691727d1 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Mon, 9 Dec 2024 12:09:48 +0200 Subject: [PATCH 22/49] fix: do not rerender when tabbing focusable children of same parent --- .../vaadin-virtual-list-selection-mixin.js | 4 +++- .../test/virtual-list-selection.common.ts | 20 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index bd437fc194..c7514bf705 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -386,7 +386,9 @@ export const SelectionMixin = (superClass) => // Set navigating state if one of the root elements, virtual-list or focusexit, is focused // Set interacting state otherwise (child element is focused) const navigating = [...this.children, this, this.$.focusexit].includes(e.target); - this.__updateNavigating(navigating); + if (navigating || this.__isNavigating()) { + this.__updateNavigating(navigating); + } // Update focus index based on the focused item const rootElement = this.__getRootElementWithFocus(); diff --git a/packages/virtual-list/test/virtual-list-selection.common.ts b/packages/virtual-list/test/virtual-list-selection.common.ts index 7afc9efd6d..5edf38c877 100644 --- a/packages/virtual-list/test/virtual-list-selection.common.ts +++ b/packages/virtual-list/test/virtual-list-selection.common.ts @@ -501,6 +501,26 @@ describe('selection', () => { expect(document.activeElement!.parentElement).to.equal(getRenderedItem(1)); }); + it('should not re-render when tabbing between focusable children inside a single parent', async () => { + // Create a renderer with two focusable children inside a single parent + const rendererSpy = sinon.spy((root, _, { item }) => { + render(html``, root); + }); + list.renderer = rendererSpy; + await nextFrame(); + + // Enter interacting state + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'Enter' }); + await nextFrame(); + rendererSpy.resetHistory(); + + // Tab to the next focusable child inside the same parent + await sendKeys({ press: 'Tab' }); + expect(rendererSpy.called).to.be.false; + }); + it('should focus the item element on escape', async () => { beforeButton.focus(); await sendKeys({ press: 'Tab' }); From d16de3d5763a4e8857d3bf3c3080c951ff4acd10 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Mon, 9 Dec 2024 12:40:18 +0200 Subject: [PATCH 23/49] docs: clarify the need for focus exit --- .../virtual-list/src/vaadin-virtual-list-selection-mixin.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index c7514bf705..40089b1737 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -331,10 +331,11 @@ export const SelectionMixin = (superClass) => if (shift) { // Focus the virtual list itself when shift-tabbing so the focus actually ends // up on the previous element in the tab order before the virtual list + // instead of some focusable child on another row. this.focus(); } else { // Focus the focus exit element when tabbing so the focus actually ends up on - // the next element in the tab order after the virtual list. + // the next element in the tab order after the virtual list instead of some focusable child on another row. // Focusing the focus exit element causes scroll top to get reset, so we need to save and restore it const scrollTop = this.scrollTop; this.$.focusexit.focus(); From 5a9e6f998410c466735e0868a0bb956b878fac19 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Tue, 10 Dec 2024 12:45:21 +0200 Subject: [PATCH 24/49] refactor: remove theme changes for now --- dev/virtual-list.html | 11 +++++++++++ .../theme/lumo/vaadin-lit-virtual-list.js | 1 - .../theme/lumo/vaadin-virtual-list-styles.js | 18 ------------------ .../theme/lumo/vaadin-virtual-list.js | 1 - 4 files changed, 11 insertions(+), 20 deletions(-) delete mode 100644 packages/virtual-list/theme/lumo/vaadin-virtual-list-styles.js diff --git a/dev/virtual-list.html b/dev/virtual-list.html index 8bb0cba453..b3bf476ddc 100644 --- a/dev/virtual-list.html +++ b/dev/virtual-list.html @@ -87,6 +87,17 @@ }); }); + + diff --git a/packages/virtual-list/theme/lumo/vaadin-lit-virtual-list.js b/packages/virtual-list/theme/lumo/vaadin-lit-virtual-list.js index 76986dfdee..6ef6374f04 100644 --- a/packages/virtual-list/theme/lumo/vaadin-lit-virtual-list.js +++ b/packages/virtual-list/theme/lumo/vaadin-lit-virtual-list.js @@ -1,2 +1 @@ -import './vaadin-virtual-list-styles.js'; import '../../src/vaadin-lit-virtual-list.js'; diff --git a/packages/virtual-list/theme/lumo/vaadin-virtual-list-styles.js b/packages/virtual-list/theme/lumo/vaadin-virtual-list-styles.js deleted file mode 100644 index e7288e4f71..0000000000 --- a/packages/virtual-list/theme/lumo/vaadin-virtual-list-styles.js +++ /dev/null @@ -1,18 +0,0 @@ -import '@vaadin/vaadin-lumo-styles/color.js'; -import { css, registerStyles } from '@vaadin/vaadin-themable-mixin'; - -const virtualListStyles = css` - ::slotted(*) { - outline: none; - } - - :host([navigating]) ::slotted([focused]) { - box-shadow: inset 0 0 0 2px var(--lumo-primary-color-50pct); - } - - ::slotted([selected]) { - background-color: var(--lumo-primary-color-10pct); - } -`; - -registerStyles('vaadin-virtual-list', virtualListStyles, { moduleId: 'lumo-virtual-list' }); diff --git a/packages/virtual-list/theme/lumo/vaadin-virtual-list.js b/packages/virtual-list/theme/lumo/vaadin-virtual-list.js index 845d2b28da..1e22fffeb8 100644 --- a/packages/virtual-list/theme/lumo/vaadin-virtual-list.js +++ b/packages/virtual-list/theme/lumo/vaadin-virtual-list.js @@ -1,2 +1 @@ -import './vaadin-virtual-list-styles.js'; import '../../src/vaadin-virtual-list.js'; From 6c8734a41b0fa1680f7f02f7939da04ce2b5f4b3 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Tue, 10 Dec 2024 13:23:43 +0200 Subject: [PATCH 25/49] fix: browser-specific fixes --- dev/virtual-list.html | 6 +----- .../src/vaadin-virtual-list-selection-mixin.js | 15 +++++++++------ .../test/virtual-list-selection.common.ts | 11 +++++++++-- packages/virtual-list/test/virtual-list.common.js | 2 +- 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/dev/virtual-list.html b/dev/virtual-list.html index b3bf476ddc..535de73a61 100644 --- a/dev/virtual-list.html +++ b/dev/virtual-list.html @@ -64,11 +64,7 @@ const selectionMode = document.querySelector('#selection-mode'); selectionMode.value = list.selectionMode; selectionMode.addEventListener('change', (e) => { - if (e.target.value === 'none') { - list.selectionMode = null; - } else { - list.selectionMode = e.target.value; - } + list.selectionMode = e.target.value; }); document.querySelector('#empty').addEventListener('click', () => { diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index 40089b1737..7d02086afb 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -115,9 +115,9 @@ export const SelectionMixin = (superClass) => el.toggleAttribute('focused', isFocusable && el.contains(document.activeElement)); el.role = this.__isSelectable() ? 'option' : 'listitem'; - el.ariaSelected = this.__isSelectable() ? this.__isSelected(item) : null; - el.ariaSetSize = this.items.length; - el.ariaPosInSet = index + 1; + el.ariaSelected = this.__isSelectable() ? String(this.__isSelected(item)) : null; + el.ariaSetSize = String(this.items.length); + el.ariaPosInSet = String(index + 1); el.ariaLabel = this.itemAccessibleNameGenerator ? this.itemAccessibleNameGenerator(item) : null; } @@ -223,7 +223,10 @@ export const SelectionMixin = (superClass) => /** @private */ __focusElementWithFocusIndex() { this.__ensureFocusedIndexInView(); - this.__getRenderedFocusIndexElement().focus(); + const focusElement = this.__getRenderedFocusIndexElement(); + if (focusElement) { + focusElement.focus(); + } } /** @private */ @@ -275,8 +278,8 @@ export const SelectionMixin = (superClass) => /** @private */ __updateFocusable() { const isFocusable = !!(this.__isNavigating() && this.items && this.items.length); - if (isFocusable) { - this.tabIndex = 0; + if (this.__isSelectable()) { + this.tabIndex = isFocusable ? 0 : -1; } else { this.removeAttribute('tabindex'); } diff --git a/packages/virtual-list/test/virtual-list-selection.common.ts b/packages/virtual-list/test/virtual-list-selection.common.ts index 5edf38c877..10be2753be 100644 --- a/packages/virtual-list/test/virtual-list-selection.common.ts +++ b/packages/virtual-list/test/virtual-list-selection.common.ts @@ -52,7 +52,7 @@ describe('selection', () => { it('should not be focusable by default', async () => { beforeButton.focus(); await sendKeys({ press: 'Tab' }); - expect(document.activeElement).to.equal(afterButton); + expect([...list.children].includes(document.activeElement!)).to.be.false; }); it('should select an item programmatically', async () => { @@ -391,7 +391,7 @@ describe('selection', () => { const listRect = list.getBoundingClientRect(); const lastVisibleItemRect = getRenderedItem(newLastVisibleIndex)!.getBoundingClientRect(); - expect(lastVisibleItemRect.bottom).to.equal(listRect.bottom); + expect(lastVisibleItemRect.bottom).to.be.closeTo(listRect.bottom, 1); }); it('should ensure focused index in viewport when navigating up with arrow keys', async () => { @@ -579,9 +579,16 @@ describe('selection', () => { }); it('should tab through focusable children when selection mode is unset', async () => { + // Because focus works diffrently on Firefox and Chrome, use a renderer with two focusable children + list.renderer = (root, _, { item }) => { + render(html``, root); + }; list.selectionMode = 'none'; + await nextFrame(); + beforeButton.focus(); await sendKeys({ press: 'Tab' }); + await sendKeys({ press: 'Tab' }); expect(document.activeElement!.parentElement).to.equal(getRenderedItem(0)); }); diff --git a/packages/virtual-list/test/virtual-list.common.js b/packages/virtual-list/test/virtual-list.common.js index 79b3aae6f5..f2e9aba130 100644 --- a/packages/virtual-list/test/virtual-list.common.js +++ b/packages/virtual-list/test/virtual-list.common.js @@ -101,7 +101,7 @@ describe('virtual-list', () => { it('should have a last visible index', () => { const item = [...list.children].find((el) => el.textContent === `value-${list.lastVisibleIndex}`); const itemRect = item.getBoundingClientRect(); - expect(list.getBoundingClientRect().bottom).to.be.within(itemRect.top, itemRect.bottom); + expect(list.getBoundingClientRect().bottom).to.be.within(itemRect.top, itemRect.bottom + 1); }); it('should clear the old content after assigning a new renderer', () => { From 33833c77a40caf4697e69f19de027028c130cafa Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Tue, 10 Dec 2024 14:15:29 +0200 Subject: [PATCH 26/49] fix types --- .../src/vaadin-virtual-list-mixin.d.ts | 16 ++++++++++++- .../src/vaadin-virtual-list-mixin.js | 1 + .../virtual-list/src/vaadin-virtual-list.d.ts | 24 +++++++++++++------ .../virtual-list/src/vaadin-virtual-list.js | 2 ++ .../test/typings/virtual-list.types.ts | 5 ++++ 5 files changed, 40 insertions(+), 8 deletions(-) diff --git a/packages/virtual-list/src/vaadin-virtual-list-mixin.d.ts b/packages/virtual-list/src/vaadin-virtual-list-mixin.d.ts index 1adf7fe99e..ea0a64823f 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-mixin.d.ts +++ b/packages/virtual-list/src/vaadin-virtual-list-mixin.d.ts @@ -6,6 +6,7 @@ import type { Constructor } from '@open-wc/dedupe-mixin'; import type { ControllerMixinClass } from '@vaadin/component-base/src/controller-mixin.js'; import type { VirtualList } from './vaadin-virtual-list.js'; +import type { VirtualListSelectionMixinClass } from './vaadin-virtual-list-selection-mixin.js'; export type VirtualListDefaultItem = any; @@ -21,11 +22,24 @@ export type VirtualListRenderer = ( model: VirtualListItemModel, ) => void; +/** + * Fired when the `selectedItems` property changes. + */ +export type VirtualListSelectedItemsChangedEvent = CustomEvent<{ value: TItem[] }>; + +export interface VirtualListCustomEventMap { + 'selected-items-changed': VirtualListSelectedItemsChangedEvent; +} + +export interface VirtualListEventMap extends HTMLElementEventMap, VirtualListCustomEventMap {} + export declare function VirtualListMixin>( base: T, ): Constructor & Constructor> & T; -export declare class VirtualListMixinClass { +export declare class VirtualListMixinClass< + TItem = VirtualListDefaultItem, +> extends VirtualListSelectionMixinClass { /** * Gets the index of the first visible item in the viewport. */ diff --git a/packages/virtual-list/src/vaadin-virtual-list-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-mixin.js index 62930d4598..b1f901c6c9 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-mixin.js @@ -11,6 +11,7 @@ import { SelectionMixin } from './vaadin-virtual-list-selection-mixin.js'; /** * @polymerMixin + * @mixes SelectionMixin * @mixes ControllerMixin */ export const VirtualListMixin = (superClass) => diff --git a/packages/virtual-list/src/vaadin-virtual-list.d.ts b/packages/virtual-list/src/vaadin-virtual-list.d.ts index 6c9d6d7258..7abab93bea 100644 --- a/packages/virtual-list/src/vaadin-virtual-list.d.ts +++ b/packages/virtual-list/src/vaadin-virtual-list.d.ts @@ -7,13 +7,11 @@ import { ElementMixin } from '@vaadin/component-base/src/element-mixin.js'; import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js'; import type { VirtualListDefaultItem, - VirtualListItemModel, + VirtualListEventMap, VirtualListMixinClass, - VirtualListRenderer, } from './vaadin-virtual-list-mixin.js'; -import type { VirtualListSelectionMixinClass } from './vaadin-virtual-list-selection-mixin.js'; -export { VirtualListDefaultItem, VirtualListItemModel, VirtualListRenderer }; +export * from './vaadin-virtual-list-mixin.js'; /** * `` is a Web Component for displaying a virtual/infinite list of items. @@ -37,13 +35,25 @@ export { VirtualListDefaultItem, VirtualListItemModel, VirtualListRenderer }; * `overflow` | Set to `top`, `bottom`, both, or none. * * See [Virtual List](https://vaadin.com/docs/latest/components/virtual-list) documentation. + * + * @fires {CustomEvent} selected-items-changed - Fired when the `selectedItems` property changes. */ declare class VirtualList extends ThemableMixin(ElementMixin(HTMLElement)) {} // eslint-disable-next-line @typescript-eslint/no-empty-object-type -interface VirtualList - extends VirtualListMixinClass, - VirtualListSelectionMixinClass {} +interface VirtualList extends VirtualListMixinClass { + addEventListener>( + type: K, + listener: (this: VirtualList, ev: VirtualListEventMap[K]) => void, + options?: AddEventListenerOptions | boolean, + ): void; + + removeEventListener>( + type: K, + listener: (this: VirtualList, ev: VirtualListEventMap[K]) => void, + options?: EventListenerOptions | boolean, + ): void; +} declare global { interface HTMLElementTagNameMap { diff --git a/packages/virtual-list/src/vaadin-virtual-list.js b/packages/virtual-list/src/vaadin-virtual-list.js index 1add5c7987..490fa50429 100644 --- a/packages/virtual-list/src/vaadin-virtual-list.js +++ b/packages/virtual-list/src/vaadin-virtual-list.js @@ -35,6 +35,8 @@ registerStyles('vaadin-virtual-list', virtualListStyles, { moduleId: 'vaadin-vir * * See [Virtual List](https://vaadin.com/docs/latest/components/virtual-list) documentation. * + * @fires {CustomEvent} selected-items-changed - Fired when the `selectedItems` property changes. + * * @customElement * @extends HTMLElement * @mixes ElementMixin diff --git a/packages/virtual-list/test/typings/virtual-list.types.ts b/packages/virtual-list/test/typings/virtual-list.types.ts index 0fde3541cc..bf9e5b6142 100644 --- a/packages/virtual-list/test/typings/virtual-list.types.ts +++ b/packages/virtual-list/test/typings/virtual-list.types.ts @@ -47,3 +47,8 @@ assertType(virtualList.selectedItems); assertType(virtualList.itemIdPath); assertType<((item: TestVirtualListItem) => string) | undefined>(virtualList.itemAccessibleNameGenerator); + +virtualList.addEventListener('selected-items-changed', (event) => { + assertType>(event); + assertType(event.detail.value); +}); From 7c5e2ec6102e53fde3b25d6a8f4c26dfe46a1cb4 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Tue, 10 Dec 2024 14:59:03 +0200 Subject: [PATCH 27/49] refactor: cleanup --- .../virtual-list/src/vaadin-lit-virtual-list.js | 2 +- .../src/vaadin-virtual-list-selection-mixin.js | 13 +++++++++---- packages/virtual-list/src/vaadin-virtual-list.js | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/virtual-list/src/vaadin-lit-virtual-list.js b/packages/virtual-list/src/vaadin-lit-virtual-list.js index 1547eb3fce..68cf923aaa 100644 --- a/packages/virtual-list/src/vaadin-lit-virtual-list.js +++ b/packages/virtual-list/src/vaadin-lit-virtual-list.js @@ -35,7 +35,7 @@ class VirtualList extends VirtualListMixin(ThemableMixin(ElementMixin(PolylitMix - +
`; } } diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index 7d02086afb..3447b05b3f 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -73,6 +73,11 @@ export const SelectionMixin = (superClass) => value: 0, sync: true, }, + + __focusExitVisible: { + type: Boolean, + value: false, + }, }; } @@ -110,7 +115,7 @@ export const SelectionMixin = (superClass) => el.__index = index; el.toggleAttribute('selected', this.__isSelected(item)); - const isFocusable = this.__isNavigating() && this.__isSelectable() && this.__focusIndex === index; + const isFocusable = this.__isNavigating() && this.__focusIndex === index; el.tabIndex = isFocusable ? 0 : -1; el.toggleAttribute('focused', isFocusable && el.contains(document.activeElement)); @@ -265,10 +270,10 @@ export const SelectionMixin = (superClass) => /** @private */ __updateNavigating(navigating) { - const isNavigating = !!(this.__isSelectable() && navigating); + const isNavigating = this.__isSelectable() && navigating; this.toggleAttribute('navigating', isNavigating); - const isInteracting = !!(this.__isSelectable() && !navigating); + const isInteracting = this.__isSelectable() && !navigating; this.toggleAttribute('interacting', isInteracting); this.__updateFocusable(); @@ -283,7 +288,7 @@ export const SelectionMixin = (superClass) => } else { this.removeAttribute('tabindex'); } - this.$.focusexit.hidden = !isFocusable; + this.__focusExitVisible = isFocusable; } /** @private */ diff --git a/packages/virtual-list/src/vaadin-virtual-list.js b/packages/virtual-list/src/vaadin-virtual-list.js index 490fa50429..5af6f8b37b 100644 --- a/packages/virtual-list/src/vaadin-virtual-list.js +++ b/packages/virtual-list/src/vaadin-virtual-list.js @@ -50,7 +50,7 @@ class VirtualList extends ElementMixin(ThemableMixin(VirtualListMixin(PolymerEle - +
`; } From 272c046c3ef58af51b764df84c2d815e14a6ae04 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Tue, 10 Dec 2024 15:39:23 +0200 Subject: [PATCH 28/49] fix: make sure focused index element gets focused on space select --- dev/virtual-list.html | 5 ++++- .../src/vaadin-virtual-list-selection-mixin.js | 4 ++-- .../test/virtual-list-selection.common.ts | 11 +++++++++++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/dev/virtual-list.html b/dev/virtual-list.html index 535de73a61..cddcda12ec 100644 --- a/dev/virtual-list.html +++ b/dev/virtual-list.html @@ -86,13 +86,16 @@ diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index 3447b05b3f..bc48ea4456 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -353,8 +353,8 @@ export const SelectionMixin = (superClass) => /** @private */ __onNavigationSpaceKey() { - // Ensure the focused item is in view before toggling selection - this.__ensureFocusedIndexInView(); + // Ensure the focused item is in view and focused before toggling selection + this.__focusElementWithFocusIndex(); this.__toggleSelection(this.__getRenderedFocusIndexElement().__item); } diff --git a/packages/virtual-list/test/virtual-list-selection.common.ts b/packages/virtual-list/test/virtual-list-selection.common.ts index 10be2753be..8199159b44 100644 --- a/packages/virtual-list/test/virtual-list-selection.common.ts +++ b/packages/virtual-list/test/virtual-list-selection.common.ts @@ -195,9 +195,20 @@ describe('selection', () => { list.scrollToIndex(100); await sendKeys({ press: 'Space' }); expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.true; + expect(getRenderedItem(0)!.hasAttribute('focused')).to.be.true; expect(list.selectedItems).to.deep.equal([list.items![0]]); }); + it('should scroll back to the focused item when selecting an item with keyboard (manual scroll)', async () => { + beforeButton.focus(); + await sendKeys({ press: 'Tab' }); + list.scrollTop = 1000; + await nextFrame(); + await sendKeys({ press: 'Space' }); + expect(getRenderedItem(0)!.hasAttribute('selected')).to.be.true; + expect(getRenderedItem(0)!.hasAttribute('focused')).to.be.true; + }); + it('should ignore selection of undefined items (Flow VirtualList)', async () => { list.items = [undefined]; beforeButton.focus(); From 2c4dae6f6ca567adae169ab958346224de005f72 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Tue, 10 Dec 2024 15:55:46 +0200 Subject: [PATCH 29/49] refactor: avoid jumpiness on arrow navigation --- .../virtual-list/src/vaadin-virtual-list-selection-mixin.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js index bc48ea4456..3aa5d05027 100644 --- a/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js +++ b/packages/virtual-list/src/vaadin-virtual-list-selection-mixin.js @@ -321,6 +321,11 @@ export const SelectionMixin = (superClass) => __onNavigationArrowKey(down) { this.__focusIndex = this.__clampIndex(this.__focusIndex + (down ? 1 : -1)); this.__focusElementWithFocusIndex(); + + if (this.__debounceRequestContentUpdate) { + // Render synchronously to avoid jumpiness when navigating near viewport edges + this.__debounceRequestContentUpdate.flush(); + } } /** @private */ From 24001d4ac7502e929879f93f59c992bc0e2fa94e Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Tue, 10 Dec 2024 15:57:21 +0200 Subject: [PATCH 30/49] chore: clean up dev page --- dev/virtual-list.html | 57 +++++++------------------------------------ 1 file changed, 9 insertions(+), 48 deletions(-) diff --git a/dev/virtual-list.html b/dev/virtual-list.html index cddcda12ec..3ee59593e7 100644 --- a/dev/virtual-list.html +++ b/dev/virtual-list.html @@ -9,56 +9,23 @@ @@ -100,9 +63,7 @@ - -