From a2a810a5f52c002a8239bd443572703a66889a33 Mon Sep 17 00:00:00 2001 From: Serhii Kulykov Date: Thu, 26 Sep 2024 13:43:17 +0300 Subject: [PATCH] refactor: remove no longer needed internal-tab event workaround (#7870) --- .../src/vaadin-grid-pro-edit-column-mixin.js | 1 - .../vaadin-grid-pro-inline-editing-mixin.js | 14 +- .../test/keyboard-navigation.common.js | 28 +-- .../grid-pro-custom-editor.test.js | 191 ++++++++++++------ 4 files changed, 127 insertions(+), 107 deletions(-) diff --git a/packages/grid-pro/src/vaadin-grid-pro-edit-column-mixin.js b/packages/grid-pro/src/vaadin-grid-pro-edit-column-mixin.js index da2a29d0e3..d4554b724f 100644 --- a/packages/grid-pro/src/vaadin-grid-pro-edit-column-mixin.js +++ b/packages/grid-pro/src/vaadin-grid-pro-edit-column-mixin.js @@ -281,7 +281,6 @@ export const GridProEditColumnMixin = (superClass) => const editor = this._getEditorComponent(cell); editor.addEventListener('focusout', this._grid.__boundEditorFocusOut); editor.addEventListener('focusin', this._grid.__boundEditorFocusIn); - editor.addEventListener('internal-tab', this._grid.__boundCancelCellSwitch); this._setEditorOptions(editor); this._setEditorValue(editor, get(this.path, model.item)); editor._grid = this._grid; 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 1a2c0c6cf7..484c1761f5 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 @@ -65,7 +65,6 @@ export const InlineEditingMixin = (superClass) => this.__boundItemPropertyChanged = this._onItemPropertyChanged.bind(this); this.__boundEditorFocusOut = this._onEditorFocusOut.bind(this); this.__boundEditorFocusIn = this._onEditorFocusIn.bind(this); - this.__boundCancelCellSwitch = this._setCancelCellSwitch.bind(this); this._addEditColumnListener('mousedown', (e) => { // Prevent grid from resetting navigating state @@ -305,8 +304,7 @@ export const InlineEditingMixin = (superClass) => /** @private */ _onEditorFocusOut(event) { - // Ignore focusout from internal tab event - if (this.__cancelCellSwitch || this.__shouldIgnoreFocusOut(event)) { + if (this.__shouldIgnoreFocusOut(event)) { return; } @@ -416,20 +414,12 @@ export const InlineEditingMixin = (superClass) => } } - /** @private */ - _setCancelCellSwitch() { - this.__cancelCellSwitch = true; - window.requestAnimationFrame(() => { - this.__cancelCellSwitch = false; - }); - } - /** * @param {!KeyboardEvent} e * @protected */ async _switchEditCell(e) { - if (this.__cancelCellSwitch || (e.defaultPrevented && e.keyCode === 9)) { + if (e.defaultPrevented && e.keyCode === 9) { return; } diff --git a/packages/grid-pro/test/keyboard-navigation.common.js b/packages/grid-pro/test/keyboard-navigation.common.js index c5b9115446..21c0906033 100644 --- a/packages/grid-pro/test/keyboard-navigation.common.js +++ b/packages/grid-pro/test/keyboard-navigation.common.js @@ -1,5 +1,5 @@ import { expect } from '@vaadin/chai-plugins'; -import { fixtureSync, nextFrame } from '@vaadin/testing-helpers'; +import { fixtureSync } from '@vaadin/testing-helpers'; import { sendKeys } from '@web/test-runner-commands'; import sinon from 'sinon'; import { createItems, dblclick, dragAndDropOver, flushGrid, getCellEditor, getContainerCell } from './helpers.js'; @@ -167,32 +167,6 @@ describe('keyboard navigation', () => { expect(getCellEditor(firstCell)).to.be.ok; }); - it('should not focus cell next available for editing on Tab if `internal-tab` was fired right before it', async () => { - const firstCell = getContainerCell(grid.$.items, 1, 0); - dblclick(firstCell._content); - - const editor = getCellEditor(firstCell); - editor.addEventListener('keydown', (e) => { - if (e.key === 'Tab') { - editor.dispatchEvent(new CustomEvent('internal-tab')); - } - }); - await sendKeys({ press: 'Tab' }); - expect(getCellEditor(firstCell)).to.be.ok; - }); - - it('should be possible to switch edit cell on Tab with delay after `internal-tab` was fired', async () => { - const firstCell = getContainerCell(grid.$.items, 1, 0); - dblclick(firstCell._content); - - getCellEditor(firstCell).dispatchEvent(new CustomEvent('internal-tab')); - const secondCell = getContainerCell(grid.$.items, 1, 1); - await nextFrame(); - - await sendKeys({ press: 'Tab' }); - expect(getCellEditor(secondCell)).to.be.ok; - }); - it('should focus previous cell available for editing within a same row in edit mode on Shift Tab', async () => { const firstCell = getContainerCell(grid.$.items, 1, 1); dblclick(firstCell._content); diff --git a/test/integration/grid-pro-custom-editor.test.js b/test/integration/grid-pro-custom-editor.test.js index aad25302a5..f8c889f639 100644 --- a/test/integration/grid-pro-custom-editor.test.js +++ b/test/integration/grid-pro-custom-editor.test.js @@ -2,83 +2,107 @@ import { expect } from '@vaadin/chai-plugins'; import { fixtureSync, middleOfNode, nextRender } from '@vaadin/testing-helpers'; import { resetMouse, sendKeys, sendMouse } from '@web/test-runner-commands'; import '@vaadin/combo-box'; +import '@vaadin/custom-field'; import '@vaadin/date-picker'; import '@vaadin/date-time-picker'; import '@vaadin/grid-pro'; import '@vaadin/grid-pro/vaadin-grid-pro-edit-column.js'; +import '@vaadin/text-field'; import '@vaadin/time-picker'; import { waitForOverlayRender } from '@vaadin/date-picker/test/helpers.js'; import { flushGrid, getContainerCell } from '@vaadin/grid-pro/test/helpers.js'; describe('grid-pro custom editor', () => { - let grid; + let grid, cell; - beforeEach(async () => { + function createGrid(path) { grid = fixtureSync(` - - - - + `); + const column = grid.querySelector(`[path="${path}"]`); + switch (path) { + case 'date': + column.editModeRenderer = (root, _, model) => { + root.innerHTML = ` + + `; + }; + break; + case 'time': + column.editModeRenderer = (root, _, model) => { + root.innerHTML = ` + + `; + }; + break; + case 'status': + column.editModeRenderer = (root, _, model) => { + root.innerHTML = ` + + `; + }; + break; + case 'datetime': + column.editModeRenderer = (root, _, model) => { + root.innerHTML = ` + + + `; + }; + break; + case 'expires': + column.editModeRenderer = (root, _, model) => { + if (!root.firstChild) { + // NOTE: using `innerHTML` doesn't work due to the timing issue in custom-field + // See https://github.com/vaadin/web-components/issues/7871 + const field = document.createElement('vaadin-custom-field'); + field.appendChild(document.createElement('vaadin-text-field')); + field.appendChild(document.createElement('vaadin-text-field')); + root.appendChild(field); + } + root.firstChild.value = model.item.expires; + }; + break; + default: + // Do nothing + } + grid.items = [ - { date: '1984-01-13', status: 'suspended', time: '09:00' }, - { date: '1977-07-12', status: 'active', time: '09:30' }, - { date: '1976-12-14', status: 'suspended', time: '10:00' }, - { date: '1972-12-04', status: 'active', time: '10:00' }, - { date: '1978-02-03', status: 'active', time: '10:00' }, + { date: '1984-01-13', status: 'suspended', time: '09:00', expires: '01\t25' }, + { date: '1977-07-12', status: 'active', time: '09:30', expires: '02\t26' }, + { date: '1976-12-14', status: 'suspended', time: '10:00', expires: '03\t27' }, + { date: '1972-12-04', status: 'active', time: '10:00', expires: '04\t28' }, + { date: '1978-02-03', status: 'active', time: '10:00', expires: '05\t29' }, ].map((item) => { return { ...item, datetime: `${item.date}T${item.time}` }; }); - grid.querySelector('[path="date"]').editModeRenderer = (root, _, model) => { - root.innerHTML = ` - - - `; - }; - - grid.querySelector('[path="status"]').editModeRenderer = (root, _, model) => { - if (!root.firstChild) { - const comboBox = document.createElement('vaadin-combo-box'); - comboBox.autoOpenDisabled = true; - comboBox.items = ['active', 'suspended']; - root.appendChild(comboBox); - } - root.firstChild.value = model.item.status; - }; - - grid.querySelector('[path="time"]').editModeRenderer = (root, _, model) => { - root.innerHTML = ` - - `; - }; - - grid.querySelector('[path="datetime"]').editModeRenderer = (root, _, model) => { - root.innerHTML = ` - - - `; - }; - flushGrid(grid); - await nextRender(); - }); + return grid; + } + + async function editFirstCell() { + cell = getContainerCell(grid.$.items, 0, 0); + cell.focus(); + await sendKeys({ press: 'Enter' }); + } afterEach(async () => { await resetMouse(); }); describe('date-picker', () => { - let cell; - beforeEach(async () => { - cell = getContainerCell(grid.$.items, 0, 0); - cell.focus(); - - await sendKeys({ press: 'Enter' }); + grid = createGrid('date'); + await nextRender(); + await editFirstCell(); }); it('should apply the updated date to the cell when exiting on Tab', async () => { @@ -145,13 +169,10 @@ describe('grid-pro custom editor', () => { }); describe('combo-box', () => { - let cell; - beforeEach(async () => { - cell = getContainerCell(grid.$.items, 0, 1); - cell.focus(); - - await sendKeys({ press: 'Enter' }); + grid = createGrid('status'); + await nextRender(); + await editFirstCell(); }); it('should apply the updated value to the cell when exiting on Tab', async () => { @@ -181,13 +202,10 @@ describe('grid-pro custom editor', () => { }); describe('time-picker', () => { - let cell; - beforeEach(async () => { - cell = getContainerCell(grid.$.items, 0, 2); - cell.focus(); - - await sendKeys({ press: 'Enter' }); + grid = createGrid('time'); + await nextRender(); + await editFirstCell(); }); it('should apply the updated time to the cell when exiting on Tab', async () => { @@ -217,13 +235,10 @@ describe('grid-pro custom editor', () => { }); describe('date-time-picker', () => { - let cell; - beforeEach(async () => { - cell = getContainerCell(grid.$.items, 0, 3); - cell.focus(); - - await sendKeys({ press: 'Enter' }); + grid = createGrid('datetime'); + await nextRender(); + await editFirstCell(); }); it('should not stop editing when switching between pickers using Tab', async () => { @@ -280,4 +295,46 @@ describe('grid-pro custom editor', () => { expect(editor.value).to.equal('1984-01-13T00:00'); }); }); + + describe('custom-field', () => { + beforeEach(async () => { + grid = createGrid('expires'); + await nextRender(); + await editFirstCell(); + }); + + it('should not stop editing when switching between fields using mouse', async () => { + // Move focus to the second field + const { x, y } = middleOfNode(cell._content.querySelectorAll('input')[1]); + await sendMouse({ type: 'click', position: [Math.floor(x), Math.floor(y)] }); + await nextRender(); + expect(cell._content.querySelector('vaadin-custom-field')).to.be.ok; + }); + + it('should not stop editing when switching between fields using Tab', async () => { + // Move focus to the second field + await sendKeys({ press: 'Tab' }); + await nextRender(); + expect(cell._content.querySelector('vaadin-custom-field')).to.be.ok; + + // Move focus to the first field + await sendKeys({ down: 'Shift' }); + await sendKeys({ press: 'Tab' }); + await sendKeys({ up: 'Shift' }); + await nextRender(); + expect(cell._content.querySelector('vaadin-custom-field')).to.be.ok; + }); + + it('should stop editing when moving focus outside the field using Tab', async () => { + // Move focus to the second field + await sendKeys({ press: 'Tab' }); + await nextRender(); + expect(cell._content.querySelector('vaadin-custom-field')).to.be.ok; + + // Move focus outside of the field + await sendKeys({ press: 'Tab' }); + await nextRender(); + expect(cell._content.querySelector('vaadin-custom-field')).to.be.not.ok; + }); + }); });