From 67b7b90ed47062ecaeee7dead3736ea899c01082 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Tue, 15 Oct 2024 13:27:57 +0200 Subject: [PATCH 01/11] basic isItemSelectable implementation --- packages/grid/src/vaadin-grid-mixin.js | 8 + ...vaadin-grid-selection-column-base-mixin.js | 35 +++- .../src/vaadin-grid-selection-column-mixin.js | 14 +- .../grid/src/vaadin-grid-selection-mixin.js | 58 ++++-- packages/grid/test/helpers.js | 8 +- .../grid/test/selectable-provider-lit.test.js | 3 + .../test/selectable-provider-polymer.test.js | 2 + .../grid/test/selectable-provider.common.js | 182 ++++++++++++++++++ 8 files changed, 284 insertions(+), 26 deletions(-) create mode 100644 packages/grid/test/selectable-provider-lit.test.js create mode 100644 packages/grid/test/selectable-provider-polymer.test.js create mode 100644 packages/grid/test/selectable-provider.common.js diff --git a/packages/grid/src/vaadin-grid-mixin.js b/packages/grid/src/vaadin-grid-mixin.js index b7b885e181..ff800dfb29 100644 --- a/packages/grid/src/vaadin-grid-mixin.js +++ b/packages/grid/src/vaadin-grid-mixin.js @@ -1020,6 +1020,14 @@ export const GridMixin = (superClass) => }; } + /** + * @private + */ + __getRowModelByItem(item) { + const row = this._getRenderedRows().find((row) => row._item === item); + return row ? this.__getRowModel(row) : null; + } + /** * @param {Event} event * @protected diff --git a/packages/grid/src/vaadin-grid-selection-column-base-mixin.js b/packages/grid/src/vaadin-grid-selection-column-base-mixin.js index 22641e9040..c0dfa1c203 100644 --- a/packages/grid/src/vaadin-grid-selection-column-base-mixin.js +++ b/packages/grid/src/vaadin-grid-selection-column-base-mixin.js @@ -129,8 +129,9 @@ export const GridSelectionColumnBaseMixin = (superClass) => * * @override */ - _defaultRenderer(root, _column, { item, selected }) { + _defaultRenderer(root, _column, model) { let checkbox = root.firstElementChild; + const { item, selected } = model; if (!checkbox) { checkbox = document.createElement('vaadin-checkbox'); checkbox.setAttribute('aria-label', 'Select Row'); @@ -145,6 +146,7 @@ export const GridSelectionColumnBaseMixin = (superClass) => checkbox.__item = item; checkbox.__rendererChecked = selected; checkbox.checked = selected; + checkbox.disabled = !this._grid.__isItemSelectable(model); } /** @@ -245,9 +247,19 @@ export const GridSelectionColumnBaseMixin = (superClass) => _onCellKeyDown(e) { const target = e.composedPath()[0]; // Toggle on Space without having to enter interaction mode first - if (e.keyCode === 32 && (target === this._headerCell || (this._cells.includes(target) && !this.autoSelect))) { + if (e.keyCode !== 32) { + return; + } + if (target === this._headerCell) { + if (this.selectAll) { + this._deselectAll(); + } else { + this._selectAll(); + } + } else if (this._cells.includes(target) && !this.autoSelect) { const checkbox = target._content.firstElementChild; - checkbox.checked = !checkbox.checked; + const item = checkbox.__item; + this.__toggleItem(item); } } @@ -346,18 +358,31 @@ export const GridSelectionColumnBaseMixin = (superClass) => /** * Override to handle the user selecting an item. - * @param {Object} item the item to select + * @param {Object} _item the item to select * @protected */ _selectItem(_item) {} /** * Override to handle the user deselecting an item. - * @param {Object} item the item to deselect + * @param {Object} _item the item to deselect * @protected */ _deselectItem(_item) {} + /** + * Toggles the selected state of the given item. + * @param item the item to toggle + * @private + */ + __toggleItem(item) { + if (this._grid._isSelected(item)) { + this._deselectItem(item); + } else { + this._selectItem(item); + } + } + /** * IOS needs indeterminate + checked at the same time * @private diff --git a/packages/grid/src/vaadin-grid-selection-column-mixin.js b/packages/grid/src/vaadin-grid-selection-column-mixin.js index 84fe7c22e8..3f9bf226ed 100644 --- a/packages/grid/src/vaadin-grid-selection-column-mixin.js +++ b/packages/grid/src/vaadin-grid-selection-column-mixin.js @@ -120,7 +120,11 @@ export const GridSelectionColumnMixin = (superClass) => * @override */ _selectItem(item) { - this._grid.selectItem(item); + const model = this._grid.__getRowModelByItem(item); + const isSelectable = this._grid.__isItemSelectable(model); + if (isSelectable) { + this._grid.selectItem(item); + } } /** @@ -132,7 +136,11 @@ export const GridSelectionColumnMixin = (superClass) => * @override */ _deselectItem(item) { - this._grid.deselectItem(item); + const model = this._grid.__getRowModelByItem(item); + const isSelectable = this._grid.__isItemSelectable(model); + if (isSelectable) { + this._grid.deselectItem(item); + } } /** @private */ @@ -141,7 +149,7 @@ export const GridSelectionColumnMixin = (superClass) => if (this.autoSelect) { const item = activeItem || this.__previousActiveItem; if (item) { - this._grid._toggleItem(item); + this.__toggleItem(item); } } this.__previousActiveItem = activeItem; diff --git a/packages/grid/src/vaadin-grid-selection-mixin.js b/packages/grid/src/vaadin-grid-selection-mixin.js index b61f4dd7d8..6738201d29 100644 --- a/packages/grid/src/vaadin-grid-selection-mixin.js +++ b/packages/grid/src/vaadin-grid-selection-mixin.js @@ -22,6 +22,31 @@ export const SelectionMixin = (superClass) => sync: true, }, + /** + * A function to check whether a specific item in the grid may be + * selected or deselected by the user. Used by the selection column to + * conditionally enable to disable checkboxes for individual items. This + * function does not prevent programmatic selection/deselection of + * items. Changing the function does not update the currently selected + * items. + * + * Receives a `model` object containing the item for an individual row, + * and should return a boolean indicating whether users may change the + * selection state of that item. + * + * The `model` object contains: + * - `model.index` The index of the item. + * - `model.item` The item. + * - `model.expanded` Sublevel toggle state. + * - `model.level` Level of the tree represented with a horizontal offset of the toggle button. + * - `model.selected` Selected state. + * + * @type {(model: GridItemModel) => boolean} + */ + isItemSelectable: { + type: Function, + }, + /** * Set of selected item ids * @private @@ -34,7 +59,7 @@ export const SelectionMixin = (superClass) => } static get observers() { - return ['__selectedItemsChanged(itemIdPath, selectedItems)']; + return ['__selectedItemsChanged(itemIdPath, selectedItems, isItemSelectable)']; } /** @@ -46,6 +71,22 @@ export const SelectionMixin = (superClass) => return this.__selectedKeys.has(this.getItemId(item)); } + /** + * Determines whether the selection state of an item may be changed by the + * user. + * + * @private + */ + __isItemSelectable(model) { + // Item is selectable by default if isItemSelectable is not configured + if (!this.isItemSelectable || !model) { + return true; + } + + // Otherwise, check isItemSelectable function + return this.isItemSelectable(model); + } + /** * Selects the given item. * @@ -70,21 +111,6 @@ export const SelectionMixin = (superClass) => } } - /** - * Toggles the selected state of the given item. - * - * @method toggle - * @param {!GridItem} item The item object - * @protected - */ - _toggleItem(item) { - if (!this._isSelected(item)) { - this.selectItem(item); - } else { - this.deselectItem(item); - } - } - /** @private */ __selectedItemsChanged() { this.requestContentUpdate(); diff --git a/packages/grid/test/helpers.js b/packages/grid/test/helpers.js index d4ee0f3845..f44b28dbb9 100644 --- a/packages/grid/test/helpers.js +++ b/packages/grid/test/helpers.js @@ -162,11 +162,15 @@ export const getHeaderCellContent = (grid, row, col) => { return getContainerCellContent(container, row, col); }; -export const getBodyCellContent = (grid, row, col) => { +export const getBodyCell = (grid, row, col) => { const physicalItems = getPhysicalItems(grid); const physicalRow = physicalItems.find((item) => item.index === row); const cells = getRowCells(physicalRow); - return getCellContent(cells[col]); + return cells[col]; +}; + +export const getBodyCellContent = (grid, row, col) => { + return getCellContent(getBodyCell(grid, row, col)); }; export const fire = (type, detail, options) => { diff --git a/packages/grid/test/selectable-provider-lit.test.js b/packages/grid/test/selectable-provider-lit.test.js new file mode 100644 index 0000000000..46ad9a7029 --- /dev/null +++ b/packages/grid/test/selectable-provider-lit.test.js @@ -0,0 +1,3 @@ +import '../theme/lumo/lit-all-imports.js'; +import '../src/lit-all-imports.js'; +import './selectable-provider.common.js'; diff --git a/packages/grid/test/selectable-provider-polymer.test.js b/packages/grid/test/selectable-provider-polymer.test.js new file mode 100644 index 0000000000..9ed8937008 --- /dev/null +++ b/packages/grid/test/selectable-provider-polymer.test.js @@ -0,0 +1,2 @@ +import '../all-imports.js'; +import './selectable-provider.common.js'; diff --git a/packages/grid/test/selectable-provider.common.js b/packages/grid/test/selectable-provider.common.js new file mode 100644 index 0000000000..3160c3f72e --- /dev/null +++ b/packages/grid/test/selectable-provider.common.js @@ -0,0 +1,182 @@ +import { expect } from '@vaadin/chai-plugins'; +import { fixtureSync } from '@vaadin/testing-helpers'; +import { sendKeys } from '@web/test-runner-commands'; +import sinon from 'sinon'; +import { fire, flushGrid, getBodyCell, getBodyCellContent } from './helpers.js'; + +describe('selectable-provider', () => { + let grid; + let selectionColumn; + let clock; + + function getItemCheckbox(rowIndex) { + return getBodyCellContent(grid, rowIndex, 0).querySelector('vaadin-checkbox'); + } + + function fireTrackEvent(targetCell, startCell, eventState) { + const targetCellRect = targetCell.getBoundingClientRect(); + const startCellRect = startCell.getBoundingClientRect(); + fire( + 'track', + { state: eventState, y: targetCellRect.y, dy: targetCellRect.y - startCellRect.y }, + { node: targetCell }, + ); + } + + beforeEach(() => { + grid = fixtureSync(` + + + + + `); + selectionColumn = grid.querySelector('vaadin-grid-selection-column'); + + // setup 10 items, first 5 are non-selectable + grid.items = Array.from({ length: 10 }, (_, i) => { + return { name: `item ${i}` }; + }); + grid.isItemSelectable = (model) => model.index >= 5; + + flushGrid(grid); + + clock = sinon.useFakeTimers({ + shouldClearNativeTimers: true, + }); + }); + + afterEach(() => { + clock.restore(); + }); + + describe('individual selection', () => { + it('should disable checkboxes for non-selectable items', () => { + for (let i = 0; i < grid.items.length; i++) { + expect(getItemCheckbox(i).disabled).to.equal(i < 5); + } + }); + + it('should update disabled checkboxes when changing isItemSelectable', () => { + grid.isItemSelectable = (model) => model.index < 5; + flushGrid(grid); + + for (let i = 0; i < grid.items.length; i++) { + expect(getItemCheckbox(i).disabled).to.equal(i >= 5); + } + }); + + it('should prevent selection on checkbox click', () => { + // prevents selection for non-selectable items + getItemCheckbox(0).click(); + expect(grid.selectedItems.length).to.equal(0); + + // allows selection for selectable items + getItemCheckbox(5).click(); + expect(grid.selectedItems.length).to.equal(1); + }); + + it('should prevent deselection on checkbox click', () => { + grid.selectedItems = [...grid.items]; + + // prevents deselection for non-selectable items + getItemCheckbox(0).click(); + expect(grid.selectedItems.length).to.equal(10); + + // allows deselection for selectable items + getItemCheckbox(5).click(); + expect(grid.selectedItems.length).to.equal(9); + }); + + it('should prevent selection on row click when using auto-select', () => { + selectionColumn.autoSelect = true; + + // prevents selection for non-selectable items + getBodyCellContent(grid, 0, 1).click(); + expect(grid.selectedItems.length).to.equal(0); + + // allows selection for selectable items + getBodyCellContent(grid, 5, 1).click(); + expect(grid.selectedItems.length).to.equal(1); + }); + + it('should prevent deselection on row click when using auto-select', () => { + grid.selectedItems = [...grid.items]; + selectionColumn.autoSelect = true; + + // prevents deselection for non-selectable items + getBodyCellContent(grid, 0, 1).click(); + expect(grid.selectedItems.length).to.equal(10); + + // allows deselection for selectable items + getBodyCellContent(grid, 5, 1).click(); + expect(grid.selectedItems.length).to.equal(9); + }); + + it('should prevent selection when pressing space on cell', async () => { + // prevents selection for non-selectable items + getBodyCell(grid, 0, 0).focus(); + await sendKeys({ press: 'Space' }); + expect(grid.selectedItems.length).to.equal(0); + + // allows selection for selectable items + getBodyCell(grid, 5, 0).focus(); + await sendKeys({ press: 'Space' }); + expect(grid.selectedItems.length).to.equal(1); + }); + + it('should prevent deselection when pressing space on cell', async () => { + grid.selectedItems = [...grid.items]; + + // prevents deselection for non-selectable items + getBodyCell(grid, 0, 0).focus(); + await sendKeys({ press: 'Space' }); + expect(grid.selectedItems.length).to.equal(10); + + // allows deselection for selectable items + getBodyCell(grid, 5, 0).focus(); + await sendKeys({ press: 'Space' }); + expect(grid.selectedItems.length).to.equal(9); + }); + + it('should prevent selection when drag selecting', () => { + selectionColumn.dragSelect = true; + + // drag select in reverse from selectable items to non-selectable items + for (let i = 9; i >= 0; i--) { + const cellContent = getBodyCellContent(grid, i, 0); + const eventState = i === 9 ? 'start' : i === 0 ? 'end' : 'track'; + fireTrackEvent(cellContent, cellContent, eventState); + clock.tick(10); + } + expect(grid.selectedItems.length).to.equal(5); + expect(grid.selectedItems).to.include.members(grid.items.slice(5)); + }); + + it('should prevent deselection when drag selecting', () => { + grid.selectedItems = [...grid.items]; + selectionColumn.dragSelect = true; + + // drag select in reverse from selectable items to non-selectable items + for (let i = 9; i >= 0; i--) { + const cellContent = getBodyCellContent(grid, i, 0); + const eventState = i === 9 ? 'start' : i === 0 ? 'end' : 'track'; + fireTrackEvent(cellContent, cellContent, eventState); + clock.tick(10); + } + expect(grid.selectedItems.length).to.equal(5); + expect(grid.selectedItems).to.include.members(grid.items.slice(0, 5)); + }); + }); + + describe('programmatic selection', () => { + it('should not prevent programmatic selection', () => { + grid.selectItem(grid.items[0]); + grid.selectItem(grid.items[1]); + expect(grid.selectedItems.length).to.equal(2); + + grid.deselectItem(grid.items[0]); + grid.deselectItem(grid.items[1]); + expect(grid.selectedItems.length).to.equal(0); + }); + }); +}); From 3961f60af8a51a7524eb1ae77983821f36d32d95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Tue, 15 Oct 2024 14:50:18 +0200 Subject: [PATCH 02/11] handle select all checkbox --- packages/grid/src/vaadin-grid-mixin.js | 8 -- .../src/vaadin-grid-selection-column-mixin.js | 53 ++++---- .../grid/src/vaadin-grid-selection-mixin.js | 21 +-- packages/grid/test/helpers.js | 8 +- .../grid/test/selectable-provider.common.js | 123 +++++++++++++++--- 5 files changed, 145 insertions(+), 68 deletions(-) diff --git a/packages/grid/src/vaadin-grid-mixin.js b/packages/grid/src/vaadin-grid-mixin.js index ff800dfb29..b7b885e181 100644 --- a/packages/grid/src/vaadin-grid-mixin.js +++ b/packages/grid/src/vaadin-grid-mixin.js @@ -1020,14 +1020,6 @@ export const GridMixin = (superClass) => }; } - /** - * @private - */ - __getRowModelByItem(item) { - const row = this._getRenderedRows().find((row) => row._item === item); - return row ? this.__getRowModel(row) : null; - } - /** * @param {Event} event * @protected diff --git a/packages/grid/src/vaadin-grid-selection-column-mixin.js b/packages/grid/src/vaadin-grid-selection-column-mixin.js index 3f9bf226ed..66c92f5e11 100644 --- a/packages/grid/src/vaadin-grid-selection-column-mixin.js +++ b/packages/grid/src/vaadin-grid-selection-column-mixin.js @@ -40,6 +40,7 @@ export const GridSelectionColumnMixin = (superClass) => this._grid.removeEventListener('data-provider-changed', this.__boundOnDataProviderChanged); this._grid.removeEventListener('filter-changed', this.__boundOnSelectedItemsChanged); this._grid.removeEventListener('selected-items-changed', this.__boundOnSelectedItemsChanged); + this._grid.removeEventListener('is-item-selectable-changed', this.__boundOnSelectedItemsChanged); super.disconnectedCallback(); } @@ -52,6 +53,7 @@ export const GridSelectionColumnMixin = (superClass) => this._grid.addEventListener('data-provider-changed', this.__boundOnDataProviderChanged); this._grid.addEventListener('filter-changed', this.__boundOnSelectedItemsChanged); this._grid.addEventListener('selected-items-changed', this.__boundOnSelectedItemsChanged); + this._grid.addEventListener('is-item-selectable-changed', this.__boundOnSelectedItemsChanged); } } @@ -72,11 +74,12 @@ export const GridSelectionColumnMixin = (superClass) => } if (selectAll && this.__hasArrayDataProvider()) { - this.__withFilteredItemsArray((items) => { - this._grid.selectedItems = items; - }); + // Select all items that users may change the selection state of + this._grid.selectedItems = this.__getSelectableItems(); } else { - this._grid.selectedItems = []; + // Deselect all, keeping items that users may not change the selection state of + const currentSelection = this._grid.selectedItems || []; + this._grid.selectedItems = currentSelection.filter((item) => !this._grid.__isItemSelectable(item)); } } @@ -120,8 +123,7 @@ export const GridSelectionColumnMixin = (superClass) => * @override */ _selectItem(item) { - const model = this._grid.__getRowModelByItem(item); - const isSelectable = this._grid.__isItemSelectable(model); + const isSelectable = this._grid.__isItemSelectable(item); if (isSelectable) { this._grid.selectItem(item); } @@ -136,8 +138,7 @@ export const GridSelectionColumnMixin = (superClass) => * @override */ _deselectItem(item) { - const model = this._grid.__getRowModelByItem(item); - const isSelectable = this._grid.__isItemSelectable(model); + const isSelectable = this._grid.__isItemSelectable(item); if (isSelectable) { this._grid.deselectItem(item); } @@ -164,18 +165,16 @@ export const GridSelectionColumnMixin = (superClass) => __onSelectedItemsChanged() { this._selectAllChangeLock = true; if (this.__hasArrayDataProvider()) { - this.__withFilteredItemsArray((items) => { - if (!this._grid.selectedItems.length) { - this.selectAll = false; - this._indeterminate = false; - } else if (this.__arrayContains(this._grid.selectedItems, items)) { - this.selectAll = true; - this._indeterminate = false; - } else { - this.selectAll = false; - this._indeterminate = true; - } - }); + if (!this._grid.selectedItems.length) { + this.selectAll = false; + this._indeterminate = false; + } else if (this.__arrayContains(this._grid.selectedItems, this.__getSelectableItems())) { + this.selectAll = true; + this._indeterminate = false; + } else { + this.selectAll = false; + this._indeterminate = true; + } } this._selectAllChangeLock = false; } @@ -186,18 +185,22 @@ export const GridSelectionColumnMixin = (superClass) => } /** - * Assuming the grid uses an items array data provider, fetches all the filtered items - * from the data provider and invokes the callback with the resulting array. - * * @private */ - __withFilteredItemsArray(callback) { + __getSelectableItems() { + // Assuming the grid uses an items array data provider, fetches all the + // filtered items from the data provider + let filteredItems = []; const params = { page: 0, pageSize: Infinity, sortOrders: [], filters: this._grid._mapFilters(), }; - this._grid.dataProvider(params, (items) => callback(items)); + this._grid.dataProvider(params, (items) => { + filteredItems = items; + }); + // Filter again for selectable items + return filteredItems.filter((item) => this._grid.__isItemSelectable(item)); } }; diff --git a/packages/grid/src/vaadin-grid-selection-mixin.js b/packages/grid/src/vaadin-grid-selection-mixin.js index 6738201d29..1d5587e610 100644 --- a/packages/grid/src/vaadin-grid-selection-mixin.js +++ b/packages/grid/src/vaadin-grid-selection-mixin.js @@ -30,21 +30,14 @@ export const SelectionMixin = (superClass) => * items. Changing the function does not update the currently selected * items. * - * Receives a `model` object containing the item for an individual row, - * and should return a boolean indicating whether users may change the - * selection state of that item. + * Receives an item instance and should return a boolean indicating + * whether users may change the selection state of that item. * - * The `model` object contains: - * - `model.index` The index of the item. - * - `model.item` The item. - * - `model.expanded` Sublevel toggle state. - * - `model.level` Level of the tree represented with a horizontal offset of the toggle button. - * - `model.selected` Selected state. - * - * @type {(model: GridItemModel) => boolean} + * @type {(item: !GridItem) => boolean} */ isItemSelectable: { type: Function, + notify: true, }, /** @@ -77,14 +70,14 @@ export const SelectionMixin = (superClass) => * * @private */ - __isItemSelectable(model) { + __isItemSelectable(item) { // Item is selectable by default if isItemSelectable is not configured - if (!this.isItemSelectable || !model) { + if (!this.isItemSelectable || !item) { return true; } // Otherwise, check isItemSelectable function - return this.isItemSelectable(model); + return this.isItemSelectable(item); } /** diff --git a/packages/grid/test/helpers.js b/packages/grid/test/helpers.js index f44b28dbb9..2bf6283764 100644 --- a/packages/grid/test/helpers.js +++ b/packages/grid/test/helpers.js @@ -157,9 +157,13 @@ export const getContainerCellContent = (container, row, col) => { return getCellContent(getContainerCell(container, row, col)); }; -export const getHeaderCellContent = (grid, row, col) => { +export const getHeaderCell = (grid, row, col) => { const container = grid.$.header; - return getContainerCellContent(container, row, col); + return getContainerCell(container, row, col); +}; + +export const getHeaderCellContent = (grid, row, col) => { + return getCellContent(getHeaderCell(grid, row, col)); }; export const getBodyCell = (grid, row, col) => { diff --git a/packages/grid/test/selectable-provider.common.js b/packages/grid/test/selectable-provider.common.js index 3160c3f72e..11ac3837cc 100644 --- a/packages/grid/test/selectable-provider.common.js +++ b/packages/grid/test/selectable-provider.common.js @@ -1,13 +1,13 @@ import { expect } from '@vaadin/chai-plugins'; -import { fixtureSync } from '@vaadin/testing-helpers'; +import { fixtureSync, nextFrame } from '@vaadin/testing-helpers'; import { sendKeys } from '@web/test-runner-commands'; import sinon from 'sinon'; -import { fire, flushGrid, getBodyCell, getBodyCellContent } from './helpers.js'; +import { fire, flushGrid, getBodyCell, getBodyCellContent, getHeaderCell, getHeaderCellContent } from './helpers.js'; describe('selectable-provider', () => { let grid; let selectionColumn; - let clock; + let selectAllCheckbox; function getItemCheckbox(rowIndex) { return getBodyCellContent(grid, rowIndex, 0).querySelector('vaadin-checkbox'); @@ -23,30 +23,25 @@ describe('selectable-provider', () => { ); } - beforeEach(() => { + beforeEach(async () => { grid = fixtureSync(` `); - selectionColumn = grid.querySelector('vaadin-grid-selection-column'); // setup 10 items, first 5 are non-selectable grid.items = Array.from({ length: 10 }, (_, i) => { - return { name: `item ${i}` }; + return { index: i, name: `item ${i}` }; }); - grid.isItemSelectable = (model) => model.index >= 5; + grid.isItemSelectable = (item) => item.index >= 5; flushGrid(grid); + await nextFrame(); - clock = sinon.useFakeTimers({ - shouldClearNativeTimers: true, - }); - }); - - afterEach(() => { - clock.restore(); + selectionColumn = grid.querySelector('vaadin-grid-selection-column'); + selectAllCheckbox = getHeaderCellContent(grid, 0, 0).querySelector('vaadin-checkbox'); }); describe('individual selection', () => { @@ -57,7 +52,7 @@ describe('selectable-provider', () => { }); it('should update disabled checkboxes when changing isItemSelectable', () => { - grid.isItemSelectable = (model) => model.index < 5; + grid.isItemSelectable = (item) => item.index < 5; flushGrid(grid); for (let i = 0; i < grid.items.length; i++) { @@ -65,25 +60,29 @@ describe('selectable-provider', () => { } }); - it('should prevent selection on checkbox click', () => { + it('should prevent selection on checkbox click', async () => { // prevents selection for non-selectable items getItemCheckbox(0).click(); + await nextFrame(); expect(grid.selectedItems.length).to.equal(0); // allows selection for selectable items getItemCheckbox(5).click(); + await nextFrame(); expect(grid.selectedItems.length).to.equal(1); }); - it('should prevent deselection on checkbox click', () => { + it('should prevent deselection on checkbox click', async () => { grid.selectedItems = [...grid.items]; // prevents deselection for non-selectable items getItemCheckbox(0).click(); + await nextFrame(); expect(grid.selectedItems.length).to.equal(10); // allows deselection for selectable items getItemCheckbox(5).click(); + await nextFrame(); expect(grid.selectedItems.length).to.equal(9); }); @@ -137,10 +136,23 @@ describe('selectable-provider', () => { await sendKeys({ press: 'Space' }); expect(grid.selectedItems.length).to.equal(9); }); + }); - it('should prevent selection when drag selecting', () => { + describe('drag selection', () => { + let clock; + + beforeEach(() => { selectionColumn.dragSelect = true; + clock = sinon.useFakeTimers({ + shouldClearNativeTimers: true, + }); + }); + + afterEach(() => { + clock.restore(); + }); + it('should prevent selection when drag selecting', () => { // drag select in reverse from selectable items to non-selectable items for (let i = 9; i >= 0; i--) { const cellContent = getBodyCellContent(grid, i, 0); @@ -154,7 +166,6 @@ describe('selectable-provider', () => { it('should prevent deselection when drag selecting', () => { grid.selectedItems = [...grid.items]; - selectionColumn.dragSelect = true; // drag select in reverse from selectable items to non-selectable items for (let i = 9; i >= 0; i--) { @@ -168,6 +179,80 @@ describe('selectable-provider', () => { }); }); + describe('select all', () => { + function isSelectAllChecked() { + return selectAllCheckbox.checked && !selectAllCheckbox.indeterminate; + } + + function isSelectAllIndeterminate() { + return selectAllCheckbox.checked && selectAllCheckbox.indeterminate; + } + + it('should be checked when all selectable items are selected', () => { + grid.selectedItems = grid.items.slice(5); + + expect(isSelectAllChecked()).to.be.true; + }); + + it('should be indeterminate when some selectable items are not selected', () => { + grid.selectedItems = grid.items.slice(6); + + expect(isSelectAllIndeterminate()).to.be.true; + }); + + it('should update when changing isItemSelectable', async () => { + // change provider so that some selectable items are not checked + grid.selectedItems = grid.items.slice(5); + grid.isItemSelectable = (item) => item.index > 1; + await nextFrame(); + + expect(isSelectAllIndeterminate()).to.be.true; + + // revert provider so that only non-selectable items are not checked + grid.isItemSelectable = (item) => item.index >= 5; + await nextFrame(); + + expect(isSelectAllChecked()).to.be.true; + }); + + it('should only check selectable items on click', async () => { + selectAllCheckbox.click(); + await nextFrame(); + + expect(grid.selectedItems.length).to.equal(5); + expect(grid.selectedItems).to.include.members(grid.items.slice(5)); + }); + + it('should only uncheck selectable items on click', async () => { + grid.selectedItems = [...grid.items]; + await nextFrame(); + + selectAllCheckbox.click(); + await nextFrame(); + + expect(grid.selectedItems.length).to.equal(5); + expect(grid.selectedItems).to.include.members(grid.items.slice(0, 5)); + }); + + it('should only check selectable items when pressing space on cell', async () => { + getHeaderCell(grid, 0, 0).focus(); + await sendKeys({ press: 'Space' }); + + expect(grid.selectedItems.length).to.equal(5); + expect(grid.selectedItems).to.include.members(grid.items.slice(5)); + }); + + it('should only uncheck selectable items when pressing space on cell', async () => { + grid.selectedItems = [...grid.items]; + + getHeaderCell(grid, 0, 0).focus(); + await sendKeys({ press: 'Space' }); + + expect(grid.selectedItems.length).to.equal(5); + expect(grid.selectedItems).to.include.members(grid.items.slice(0, 5)); + }); + }); + describe('programmatic selection', () => { it('should not prevent programmatic selection', () => { grid.selectItem(grid.items[0]); From cc11334589a321a8d118e610da271b15fcab943f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Tue, 15 Oct 2024 14:52:52 +0200 Subject: [PATCH 03/11] fix default renderer --- packages/grid/src/vaadin-grid-selection-column-base-mixin.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/grid/src/vaadin-grid-selection-column-base-mixin.js b/packages/grid/src/vaadin-grid-selection-column-base-mixin.js index c0dfa1c203..a0e354bc29 100644 --- a/packages/grid/src/vaadin-grid-selection-column-base-mixin.js +++ b/packages/grid/src/vaadin-grid-selection-column-base-mixin.js @@ -129,9 +129,8 @@ export const GridSelectionColumnBaseMixin = (superClass) => * * @override */ - _defaultRenderer(root, _column, model) { + _defaultRenderer(root, _column, { item, selected }) { let checkbox = root.firstElementChild; - const { item, selected } = model; if (!checkbox) { checkbox = document.createElement('vaadin-checkbox'); checkbox.setAttribute('aria-label', 'Select Row'); @@ -146,7 +145,7 @@ export const GridSelectionColumnBaseMixin = (superClass) => checkbox.__item = item; checkbox.__rendererChecked = selected; checkbox.checked = selected; - checkbox.disabled = !this._grid.__isItemSelectable(model); + checkbox.disabled = !this._grid.__isItemSelectable(item); } /** From e4b361cb947d1df0f7da64c00e5f7b5095e14250 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Tue, 15 Oct 2024 15:44:31 +0200 Subject: [PATCH 04/11] fix more select all cases --- .../src/vaadin-grid-selection-column-mixin.js | 41 ++++++++++++++++--- .../grid/test/selectable-provider.common.js | 19 +++++++++ 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/packages/grid/src/vaadin-grid-selection-column-mixin.js b/packages/grid/src/vaadin-grid-selection-column-mixin.js index 66c92f5e11..cd0dd9f983 100644 --- a/packages/grid/src/vaadin-grid-selection-column-mixin.js +++ b/packages/grid/src/vaadin-grid-selection-column-mixin.js @@ -74,12 +74,17 @@ export const GridSelectionColumnMixin = (superClass) => } if (selectAll && this.__hasArrayDataProvider()) { - // Select all items that users may change the selection state of - this._grid.selectedItems = this.__getSelectableItems(); + // Select all items that users may change the selection state of, keeping items that users may not change the selection state of + const newSelection = this.__getSelectableItems(); + for (const item of this.__getNonModifiableSelectedItems()) { + if (!newSelection.includes(item)) { + newSelection.push(item); + } + } + this._grid.selectedItems = newSelection; } else { // Deselect all, keeping items that users may not change the selection state of - const currentSelection = this._grid.selectedItems || []; - this._grid.selectedItems = currentSelection.filter((item) => !this._grid.__isItemSelectable(item)); + this._grid.selectedItems = this.__getNonModifiableSelectedItems(); } } @@ -165,10 +170,11 @@ export const GridSelectionColumnMixin = (superClass) => __onSelectedItemsChanged() { this._selectAllChangeLock = true; if (this.__hasArrayDataProvider()) { - if (!this._grid.selectedItems.length) { + const modifiableSelection = this.__getModifiableSelectedItems(); + if (!modifiableSelection.length) { this.selectAll = false; this._indeterminate = false; - } else if (this.__arrayContains(this._grid.selectedItems, this.__getSelectableItems())) { + } else if (this.__arrayContains(modifiableSelection, this.__getSelectableItems())) { this.selectAll = true; this._indeterminate = false; } else { @@ -185,6 +191,9 @@ export const GridSelectionColumnMixin = (superClass) => } /** + * Returns all items that the user may change the selection state of, based + * on the filter. + * * @private */ __getSelectableItems() { @@ -203,4 +212,24 @@ export const GridSelectionColumnMixin = (superClass) => // Filter again for selectable items return filteredItems.filter((item) => this._grid.__isItemSelectable(item)); } + + /** + * Returns all selected items that the user may change the selection state + * of. + * + * @private + */ + __getModifiableSelectedItems() { + return this._grid.selectedItems.filter((item) => this._grid.__isItemSelectable(item)); + } + + /** + * Returns all selected items that the user may not change the selection + * state of. + * + * @private + */ + __getNonModifiableSelectedItems() { + return this._grid.selectedItems.filter((item) => !this._grid.__isItemSelectable(item)); + } }; diff --git a/packages/grid/test/selectable-provider.common.js b/packages/grid/test/selectable-provider.common.js index 11ac3837cc..4f1d6ebe1d 100644 --- a/packages/grid/test/selectable-provider.common.js +++ b/packages/grid/test/selectable-provider.common.js @@ -188,6 +188,10 @@ describe('selectable-provider', () => { return selectAllCheckbox.checked && selectAllCheckbox.indeterminate; } + function isSelectAllUnchecked() { + return !selectAllCheckbox.checked && !selectAllCheckbox.indeterminate; + } + it('should be checked when all selectable items are selected', () => { grid.selectedItems = grid.items.slice(5); @@ -200,6 +204,12 @@ describe('selectable-provider', () => { expect(isSelectAllIndeterminate()).to.be.true; }); + it('should be unchecked when only unselectable items are selected', () => { + grid.selectedItems = grid.items.slice(0, 5); + + expect(isSelectAllUnchecked()).to.be.true; + }); + it('should update when changing isItemSelectable', async () => { // change provider so that some selectable items are not checked grid.selectedItems = grid.items.slice(5); @@ -223,6 +233,15 @@ describe('selectable-provider', () => { expect(grid.selectedItems).to.include.members(grid.items.slice(5)); }); + it('should not uncheck non-selectable items on click', async () => { + grid.selectedItems = grid.items.slice(0, 5); + selectAllCheckbox.click(); + await nextFrame(); + + expect(grid.selectedItems.length).to.equal(10); + expect(grid.selectedItems).to.include.members(grid.items); + }); + it('should only uncheck selectable items on click', async () => { grid.selectedItems = [...grid.items]; await nextFrame(); From 9a0dd777c22419a50d7c035ab0e5e7a4f3d23e80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Wed, 16 Oct 2024 10:20:54 +0200 Subject: [PATCH 05/11] make checkboxes readonly or hidden --- ...vaadin-grid-selection-column-base-mixin.js | 3 ++- .../grid/test/selectable-provider.common.js | 22 ++++++++++++++----- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/packages/grid/src/vaadin-grid-selection-column-base-mixin.js b/packages/grid/src/vaadin-grid-selection-column-base-mixin.js index a0e354bc29..5d99eb6abd 100644 --- a/packages/grid/src/vaadin-grid-selection-column-base-mixin.js +++ b/packages/grid/src/vaadin-grid-selection-column-base-mixin.js @@ -145,7 +145,8 @@ export const GridSelectionColumnBaseMixin = (superClass) => checkbox.__item = item; checkbox.__rendererChecked = selected; checkbox.checked = selected; - checkbox.disabled = !this._grid.__isItemSelectable(item); + checkbox.readonly = !this._grid.__isItemSelectable(item); + checkbox.hidden = !this._grid.__isItemSelectable(item) && !selected; } /** diff --git a/packages/grid/test/selectable-provider.common.js b/packages/grid/test/selectable-provider.common.js index 4f1d6ebe1d..2a3b5ae422 100644 --- a/packages/grid/test/selectable-provider.common.js +++ b/packages/grid/test/selectable-provider.common.js @@ -44,22 +44,34 @@ describe('selectable-provider', () => { selectAllCheckbox = getHeaderCellContent(grid, 0, 0).querySelector('vaadin-checkbox'); }); - describe('individual selection', () => { - it('should disable checkboxes for non-selectable items', () => { + describe('checkbox states', () => { + it('should hide checkboxes for non-selectable items that are not selected', () => { for (let i = 0; i < grid.items.length; i++) { - expect(getItemCheckbox(i).disabled).to.equal(i < 5); + expect(getItemCheckbox(i).readonly).to.equal(i < 5); + expect(getItemCheckbox(i).hidden).to.equal(i < 5); } }); - it('should update disabled checkboxes when changing isItemSelectable', () => { + it('should show readonly checkboxes for non-selectable items that are selected', () => { + grid.selectedItems = [...grid.items]; + + for (let i = 0; i < grid.items.length; i++) { + expect(getItemCheckbox(i).readonly).to.equal(i < 5); + expect(getItemCheckbox(i).hidden).to.be.false; + } + }); + + it('should update checkboxes when changing isItemSelectable', () => { grid.isItemSelectable = (item) => item.index < 5; flushGrid(grid); for (let i = 0; i < grid.items.length; i++) { - expect(getItemCheckbox(i).disabled).to.equal(i >= 5); + expect(getItemCheckbox(i).hidden).to.equal(i >= 5); } }); + }); + describe('individual selection', () => { it('should prevent selection on checkbox click', async () => { // prevents selection for non-selectable items getItemCheckbox(0).click(); From f64a6be9119af386deea4ad4e7c65c1565ca512b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Wed, 16 Oct 2024 10:51:45 +0200 Subject: [PATCH 06/11] reuse selectable check --- .../grid/src/vaadin-grid-selection-column-base-mixin.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/grid/src/vaadin-grid-selection-column-base-mixin.js b/packages/grid/src/vaadin-grid-selection-column-base-mixin.js index 5d99eb6abd..55afc50f94 100644 --- a/packages/grid/src/vaadin-grid-selection-column-base-mixin.js +++ b/packages/grid/src/vaadin-grid-selection-column-base-mixin.js @@ -145,8 +145,10 @@ export const GridSelectionColumnBaseMixin = (superClass) => checkbox.__item = item; checkbox.__rendererChecked = selected; checkbox.checked = selected; - checkbox.readonly = !this._grid.__isItemSelectable(item); - checkbox.hidden = !this._grid.__isItemSelectable(item) && !selected; + + const isSelectable = this._grid.__isItemSelectable(item); + checkbox.readonly = !isSelectable; + checkbox.hidden = !isSelectable && !selected; } /** From 2cdf83cb76a213733991d0e1dbc440a74973c5c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Wed, 16 Oct 2024 11:02:02 +0200 Subject: [PATCH 07/11] add TS types --- .../grid/src/vaadin-grid-selection-mixin.d.ts | 16 ++++++++++++++++ packages/grid/src/vaadin-grid-selection-mixin.js | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/grid/src/vaadin-grid-selection-mixin.d.ts b/packages/grid/src/vaadin-grid-selection-mixin.d.ts index 6c1311090b..ac3a387699 100644 --- a/packages/grid/src/vaadin-grid-selection-mixin.d.ts +++ b/packages/grid/src/vaadin-grid-selection-mixin.d.ts @@ -28,4 +28,20 @@ export declare class SelectionMixinClass { * @param item The item object */ deselectItem(item: TItem): void; + + /** + * A function to check whether a specific item in the grid may be + * selected or deselected by the user. Used by the selection column to + * conditionally enable to disable checkboxes for individual items. This + * function does not prevent programmatic selection/deselection of + * items. Changing the function does not modify the currently selected + * items. + * + * Receives an item instance and should return a boolean indicating + * whether users may change the selection state of that item. + * + * @param item The item object + * @return Whether the item is selectable + */ + isItemSelectable: (item: TItem) => boolean; } diff --git a/packages/grid/src/vaadin-grid-selection-mixin.js b/packages/grid/src/vaadin-grid-selection-mixin.js index 1d5587e610..4977e97287 100644 --- a/packages/grid/src/vaadin-grid-selection-mixin.js +++ b/packages/grid/src/vaadin-grid-selection-mixin.js @@ -27,7 +27,7 @@ export const SelectionMixin = (superClass) => * selected or deselected by the user. Used by the selection column to * conditionally enable to disable checkboxes for individual items. This * function does not prevent programmatic selection/deselection of - * items. Changing the function does not update the currently selected + * items. Changing the function does not modify the currently selected * items. * * Receives an item instance and should return a boolean indicating From 6cb554cd9fd1fb8d6128c27d803c235921e9ae55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Wed, 16 Oct 2024 12:13:56 +0200 Subject: [PATCH 08/11] cleanup --- ...vaadin-grid-selection-column-base-mixin.js | 7 +++--- .../src/vaadin-grid-selection-column-mixin.js | 6 ++--- .../grid/test/selectable-provider.common.js | 22 +++++++++---------- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/packages/grid/src/vaadin-grid-selection-column-base-mixin.js b/packages/grid/src/vaadin-grid-selection-column-base-mixin.js index 55afc50f94..956ec89013 100644 --- a/packages/grid/src/vaadin-grid-selection-column-base-mixin.js +++ b/packages/grid/src/vaadin-grid-selection-column-base-mixin.js @@ -260,8 +260,7 @@ export const GridSelectionColumnBaseMixin = (superClass) => } } else if (this._cells.includes(target) && !this.autoSelect) { const checkbox = target._content.firstElementChild; - const item = checkbox.__item; - this.__toggleItem(item); + this.__toggleItem(checkbox.__item); } } @@ -360,14 +359,14 @@ export const GridSelectionColumnBaseMixin = (superClass) => /** * Override to handle the user selecting an item. - * @param {Object} _item the item to select + * @param {Object} item the item to select * @protected */ _selectItem(_item) {} /** * Override to handle the user deselecting an item. - * @param {Object} _item the item to deselect + * @param {Object} item the item to deselect * @protected */ _deselectItem(_item) {} diff --git a/packages/grid/src/vaadin-grid-selection-column-mixin.js b/packages/grid/src/vaadin-grid-selection-column-mixin.js index cd0dd9f983..d2658b5981 100644 --- a/packages/grid/src/vaadin-grid-selection-column-mixin.js +++ b/packages/grid/src/vaadin-grid-selection-column-mixin.js @@ -128,8 +128,7 @@ export const GridSelectionColumnMixin = (superClass) => * @override */ _selectItem(item) { - const isSelectable = this._grid.__isItemSelectable(item); - if (isSelectable) { + if (this._grid.__isItemSelectable(item)) { this._grid.selectItem(item); } } @@ -143,8 +142,7 @@ export const GridSelectionColumnMixin = (superClass) => * @override */ _deselectItem(item) { - const isSelectable = this._grid.__isItemSelectable(item); - if (isSelectable) { + if (this._grid.__isItemSelectable(item)) { this._grid.deselectItem(item); } } diff --git a/packages/grid/test/selectable-provider.common.js b/packages/grid/test/selectable-provider.common.js index 2a3b5ae422..eafe193961 100644 --- a/packages/grid/test/selectable-provider.common.js +++ b/packages/grid/test/selectable-provider.common.js @@ -13,16 +13,6 @@ describe('selectable-provider', () => { return getBodyCellContent(grid, rowIndex, 0).querySelector('vaadin-checkbox'); } - function fireTrackEvent(targetCell, startCell, eventState) { - const targetCellRect = targetCell.getBoundingClientRect(); - const startCellRect = startCell.getBoundingClientRect(); - fire( - 'track', - { state: eventState, y: targetCellRect.y, dy: targetCellRect.y - startCellRect.y }, - { node: targetCell }, - ); - } - beforeEach(async () => { grid = fixtureSync(` @@ -164,6 +154,16 @@ describe('selectable-provider', () => { clock.restore(); }); + function fireTrackEvent(targetCell, startCell, eventState) { + const targetCellRect = targetCell.getBoundingClientRect(); + const startCellRect = startCell.getBoundingClientRect(); + fire( + 'track', + { state: eventState, y: targetCellRect.y, dy: targetCellRect.y - startCellRect.y }, + { node: targetCell }, + ); + } + it('should prevent selection when drag selecting', () => { // drag select in reverse from selectable items to non-selectable items for (let i = 9; i >= 0; i--) { @@ -216,7 +216,7 @@ describe('selectable-provider', () => { expect(isSelectAllIndeterminate()).to.be.true; }); - it('should be unchecked when only unselectable items are selected', () => { + it('should be unchecked when only non-selectable items are selected', () => { grid.selectedItems = grid.items.slice(0, 5); expect(isSelectAllUnchecked()).to.be.true; From bfa2d0a9064590c6293f72c64823732feacf25ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Wed, 16 Oct 2024 16:29:28 +0200 Subject: [PATCH 09/11] simplify select all logic --- packages/grid/src/vaadin-grid-selection-column-mixin.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/grid/src/vaadin-grid-selection-column-mixin.js b/packages/grid/src/vaadin-grid-selection-column-mixin.js index d2658b5981..a720d5f25c 100644 --- a/packages/grid/src/vaadin-grid-selection-column-mixin.js +++ b/packages/grid/src/vaadin-grid-selection-column-mixin.js @@ -75,13 +75,7 @@ export const GridSelectionColumnMixin = (superClass) => if (selectAll && this.__hasArrayDataProvider()) { // Select all items that users may change the selection state of, keeping items that users may not change the selection state of - const newSelection = this.__getSelectableItems(); - for (const item of this.__getNonModifiableSelectedItems()) { - if (!newSelection.includes(item)) { - newSelection.push(item); - } - } - this._grid.selectedItems = newSelection; + this._grid.selectedItems = [...this.__getNonModifiableSelectedItems(), ...this.__getSelectableItems()]; } else { // Deselect all, keeping items that users may not change the selection state of this._grid.selectedItems = this.__getNonModifiableSelectedItems(); From 57bea9c1ba46109e4fdb5fbedc37478b86f88a63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Thu, 17 Oct 2024 11:08:18 +0200 Subject: [PATCH 10/11] fix select all check --- packages/grid/src/vaadin-grid-selection-column-mixin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grid/src/vaadin-grid-selection-column-mixin.js b/packages/grid/src/vaadin-grid-selection-column-mixin.js index d27cf0f6ba..10c1b718bb 100644 --- a/packages/grid/src/vaadin-grid-selection-column-mixin.js +++ b/packages/grid/src/vaadin-grid-selection-column-mixin.js @@ -157,7 +157,7 @@ export const GridSelectionColumnMixin = (superClass) => if (!modifiableSelection.length) { this.selectAll = false; this._indeterminate = false; - } else if (modifiableSelection.every((item) => this._grid._isSelected(item))) { + } else if (this.__getSelectableItems().every((item) => this._grid._isSelected(item))) { this.selectAll = true; this._indeterminate = false; } else { From 35591f29e3869f7fbd164f4dbd90b71bcc1275a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Wed, 23 Oct 2024 12:19:54 +0200 Subject: [PATCH 11/11] hide select all when using conditional selection --- .../src/vaadin-grid-selection-column-mixin.js | 83 ++++++----------- .../grid/src/vaadin-grid-selection-mixin.d.ts | 5 + .../grid/src/vaadin-grid-selection-mixin.js | 5 + .../grid/test/selectable-provider.common.js | 92 ++----------------- 4 files changed, 46 insertions(+), 139 deletions(-) diff --git a/packages/grid/src/vaadin-grid-selection-column-mixin.js b/packages/grid/src/vaadin-grid-selection-column-mixin.js index 10c1b718bb..8bd81283a6 100644 --- a/packages/grid/src/vaadin-grid-selection-column-mixin.js +++ b/packages/grid/src/vaadin-grid-selection-column-mixin.js @@ -30,17 +30,17 @@ export const GridSelectionColumnMixin = (superClass) => super(); this.__boundOnActiveItemChanged = this.__onActiveItemChanged.bind(this); - this.__boundOnDataProviderChanged = this.__onDataProviderChanged.bind(this); + this.__boundUpdateSelectAllVisibility = this.__updateSelectAllVisibility.bind(this); this.__boundOnSelectedItemsChanged = this.__onSelectedItemsChanged.bind(this); } /** @protected */ disconnectedCallback() { this._grid.removeEventListener('active-item-changed', this.__boundOnActiveItemChanged); - this._grid.removeEventListener('data-provider-changed', this.__boundOnDataProviderChanged); + this._grid.removeEventListener('data-provider-changed', this.__boundUpdateSelectAllVisibility); + this._grid.removeEventListener('is-item-selectable-changed', this.__boundUpdateSelectAllVisibility); this._grid.removeEventListener('filter-changed', this.__boundOnSelectedItemsChanged); this._grid.removeEventListener('selected-items-changed', this.__boundOnSelectedItemsChanged); - this._grid.removeEventListener('is-item-selectable-changed', this.__boundOnSelectedItemsChanged); super.disconnectedCallback(); } @@ -50,10 +50,10 @@ export const GridSelectionColumnMixin = (superClass) => super.connectedCallback(); if (this._grid) { this._grid.addEventListener('active-item-changed', this.__boundOnActiveItemChanged); - this._grid.addEventListener('data-provider-changed', this.__boundOnDataProviderChanged); + this._grid.addEventListener('data-provider-changed', this.__boundUpdateSelectAllVisibility); + this._grid.addEventListener('is-item-selectable-changed', this.__boundUpdateSelectAllVisibility); this._grid.addEventListener('filter-changed', this.__boundOnSelectedItemsChanged); this._grid.addEventListener('selected-items-changed', this.__boundOnSelectedItemsChanged); - this._grid.addEventListener('is-item-selectable-changed', this.__boundOnSelectedItemsChanged); } } @@ -74,11 +74,11 @@ export const GridSelectionColumnMixin = (superClass) => } if (selectAll && this.__hasArrayDataProvider()) { - // Select all items that users may change the selection state of, keeping items that users may not change the selection state of - this._grid.selectedItems = [...this.__getNonModifiableSelectedItems(), ...this.__getSelectableItems()]; + this.__withFilteredItemsArray((items) => { + this._grid.selectedItems = items; + }); } else { - // Deselect all, keeping items that users may not change the selection state of - this._grid.selectedItems = this.__getNonModifiableSelectedItems(); + this._grid.selectedItems = []; } } @@ -153,66 +153,43 @@ export const GridSelectionColumnMixin = (superClass) => __onSelectedItemsChanged() { this._selectAllChangeLock = true; if (this.__hasArrayDataProvider()) { - const modifiableSelection = this.__getModifiableSelectedItems(); - if (!modifiableSelection.length) { - this.selectAll = false; - this._indeterminate = false; - } else if (this.__getSelectableItems().every((item) => this._grid._isSelected(item))) { - this.selectAll = true; - this._indeterminate = false; - } else { - this.selectAll = false; - this._indeterminate = true; - } + this.__withFilteredItemsArray((items) => { + if (!this._grid.selectedItems.length) { + this.selectAll = false; + this._indeterminate = false; + } else if (items.every((item) => this._grid._isSelected(item))) { + this.selectAll = true; + this._indeterminate = false; + } else { + this.selectAll = false; + this._indeterminate = true; + } + }); } this._selectAllChangeLock = false; } /** @private */ - __onDataProviderChanged() { - this._selectAllHidden = !Array.isArray(this._grid.items); + __updateSelectAllVisibility() { + // Hide select all checkbox when we can not easily determine the select all checkbox state: + // - When using a custom data provider + // - When using conditional selection, where users may not select all items + this._selectAllHidden = !Array.isArray(this._grid.items) || !!this._grid.isItemSelectable; } /** - * Returns all items that the user may change the selection state of, based - * on the filter. + * Assuming the grid uses an items array data provider, fetches all the filtered items + * from the data provider and invokes the callback with the resulting array. * * @private */ - __getSelectableItems() { - // Assuming the grid uses an items array data provider, fetches all the - // filtered items from the data provider - let filteredItems = []; + __withFilteredItemsArray(callback) { const params = { page: 0, pageSize: Infinity, sortOrders: [], filters: this._grid._mapFilters(), }; - this._grid.dataProvider(params, (items) => { - filteredItems = items; - }); - // Filter again for selectable items - return filteredItems.filter((item) => this._grid.__isItemSelectable(item)); - } - - /** - * Returns all selected items that the user may change the selection state - * of. - * - * @private - */ - __getModifiableSelectedItems() { - return this._grid.selectedItems.filter((item) => this._grid.__isItemSelectable(item)); - } - - /** - * Returns all selected items that the user may not change the selection - * state of. - * - * @private - */ - __getNonModifiableSelectedItems() { - return this._grid.selectedItems.filter((item) => !this._grid.__isItemSelectable(item)); + this._grid.dataProvider(params, (items) => callback(items)); } }; diff --git a/packages/grid/src/vaadin-grid-selection-mixin.d.ts b/packages/grid/src/vaadin-grid-selection-mixin.d.ts index ac3a387699..f17f76ec44 100644 --- a/packages/grid/src/vaadin-grid-selection-mixin.d.ts +++ b/packages/grid/src/vaadin-grid-selection-mixin.d.ts @@ -37,6 +37,11 @@ export declare class SelectionMixinClass { * items. Changing the function does not modify the currently selected * items. * + * Configuring this function hides the select all checkbox of the grid + * selection column, which means users can not select or deselect all + * items anymore, nor do they get feedback on whether all items are + * selected or not. + * * Receives an item instance and should return a boolean indicating * whether users may change the selection state of that item. * diff --git a/packages/grid/src/vaadin-grid-selection-mixin.js b/packages/grid/src/vaadin-grid-selection-mixin.js index 4977e97287..1dfe379730 100644 --- a/packages/grid/src/vaadin-grid-selection-mixin.js +++ b/packages/grid/src/vaadin-grid-selection-mixin.js @@ -30,6 +30,11 @@ export const SelectionMixin = (superClass) => * items. Changing the function does not modify the currently selected * items. * + * Configuring this function hides the select all checkbox of the grid + * selection column, which means users can not select or deselect all + * items anymore, nor do they get feedback on whether all items are + * selected or not. + * * Receives an item instance and should return a boolean indicating * whether users may change the selection state of that item. * diff --git a/packages/grid/test/selectable-provider.common.js b/packages/grid/test/selectable-provider.common.js index eafe193961..2cf5f32d92 100644 --- a/packages/grid/test/selectable-provider.common.js +++ b/packages/grid/test/selectable-provider.common.js @@ -2,7 +2,7 @@ import { expect } from '@vaadin/chai-plugins'; import { fixtureSync, nextFrame } from '@vaadin/testing-helpers'; import { sendKeys } from '@web/test-runner-commands'; import sinon from 'sinon'; -import { fire, flushGrid, getBodyCell, getBodyCellContent, getHeaderCell, getHeaderCellContent } from './helpers.js'; +import { fire, flushGrid, getBodyCell, getBodyCellContent, getHeaderCellContent } from './helpers.js'; describe('selectable-provider', () => { let grid; @@ -192,95 +192,15 @@ describe('selectable-provider', () => { }); describe('select all', () => { - function isSelectAllChecked() { - return selectAllCheckbox.checked && !selectAllCheckbox.indeterminate; - } - - function isSelectAllIndeterminate() { - return selectAllCheckbox.checked && selectAllCheckbox.indeterminate; - } - - function isSelectAllUnchecked() { - return !selectAllCheckbox.checked && !selectAllCheckbox.indeterminate; - } - - it('should be checked when all selectable items are selected', () => { - grid.selectedItems = grid.items.slice(5); - - expect(isSelectAllChecked()).to.be.true; - }); - - it('should be indeterminate when some selectable items are not selected', () => { - grid.selectedItems = grid.items.slice(6); - - expect(isSelectAllIndeterminate()).to.be.true; - }); - - it('should be unchecked when only non-selectable items are selected', () => { - grid.selectedItems = grid.items.slice(0, 5); - - expect(isSelectAllUnchecked()).to.be.true; - }); - - it('should update when changing isItemSelectable', async () => { - // change provider so that some selectable items are not checked - grid.selectedItems = grid.items.slice(5); - grid.isItemSelectable = (item) => item.index > 1; - await nextFrame(); - - expect(isSelectAllIndeterminate()).to.be.true; - - // revert provider so that only non-selectable items are not checked - grid.isItemSelectable = (item) => item.index >= 5; - await nextFrame(); - - expect(isSelectAllChecked()).to.be.true; - }); - - it('should only check selectable items on click', async () => { - selectAllCheckbox.click(); - await nextFrame(); - - expect(grid.selectedItems.length).to.equal(5); - expect(grid.selectedItems).to.include.members(grid.items.slice(5)); + it('should hide select all checkbox when using isItemSelectable provider', () => { + expect(selectAllCheckbox.hasAttribute('hidden')).to.be.true; }); - it('should not uncheck non-selectable items on click', async () => { - grid.selectedItems = grid.items.slice(0, 5); - selectAllCheckbox.click(); - await nextFrame(); - - expect(grid.selectedItems.length).to.equal(10); - expect(grid.selectedItems).to.include.members(grid.items); - }); - - it('should only uncheck selectable items on click', async () => { - grid.selectedItems = [...grid.items]; + it('should show select all checkbox when removing isItemSelectable provider', async () => { + grid.isItemSelectable = null; await nextFrame(); - selectAllCheckbox.click(); - await nextFrame(); - - expect(grid.selectedItems.length).to.equal(5); - expect(grid.selectedItems).to.include.members(grid.items.slice(0, 5)); - }); - - it('should only check selectable items when pressing space on cell', async () => { - getHeaderCell(grid, 0, 0).focus(); - await sendKeys({ press: 'Space' }); - - expect(grid.selectedItems.length).to.equal(5); - expect(grid.selectedItems).to.include.members(grid.items.slice(5)); - }); - - it('should only uncheck selectable items when pressing space on cell', async () => { - grid.selectedItems = [...grid.items]; - - getHeaderCell(grid, 0, 0).focus(); - await sendKeys({ press: 'Space' }); - - expect(grid.selectedItems.length).to.equal(5); - expect(grid.selectedItems).to.include.members(grid.items.slice(0, 5)); + expect(selectAllCheckbox.hasAttribute('hidden')).to.be.false; }); });