From 499559496c77f3510e372da5eb278286e4de5085 Mon Sep 17 00:00:00 2001 From: Serhii Kulykov Date: Thu, 26 Sep 2024 10:06:10 +0300 Subject: [PATCH] refactor: replace global focusin listener with focusout check (#7858) --- .../src/vaadin-grid-pro-edit-column-mixin.js | 3 - .../vaadin-grid-pro-inline-editing-mixin.js | 50 +++++--- test/integration/dialog-grid-pro.test.js | 114 ------------------ .../grid-pro-custom-editor.test.js | 52 +++++--- 4 files changed, 69 insertions(+), 150 deletions(-) delete mode 100644 test/integration/dialog-grid-pro.test.js 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 c02dd07dce..da2a29d0e3 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 @@ -282,7 +282,6 @@ export const GridProEditColumnMixin = (superClass) => editor.addEventListener('focusout', this._grid.__boundEditorFocusOut); editor.addEventListener('focusin', this._grid.__boundEditorFocusIn); editor.addEventListener('internal-tab', this._grid.__boundCancelCellSwitch); - document.body.addEventListener('focusin', this._grid.__boundGlobalFocusIn); this._setEditorOptions(editor); this._setEditorValue(editor, get(this.path, model.item)); editor._grid = this._grid; @@ -300,8 +299,6 @@ export const GridProEditColumnMixin = (superClass) => * @protected */ _stopCellEdit(cell, model) { - document.body.removeEventListener('focusin', this._grid.__boundGlobalFocusIn); - this._removeEditor(cell, model); } }; 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 38ee4ac779..1a2c0c6cf7 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 @@ -66,7 +66,6 @@ export const InlineEditingMixin = (superClass) => this.__boundEditorFocusOut = this._onEditorFocusOut.bind(this); this.__boundEditorFocusIn = this._onEditorFocusIn.bind(this); this.__boundCancelCellSwitch = this._setCancelCellSwitch.bind(this); - this.__boundGlobalFocusIn = this._onGlobalFocusIn.bind(this); this._addEditColumnListener('mousedown', (e) => { // Prevent grid from resetting navigating state @@ -305,36 +304,37 @@ export const InlineEditingMixin = (superClass) => } /** @private */ - _onEditorFocusOut() { + _onEditorFocusOut(event) { // Ignore focusout from internal tab event - if (this.__cancelCellSwitch) { + if (this.__cancelCellSwitch || this.__shouldIgnoreFocusOut(event)) { return; } + // Schedule stop on editor component focusout this._debouncerStopEdit = Debouncer.debounce(this._debouncerStopEdit, animationFrame, this._stopEdit.bind(this)); } /** @private */ - _onEditorFocusIn() { - this._cancelStopEdit(); - } - - /** @private */ - _onGlobalFocusIn(e) { + __shouldIgnoreFocusOut(event) { const edited = this.__edited; if (edited) { - // Detect focus moving to e.g. vaadin-select-overlay - const overlay = Array.from(e.composedPath()).filter( - (node) => node.nodeType === Node.ELEMENT_NODE && /^vaadin-(?!dialog).*-overlay$/iu.test(node.localName), - )[0]; - - if (overlay) { - overlay.addEventListener('vaadin-overlay-outside-click', this.__boundEditorFocusOut); - this._cancelStopEdit(); + const { cell, column } = this.__edited; + const editor = column._getEditorComponent(cell); + + const path = event.composedPath(); + const nodes = path.slice(0, path.indexOf(editor) + 1).filter((node) => node.nodeType === Node.ELEMENT_NODE); + // Detect focus moving to e.g. vaadin-select-overlay or vaadin-date-picker-overlay + if (nodes.some((el) => typeof el._shouldRemoveFocus === 'function' && !el._shouldRemoveFocus(event))) { + return true; } } } + /** @private */ + _onEditorFocusIn() { + this._cancelStopEdit(); + } + /** @private */ _startEdit(cell, column) { const isCellEditable = this._isCellEditable(cell); @@ -446,9 +446,21 @@ export const InlineEditingMixin = (superClass) => // Do not prevent Tab to allow native input blur and wait for it, // unless the keydown event is from the edit cell select overlay. if (e.key === 'Tab' && editor && editor.contains(e.target)) { - await new Promise((resolve) => { - editor.addEventListener('focusout', () => resolve(), { once: true }); + const ignore = await new Promise((resolve) => { + editor.addEventListener( + 'focusout', + (event) => { + resolve(this.__shouldIgnoreFocusOut(event)); + }, + { once: true }, + ); }); + + // Ignore focusout event after which focus stays in the field, + // e.g. Tab between date and time pickers in date-time-picker. + if (ignore) { + return; + } } else { e.preventDefault(); } diff --git a/test/integration/dialog-grid-pro.test.js b/test/integration/dialog-grid-pro.test.js deleted file mode 100644 index c3f4b23f4e..0000000000 --- a/test/integration/dialog-grid-pro.test.js +++ /dev/null @@ -1,114 +0,0 @@ -import { expect } from '@vaadin/chai-plugins'; -import { enter, fixtureSync, focusin, focusout, nextFrame, nextRender, outsideClick } from '@vaadin/testing-helpers'; -import '@vaadin/date-picker'; -import '@vaadin/dialog'; -import '@vaadin/grid-pro'; -import '@vaadin/grid-pro/vaadin-grid-pro-edit-column.js'; -import '@vaadin/polymer-legacy-adapter/template-renderer.js'; -import { createItems, flushGrid, getCellEditor, getContainerCell } from '@vaadin/grid-pro/test/helpers.js'; - -async function clickOverlay(element) { - focusout(element); - - // Add a microTask in between - await Promise.resolve(); - - focusin(element.$.overlay); -} - -const fixtures = { - default: ` - - - - `, - template: ` - - - - `, -}; - -['default', 'template'].forEach((type) => { - describe(`${type} grid-pro in dialog`, () => { - let dialog, grid, dateCell; - - beforeEach(async () => { - dialog = fixtureSync(fixtures[type]); - await nextRender(); - grid = dialog.$.overlay.querySelector('vaadin-grid-pro'); - grid.items = createItems(); - grid.style.width = '100px'; // Column default min width is 100px - flushGrid(grid); - - dateCell = getContainerCell(grid.$.items, 0, 1); - - if (type === 'default') { - grid.querySelector('[path="date"]').editModeRenderer = function (root) { - root.innerHTML = ''; - const inputWrapper = document.createElement('div'); - const datePicker = document.createElement('vaadin-date-picker'); - inputWrapper.appendChild(datePicker); - root.appendChild(inputWrapper); - }; - } - }); - - it('should not stop editing when focusing input related overlay', async () => { - enter(dateCell); - const datePicker = getCellEditor(dateCell).querySelector('vaadin-date-picker'); - datePicker.click(); - - await clickOverlay(datePicker); - grid._debouncerStopEdit?.flush(); - - await nextFrame(); - expect(getCellEditor(dateCell)).to.be.ok; - }); - - it('should stop editing on outside click from input related overlay', async () => { - enter(dateCell); - const datePicker = getCellEditor(dateCell).querySelector('vaadin-date-picker'); - datePicker.click(); - - clickOverlay(datePicker); - await nextFrame(); - - outsideClick(); - - grid._debouncerStopEdit?.flush(); - - expect(getCellEditor(dateCell)).not.to.be.ok; - }); - - it('should stop editing when focusing overlay containing grid', () => { - enter(dateCell); - const datePicker = getCellEditor(dateCell).querySelector('vaadin-date-picker'); - - // Mimic clicking the dialog overlay - focusout(datePicker); - focusin(dialog.$.overlay); - grid._debouncerStopEdit?.flush(); - - expect(getCellEditor(dateCell)).to.be.not.ok; - }); - }); -}); diff --git a/test/integration/grid-pro-custom-editor.test.js b/test/integration/grid-pro-custom-editor.test.js index 71888587ff..aad25302a5 100644 --- a/test/integration/grid-pro-custom-editor.test.js +++ b/test/integration/grid-pro-custom-editor.test.js @@ -105,7 +105,21 @@ describe('grid-pro custom editor', () => { expect(cell._content.querySelector('vaadin-date-picker')).to.be.ok; }); - it('should stop editing and update value when closing on outside click', async () => { + it('should not stop editing when clicking inside the overlay but not on focusable element', async () => { + // Open the overlay + await sendKeys({ press: 'ArrowDown' }); + await waitForOverlayRender(); + + // Click between toolbar buttons + const overlayContent = document.querySelector('vaadin-date-picker-overlay-content'); + const { x, y } = middleOfNode(overlayContent.shadowRoot.querySelector('[part="toolbar"]')); + await sendMouse({ type: 'click', position: [Math.floor(x), Math.floor(y)] }); + await nextRender(); + + expect(cell._content.querySelector('vaadin-date-picker')).to.be.ok; + }); + + it('should not stop editing and update value when closing on outside click', async () => { // Open the overlay await sendKeys({ press: 'ArrowDown' }); await waitForOverlayRender(); @@ -124,11 +138,9 @@ describe('grid-pro custom editor', () => { await sendMouse({ type: 'click', position: [10, 10] }); await nextRender(); - // TODO: closing occurs in `vaadin-overlay-outside-click` listener added on global `focusin` - // in grid-pro. Consider replacing it with `_shouldRemoveFocus()` check on editor `focusout` - // so that we don't stop editing on outside click, to align with the combo-box behavior. - expect(cell._content.querySelector('vaadin-date-picker')).to.be.not.ok; - expect(cell._content.textContent).to.equal('1984-01-12'); + const editor = cell._content.querySelector('vaadin-date-picker'); + expect(editor).to.be.ok; + expect(editor.value).to.equal('1984-01-12'); }); }); @@ -214,7 +226,21 @@ describe('grid-pro custom editor', () => { await sendKeys({ press: 'Enter' }); }); - it('should stop editing and update value when closing on date-picker outside click', async () => { + it('should not stop editing when switching between pickers using Tab', async () => { + // Move focus to the time-picker + await sendKeys({ press: 'Tab' }); + await nextRender(); + expect(cell._content.querySelector('vaadin-date-time-picker')).to.be.ok; + + // Move focus to the date-picker + await sendKeys({ down: 'Shift' }); + await sendKeys({ press: 'Tab' }); + await sendKeys({ up: 'Shift' }); + await nextRender(); + expect(cell._content.querySelector('vaadin-date-time-picker')).to.be.ok; + }); + + it('should not stop editing and update value when closing on date-picker outside click', async () => { await sendKeys({ press: 'ArrowDown' }); await waitForOverlayRender(); @@ -232,16 +258,13 @@ describe('grid-pro custom editor', () => { await sendMouse({ type: 'click', position: [10, 10] }); await nextRender(); - // TODO: closing occurs in `vaadin-overlay-outside-click` listener added on global `focusin` - // in grid-pro. Consider replacing it with `_shouldRemoveFocus()` check on editor `focusout` - // so that we don't stop editing on outside click, to align with the combo-box behavior. - expect(cell._content.querySelector('vaadin-date-time-picker')).to.be.not.ok; - expect(cell._content.textContent).to.equal('1984-01-12T09:00'); + const editor = cell._content.querySelector('vaadin-date-time-picker'); + expect(editor).to.be.ok; + expect(editor.value).to.equal('1984-01-12T09:00'); }); it('should not stop editing and update value when closing on time-picker outside click', async () => { - // TODO: replace with Tab and add a separate test to not stop editing on Tab - cell._content.querySelector('vaadin-time-picker').focus(); + await sendKeys({ press: 'Tab' }); // Open the overlay await sendKeys({ press: 'ArrowDown' }); @@ -250,6 +273,7 @@ describe('grid-pro custom editor', () => { await sendKeys({ press: 'ArrowDown' }); await sendMouse({ type: 'click', position: [10, 10] }); + await nextRender(); const editor = cell._content.querySelector('vaadin-date-time-picker'); expect(editor).to.be.ok;