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: enable input into autoOpenDisabled date picker on mobile #3165

Merged
merged 13 commits into from
Dec 15, 2021
24 changes: 13 additions & 11 deletions packages/date-picker/src/vaadin-date-picker-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
* This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
*/
import { isIOS } from '@vaadin/component-base/src/browser-utils.js';
import { addListener } from '@vaadin/component-base/src/gestures.js';
import { KeyboardMixin } from '@vaadin/component-base/src/keyboard-mixin.js';
import { DelegateFocusMixin } from '@vaadin/field-base/src/delegate-focus-mixin.js';
import { InputMixin } from '@vaadin/field-base/src/input-mixin.js';
Expand Down Expand Up @@ -302,7 +301,7 @@ export const DatePickerMixin = (subclass) =>
/** @private */
_noInput: {
type: Boolean,
computed: '_isNoInput(inputElement, _fullscreen, _ios, i18n, i18n.*)'
computed: '_isNoInput(inputElement, _fullscreen, _ios, i18n, opened, autoOpenDisabled)'
},

/** @private */
Expand Down Expand Up @@ -415,17 +414,11 @@ export const DatePickerMixin = (subclass) =>
ready() {
super.ready();

addListener(this, 'tap', (e) => {
this.addEventListener('click', (e) => {
if (!this._isClearButton(e) && (!this.autoOpenDisabled || this._noInput)) {
this.open();
}
});

this.addEventListener('touchend', (e) => {
if (!this._isClearButton(e)) {
e.preventDefault();
}
});
Comment on lines -423 to -428
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Canceling the "touchend" events seems not to be relevant anymore. It was originally added 6 years ago and was probably related to preventing ghost clicks.

}

/** @protected */
Expand Down Expand Up @@ -541,8 +534,17 @@ export const DatePickerMixin = (subclass) =>
}

/** @private */
_isNoInput(inputElement, fullscreen, ios, i18n) {
return !inputElement || fullscreen || ios || !i18n.parseDate;
// eslint-disable-next-line max-params
_isNoInput(inputElement, fullscreen, ios, i18n, opened, autoOpenDisabled) {
// On fullscreen mode, text input is disabled if auto-open isn't disabled or
// whenever the dropdown is opened
const noInputOnFullscreenMode = fullscreen && (!autoOpenDisabled || opened);
// On iOS, text input is disabled whenever the dropdown is opened, because
// the virtual keyboard doesn't affect the viewport metrics and thus the
// dropdown could get covered by the keyboard.
const noInputOnIos = ios && opened;

return !inputElement || noInputOnFullscreenMode || noInputOnIos || !i18n.parseDate;
}

/** @private */
Expand Down
155 changes: 134 additions & 21 deletions packages/date-picker/test/basic.test.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,26 @@
import { expect } from '@esm-bundle/chai';
import { aTimeout, click, fixtureSync, oneEvent, tap } from '@vaadin/testing-helpers';
import { aTimeout, click, fixtureSync, makeSoloTouchEvent, oneEvent, tap } from '@vaadin/testing-helpers';
import { sendKeys } from '@web/test-runner-commands';
import sinon from 'sinon';
import '../src/vaadin-date-picker.js';
import * as settings from '@polymer/polymer/lib/utils/settings.js';
import { close, getOverlayContent, monthsEqual, open } from './common.js';

settings.setCancelSyntheticClickEvents(false);

async function touchTap(target) {
const start = makeSoloTouchEvent('touchstart', null, target);
const end = makeSoloTouchEvent('touchend', null, target);
if (!start.defaultPrevented && !end.defaultPrevented) {
target.click();
target.focus();
}
}

function isFocused(target) {
return target.getRootNode().activeElement === target;
}

describe('basic features', () => {
let datepicker, input, toggleButton;

Expand Down Expand Up @@ -96,14 +112,21 @@ describe('basic features', () => {
await oneEvent(datepicker.$.overlay, 'vaadin-overlay-open');
});

it('should not open on input tap when autoOpenDisabled is true and not on mobile', () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was moved inside "auto open disabled"

datepicker.autoOpenDisabled = true;
tap(input);
if (!datepicker._noInput) {
expect(datepicker.opened).not.to.be.true;
} else {
expect(datepicker.opened).to.be.true;
}
it('should focus the input on touch tap', () => {
touchTap(input);
expect(isFocused(input)).to.be.true;
});

it('should not focus the input on touch tap on fullscreen', () => {
datepicker._fullscreen = true;
touchTap(input);
expect(isFocused(input)).to.be.false;
});

it('should blur the input on fullscreen', () => {
datepicker._fullscreen = true;
datepicker.focus();
expect(isFocused(input)).to.be.false;
});

it('should pass the placeholder attribute to the input tag', () => {
Expand Down Expand Up @@ -188,12 +211,6 @@ describe('basic features', () => {
await oneEvent(datepicker.$.overlay, 'vaadin-overlay-open');
});

it('should open by tapping the calendar icon even if autoOpenDisabled is true', async () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was moved inside "auto open disabled"

window.autoOpenDisabled = true;
tap(toggleButton);
await oneEvent(datepicker.$.overlay, 'vaadin-overlay-open');
});

it('should scroll to a date on open', async () => {
const overlayContent = getOverlayContent(datepicker);
// We must scroll to a date on every open because at least IE11 seems to reset
Expand Down Expand Up @@ -504,7 +521,7 @@ describe('inside flexbox', () => {
});
});

describe('clear-button-visible', () => {
describe('clear button', () => {
let datepicker, clearButton;

beforeEach(() => {
Expand All @@ -516,17 +533,16 @@ describe('clear-button-visible', () => {
expect(datepicker).to.have.property('clearButtonVisible', true);
});

it('should clear the value', () => {
it('should clear the value on click', () => {
datepicker.value = '2000-02-01';
click(clearButton);
expect(datepicker.value).to.equal('');
});

it('should not prevent touchend event on clear button', () => {
it('should clear the value on touch tap', () => {
datepicker.value = '2000-02-01';
const e = new CustomEvent('touchend', { cancelable: true });
clearButton.dispatchEvent(e);
expect(e.defaultPrevented).to.be.false;
touchTap(clearButton);
expect(datepicker.value).to.equal('');
});

it('should validate on clear', () => {
Expand Down Expand Up @@ -576,3 +592,100 @@ describe('initial value attribute', () => {
expect(input.value).to.equal('1/1/2000');
});
});

describe('auto open disabled', () => {
let datepicker, input, toggleButton;

beforeEach(() => {
datepicker = fixtureSync('<vaadin-date-picker value="2000-01-01"></vaadin-date-picker>');
input = datepicker.inputElement;
toggleButton = datepicker.shadowRoot.querySelector('[part="toggle-button"]');
datepicker.autoOpenDisabled = true;
});

it('should focus the input on touch tap', () => {
touchTap(input);
expect(isFocused(input)).to.be.true;
});

it('should not blur the input on open', async () => {
touchTap(input);
await open(datepicker);
expect(isFocused(input)).to.be.true;
});

it('should blur the input on fullscreen open', async () => {
datepicker._fullscreen = true;
touchTap(input);
await open(datepicker);
expect(isFocused(input)).to.be.false;
});

it('should not open on input tap when autoOpenDisabled is true and not on mobile', () => {
tap(input);
expect(datepicker.opened).not.to.be.true;
});

it('should open by tapping the calendar icon even if autoOpenDisabled is true', async () => {
tap(toggleButton);
await oneEvent(datepicker.$.overlay, 'vaadin-overlay-open');
});
});

describe('ios', () => {
let datepicker, input;

beforeEach(() => {
datepicker = fixtureSync('<vaadin-date-picker value="2000-01-01"></vaadin-date-picker>');
input = datepicker.inputElement;
datepicker._ios = true;
});

it('should focus the input when closed', () => {
vursen marked this conversation as resolved.
Show resolved Hide resolved
datepicker.focus();
expect(isFocused(input)).to.be.true;
});

it('should blur the input when opened', async () => {
datepicker.focus();
await open(datepicker);
expect(isFocused(input)).to.be.false;
});

describe('auto open disabled', () => {
let toggleButton;

beforeEach(() => {
datepicker.autoOpenDisabled = true;
toggleButton = datepicker.shadowRoot.querySelector('[part="toggle-button"]');
});

it('should focus the input on touch tap', () => {
touchTap(input);
expect(isFocused(input)).to.be.true;
});

it('should blur the input on open', async () => {
touchTap(input);
await open(datepicker);
expect(isFocused(input)).to.be.false;
});

it('should blur the input on fullscreen open', async () => {
datepicker._fullscreen = true;
touchTap(input);
await open(datepicker);
expect(isFocused(input)).to.be.false;
});

it('should not open on input tap when autoOpenDisabled is true and not on mobile', () => {
vursen marked this conversation as resolved.
Show resolved Hide resolved
tap(input);
expect(datepicker.opened).not.to.be.true;
});

it('should open by tapping the calendar icon even if autoOpenDisabled is true', async () => {
tap(toggleButton);
await oneEvent(datepicker.$.overlay, 'vaadin-overlay-open');
});
});
});
5 changes: 4 additions & 1 deletion packages/date-picker/test/keyboard-input.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,10 @@ import { close, getOverlayContent, open } from './common.js';

describe('no parseDate', () => {
beforeEach(() => {
datepicker.set('i18n.parseDate', null);
datepicker.i18n = {
...datepicker.i18n,
parseDate: null
};
});

it('should prevent key input', () => {
Expand Down