diff --git a/packages/grid-pro/src/vaadin-grid-pro-inline-editing-mixin.js b/packages/grid-pro/src/vaadin-grid-pro-inline-editing-mixin.js index f5eb6b2dde..ccfea3a009 100644 --- a/packages/grid-pro/src/vaadin-grid-pro-inline-editing-mixin.js +++ b/packages/grid-pro/src/vaadin-grid-pro-inline-editing-mixin.js @@ -120,15 +120,6 @@ export const InlineEditingMixin = (superClass) => this.$.table.addEventListener('click', (e) => { const column = this.getEventContext(e).column; if (column && this._isEditColumn(column)) { - if (e.target.matches(':not([type=checkbox])')) { - // Prevents the `click` event from a column's cell in order to prevent making the cell's parent row active. - // Note, it should not prevent the `click` event from checkboxes. Otherwise, they will not respond to user interactions. - // Read more: https://github.com/vaadin/web-components/pull/2539#discussion_r712076433. - // TODO: Using `preventDefault()` for any other purpose than preventing the default browser's behavior is bad practice - // and therefore it would be great to refactor this part someday. - e.preventDefault(); - } - if (this.editOnClick) { this._enterEditFromEvent(e); } @@ -166,6 +157,20 @@ export const InlineEditingMixin = (superClass) => } } + /** + * Prevents making an edit column cell's parent row active on click. + * + * @override + * @protected + */ + _shouldPreventCellActivationOnClick(e) { + return ( + super._shouldPreventCellActivationOnClick(e) || + // The clicked cell is editable + this._isEditColumn(this.getEventContext(e).column) + ); + } + /** * Override an observer from `DisabledMixin` to stop * editing when grid element becomes disabled. diff --git a/packages/grid-pro/test/edit-column.common.js b/packages/grid-pro/test/edit-column.common.js index 3a3a4b5ec8..1d60d481ed 100644 --- a/packages/grid-pro/test/edit-column.common.js +++ b/packages/grid-pro/test/edit-column.common.js @@ -695,6 +695,22 @@ describe('edit column', () => { cell._content.click(); expect(activeItemChangedSpy.called).to.be.true; }); + + it('should not fire the event on focusable content click in the not editable column', () => { + cell = getContainerCell(grid.$.items, 0, 3); + cell._content.append(document.createElement('button')); + cell._content.querySelector('button').click(); + expect(activeItemChangedSpy.called).to.be.false; + }); + + it('should not cancel click events on editable column cells', () => { + const clickSpy = sinon.spy(); + grid.addEventListener('click', clickSpy); + cell = getContainerCell(grid.$.items, 0, 1); + cell._content.click(); + expect(clickSpy.called).to.be.true; + expect(clickSpy.getCall(0).args[0].defaultPrevented).to.be.false; + }); }); }); diff --git a/packages/grid/src/vaadin-grid-active-item-mixin.js b/packages/grid/src/vaadin-grid-active-item-mixin.js index 9d603e9e1e..c31858f36c 100644 --- a/packages/grid/src/vaadin-grid-active-item-mixin.js +++ b/packages/grid/src/vaadin-grid-active-item-mixin.js @@ -69,27 +69,41 @@ export const ActiveItemMixin = (superClass) => } /** - * We need to listen to click instead of tap because on mobile safari, the - * document.activeElement has not been updated (focus has not been shifted) - * yet at the point when tap event is being executed. + * Checks whether the click event should not activate the cell on which it occurred. + * + * @protected + */ + _shouldPreventCellActivationOnClick(e) { + const { cell } = this._getGridEventLocation(e); + return ( + // Something has handled this click already, e. g., + e.defaultPrevented || + // No clicked cell available + !cell || + // Cell is a details cell + cell.getAttribute('part').includes('details-cell') || + // Cell is the empty state cell + cell === this.$.emptystatecell || + // Cell content is focused + cell._content.contains(this.getRootNode().activeElement) || + // Clicked on a focusable element + this._isFocusable(e.target) || + // Clicked on a label element + e.target instanceof HTMLLabelElement + ); + } + + /** * @param {!MouseEvent} e * @protected */ _onClick(e) { - if (e.defaultPrevented) { - // Something has handled this click already, e. g., + if (this._shouldPreventCellActivationOnClick(e)) { return; } const { cell } = this._getGridEventLocation(e); - if (!cell || cell.getAttribute('part').indexOf('details-cell') > -1 || cell === this.$.emptystatecell) { - return; - } - const cellContent = cell._content; - - const activeElement = this.getRootNode().activeElement; - const cellContentHasFocus = cellContent.contains(activeElement); - if (!cellContentHasFocus && !this._isFocusable(e.target) && !(e.target instanceof HTMLLabelElement)) { + if (cell) { this.dispatchEvent( new CustomEvent('cell-activate', { detail: {