From 5c59e40013bdaa9add262222678858cd2014821e Mon Sep 17 00:00:00 2001 From: Serhii Kulykov Date: Tue, 24 Sep 2024 16:29:51 +0300 Subject: [PATCH] fix!: prevent focusout when closing date-picker on outside click (#7855) --- .../src/vaadin-date-picker-overlay-mixin.js | 43 +++++++++++++++ .../src/vaadin-date-picker-overlay.js | 8 ++- .../src/vaadin-lit-date-picker-overlay.js | 8 ++- packages/date-picker/test/dropdown.common.js | 22 ++++---- .../grid-pro-custom-editor.test.js | 52 ++++++++++++++++--- 5 files changed, 105 insertions(+), 28 deletions(-) create mode 100644 packages/date-picker/src/vaadin-date-picker-overlay-mixin.js diff --git a/packages/date-picker/src/vaadin-date-picker-overlay-mixin.js b/packages/date-picker/src/vaadin-date-picker-overlay-mixin.js new file mode 100644 index 0000000000..66ec5537eb --- /dev/null +++ b/packages/date-picker/src/vaadin-date-picker-overlay-mixin.js @@ -0,0 +1,43 @@ +/** + * @license + * Copyright (c) 2015 - 2024 Vaadin Ltd. + * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/ + */ +import { isElementFocusable } from '@vaadin/a11y-base/src/focus-utils.js'; +import { OverlayMixin } from '@vaadin/overlay/src/vaadin-overlay-mixin.js'; +import { PositionMixin } from '@vaadin/overlay/src/vaadin-overlay-position-mixin.js'; + +/** + * @polymerMixin + * @mixes OverlayMixin + * @mixes PositionMixin + */ +export const DatePickerOverlayMixin = (superClass) => + class DatePickerOverlayMixin extends PositionMixin(OverlayMixin(superClass)) { + /** + * Override method inherited from `OverlayMixin` to not close on input click. + * Needed to ignore date-picker's own input in the mousedown listener below. + * + * @param {Event} event + * @return {boolean} + * @protected + */ + _shouldCloseOnOutsideClick(event) { + const eventPath = event.composedPath(); + return !eventPath.includes(this.positionTarget); + } + + /** + * @protected + * @override + */ + _mouseDownListener(event) { + super._mouseDownListener(event); + + // Prevent global mousedown event to avoid losing focus on outside click, + // unless the clicked element is also focusable (e.g. in date-time-picker). + if (this._shouldCloseOnOutsideClick(event) && !isElementFocusable(event.composedPath()[0])) { + event.preventDefault(); + } + } + }; diff --git a/packages/date-picker/src/vaadin-date-picker-overlay.js b/packages/date-picker/src/vaadin-date-picker-overlay.js index ab2316c41c..a7f48bf85a 100644 --- a/packages/date-picker/src/vaadin-date-picker-overlay.js +++ b/packages/date-picker/src/vaadin-date-picker-overlay.js @@ -6,10 +6,9 @@ import { html, PolymerElement } from '@polymer/polymer/polymer-element.js'; import { defineCustomElement } from '@vaadin/component-base/src/define.js'; import { DirMixin } from '@vaadin/component-base/src/dir-mixin.js'; -import { OverlayMixin } from '@vaadin/overlay/src/vaadin-overlay-mixin.js'; -import { PositionMixin } from '@vaadin/overlay/src/vaadin-overlay-position-mixin.js'; import { overlayStyles } from '@vaadin/overlay/src/vaadin-overlay-styles.js'; import { registerStyles, ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js'; +import { DatePickerOverlayMixin } from './vaadin-date-picker-overlay-mixin.js'; import { datePickerOverlayStyles } from './vaadin-date-picker-overlay-styles.js'; registerStyles('vaadin-date-picker-overlay', [overlayStyles, datePickerOverlayStyles], { @@ -21,13 +20,12 @@ registerStyles('vaadin-date-picker-overlay', [overlayStyles, datePickerOverlaySt * * @customElement * @extends HTMLElement - * @mixes PositionMixin - * @mixes OverlayMixin + * @mixes DatePickerOverlayMixin * @mixes DirMixin * @mixes ThemableMixin * @private */ -class DatePickerOverlay extends PositionMixin(OverlayMixin(DirMixin(ThemableMixin(PolymerElement)))) { +class DatePickerOverlay extends DatePickerOverlayMixin(DirMixin(ThemableMixin(PolymerElement))) { static get is() { return 'vaadin-date-picker-overlay'; } diff --git a/packages/date-picker/src/vaadin-lit-date-picker-overlay.js b/packages/date-picker/src/vaadin-lit-date-picker-overlay.js index 37bdcc5cdd..55293ad0ac 100644 --- a/packages/date-picker/src/vaadin-lit-date-picker-overlay.js +++ b/packages/date-picker/src/vaadin-lit-date-picker-overlay.js @@ -7,23 +7,21 @@ import { html, LitElement } from 'lit'; import { defineCustomElement } from '@vaadin/component-base/src/define.js'; import { DirMixin } from '@vaadin/component-base/src/dir-mixin.js'; import { PolylitMixin } from '@vaadin/component-base/src/polylit-mixin.js'; -import { OverlayMixin } from '@vaadin/overlay/src/vaadin-overlay-mixin.js'; -import { PositionMixin } from '@vaadin/overlay/src/vaadin-overlay-position-mixin.js'; import { overlayStyles } from '@vaadin/overlay/src/vaadin-overlay-styles.js'; import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js'; +import { DatePickerOverlayMixin } from './vaadin-date-picker-overlay-mixin.js'; import { datePickerOverlayStyles } from './vaadin-date-picker-overlay-styles.js'; /** * An element used internally by ``. Not intended to be used separately. * * @extends HTMLElement - * @mixes PositionMixin - * @mixes OverlayMixin + * @mixes DatePickerOverlayMixin * @mixes DirMixin * @mixes ThemableMixin * @private */ -class DatePickerOverlay extends PositionMixin(OverlayMixin(DirMixin(ThemableMixin(PolylitMixin(LitElement))))) { +class DatePickerOverlay extends DatePickerOverlayMixin(DirMixin(ThemableMixin(PolylitMixin(LitElement)))) { static get is() { return 'vaadin-date-picker-overlay'; } diff --git a/packages/date-picker/test/dropdown.common.js b/packages/date-picker/test/dropdown.common.js index 9c8bb7fd21..aec911586a 100644 --- a/packages/date-picker/test/dropdown.common.js +++ b/packages/date-picker/test/dropdown.common.js @@ -7,10 +7,9 @@ import { nextRender, nextUpdate, oneEvent, - outsideClick, touchstart, } from '@vaadin/testing-helpers'; -import { sendKeys } from '@web/test-runner-commands'; +import { resetMouse, sendKeys, sendMouse } from '@web/test-runner-commands'; import sinon from 'sinon'; import { getFocusedCell, monthsEqual, open, waitForOverlayRender } from './helpers.js'; @@ -187,19 +186,22 @@ describe('dropdown', () => { }); describe('outside click', () => { - it('should restore focus to the input on outside click', async () => { + afterEach(async () => { + await resetMouse(); + }); + + it('should keep focus in the input on outside mousedown', async () => { input.focus(); await open(datePicker); - outsideClick(); - await nextUpdate(datePicker); - await aTimeout(0); + await sendMouse({ type: 'move', position: [200, 10] }); + await sendMouse({ type: 'down' }); expect(document.activeElement).to.equal(input); }); it('should focus the input on outside click', async () => { expect(document.activeElement).to.equal(document.body); await open(datePicker); - outsideClick(); + await sendMouse({ type: 'click', position: [200, 10] }); await nextUpdate(datePicker); await aTimeout(0); expect(document.activeElement).to.equal(input); @@ -209,7 +211,7 @@ describe('dropdown', () => { // Focus the input with Tab await sendKeys({ press: 'Tab' }); await open(datePicker); - outsideClick(); + await sendMouse({ type: 'click', position: [200, 10] }); await nextUpdate(datePicker); await aTimeout(0); expect(datePicker.hasAttribute('focus-ring')).to.be.true; @@ -220,14 +222,14 @@ describe('dropdown', () => { input.focus(); // Move focus to the calendar await sendKeys({ press: 'Tab' }); - outsideClick(); + await sendMouse({ type: 'click', position: [200, 10] }); await aTimeout(0); expect(datePicker.hasAttribute('focus-ring')).to.be.true; }); it('should not set focus-ring attribute if it was not set before opening', async () => { await open(datePicker); - outsideClick(); + await sendMouse({ type: 'click', position: [200, 10] }); await aTimeout(0); expect(datePicker.hasAttribute('focus-ring')).to.be.false; }); diff --git a/test/integration/grid-pro-custom-editor.test.js b/test/integration/grid-pro-custom-editor.test.js index 59ec1b8b34..a8c09ac9d1 100644 --- a/test/integration/grid-pro-custom-editor.test.js +++ b/test/integration/grid-pro-custom-editor.test.js @@ -1,11 +1,12 @@ import { expect } from '@vaadin/chai-plugins'; -import { fixtureSync, nextRender } from '@vaadin/testing-helpers'; +import { fixtureSync, middleOfNode, nextRender } from '@vaadin/testing-helpers'; import { resetMouse, sendKeys, sendMouse } from '@web/test-runner-commands'; import '@vaadin/combo-box'; import '@vaadin/date-picker'; import '@vaadin/grid-pro'; import '@vaadin/grid-pro/vaadin-grid-pro-edit-column.js'; 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', () => { @@ -62,12 +63,16 @@ describe('grid-pro custom editor', () => { }); describe('date-picker', () => { - it('should apply the updated date to the cell when exiting on Tab', async () => { - const cell = getContainerCell(grid.$.items, 0, 2); + let cell; + + beforeEach(async () => { + cell = getContainerCell(grid.$.items, 0, 2); cell.focus(); await sendKeys({ press: 'Enter' }); + }); + it('should apply the updated date to the cell when exiting on Tab', async () => { await sendKeys({ type: '1/12/1984' }); await sendKeys({ press: 'Tab' }); @@ -75,16 +80,47 @@ describe('grid-pro custom editor', () => { }); it('should apply the updated date to the cell when exiting on Enter', async () => { - const cell = getContainerCell(grid.$.items, 0, 2); - cell.focus(); - - await sendKeys({ press: 'Enter' }); - await sendKeys({ type: '1/12/1984' }); await sendKeys({ press: 'Enter' }); expect(cell._content.textContent).to.equal('1984-01-12'); }); + + it('should not stop editing on input click when focus is in the overlay', async () => { + // Open the overlay + await sendKeys({ press: 'ArrowDown' }); + + const { x, y } = middleOfNode(cell._content.querySelector('input')); + await sendMouse({ type: 'click', position: [Math.floor(x), Math.floor(y)] }); + + expect(cell._content.querySelector('vaadin-date-picker')).to.be.ok; + }); + + it('should stop editing and update value when closing on outside click', async () => { + // Open the overlay + await sendKeys({ press: 'ArrowDown' }); + await waitForOverlayRender(); + + // Move focus back to the input + await sendKeys({ down: 'Shift' }); + await sendKeys({ press: 'Tab' }); + await sendKeys({ up: 'Shift' }); + + // Change single digit to avoid calendar scroll + const input = cell._content.querySelector('input'); + input.setSelectionRange(3, 4); + + await sendKeys({ type: '2' }); + + 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'); + }); }); describe('combo-box', () => {