Skip to content

Commit

Permalink
fix!: prevent focusout when closing date-picker on outside click (#7855)
Browse files Browse the repository at this point in the history
  • Loading branch information
web-padawan authored Sep 24, 2024
1 parent 48acc62 commit 5c59e40
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 28 deletions.
43 changes: 43 additions & 0 deletions packages/date-picker/src/vaadin-date-picker-overlay-mixin.js
Original file line number Diff line number Diff line change
@@ -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();
}
}
};
8 changes: 3 additions & 5 deletions packages/date-picker/src/vaadin-date-picker-overlay.js
Original file line number Diff line number Diff line change
Expand Up @@ -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], {
Expand All @@ -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';
}
Expand Down
8 changes: 3 additions & 5 deletions packages/date-picker/src/vaadin-lit-date-picker-overlay.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<vaadin-date-picker>`. 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';
}
Expand Down
22 changes: 12 additions & 10 deletions packages/date-picker/test/dropdown.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -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;
});
Expand Down
52 changes: 44 additions & 8 deletions test/integration/grid-pro-custom-editor.test.js
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -62,29 +63,64 @@ 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' });

expect(cell._content.textContent).to.equal('1984-01-12');
});

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', () => {
Expand Down

0 comments on commit 5c59e40

Please sign in to comment.