Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix!: prevent focusout when closing date-picker on outside click (#7855) (CP: 24.5) #7863

Merged
merged 1 commit into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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