Skip to content

Commit

Permalink
refactor: cleanup date-picker closing logic, simplify tests
Browse files Browse the repository at this point in the history
  • Loading branch information
web-padawan committed Dec 2, 2024
1 parent c5bc5bc commit e415e90
Show file tree
Hide file tree
Showing 10 changed files with 20 additions and 45 deletions.
2 changes: 1 addition & 1 deletion packages/date-picker/src/vaadin-date-picker-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ export const DatePickerMixin = (subclass) =>
* Closes the dropdown.
*/
close() {
this.$.overlay.close();
this.opened = false;
}

/** @private */
Expand Down
9 changes: 0 additions & 9 deletions packages/date-picker/src/vaadin-date-picker.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,15 +241,6 @@ class DatePicker extends DatePickerMixin(InputControlMixin(ThemableMixin(Element

const toggleButton = this.shadowRoot.querySelector('[part="toggle-button"]');
toggleButton.addEventListener('mousedown', (e) => e.preventDefault());

this.$.overlay.addEventListener('vaadin-overlay-close', this._onVaadinOverlayClose.bind(this));
}

/** @private */
_onVaadinOverlayClose(e) {
if (e.detail.sourceEvent && e.detail.sourceEvent.composedPath().includes(this)) {
e.preventDefault();
}
}

/** @private */
Expand Down
8 changes: 0 additions & 8 deletions packages/date-picker/src/vaadin-lit-date-picker.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ class DatePicker extends DatePickerMixin(InputControlMixin(ThemableMixin(Element
@opened-changed="${this._onOpenedChanged}"
@vaadin-overlay-escape-press="${this._onOverlayEscapePress}"
@vaadin-overlay-open="${this._onOverlayOpened}"
@vaadin-overlay-close="${this._onVaadinOverlayClose}"
@vaadin-overlay-closing="${this._onOverlayClosed}"
restore-focus-on-close
no-vertical-overlap
Expand Down Expand Up @@ -148,13 +147,6 @@ class DatePicker extends DatePickerMixin(InputControlMixin(ThemableMixin(Element
this.opened = event.detail.value;
}

/** @private */
_onVaadinOverlayClose(e) {
if (e.detail.sourceEvent && e.detail.sourceEvent.composedPath().includes(this)) {
e.preventDefault();
}
}

/** @private */
_toggle(e) {
e.stopPropagation();
Expand Down
6 changes: 3 additions & 3 deletions packages/date-picker/test/basic.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { click, fixtureSync, keyboardEventFor, nextRender, oneEvent, tap } from
import { sendKeys } from '@web/test-runner-commands';
import sinon from 'sinon';
import { parseDate } from '../src/vaadin-date-picker-helper.js';
import { close, open, touchTap, waitForOverlayRender } from './helpers.js';
import { open, touchTap, waitForOverlayRender } from './helpers.js';

describe('basic features', () => {
let datePicker, input, overlay;
Expand Down Expand Up @@ -60,13 +60,13 @@ describe('basic features', () => {
datePicker.addEventListener('opened-changed', spy);
await open(datePicker);
expect(spy.calledOnce).to.be.true;
await close(datePicker);
datePicker.close();
expect(spy.calledTwice).to.be.true;
});

it('should set opened to false with close call', async () => {
await open(datePicker);
await close(datePicker);
datePicker.close();
expect(datePicker.opened).to.be.false;
});

Expand Down
6 changes: 3 additions & 3 deletions packages/date-picker/test/events.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { expect } from '@vaadin/chai-plugins';
import { fixtureSync, nextRender } from '@vaadin/testing-helpers';
import { sendKeys } from '@web/test-runner-commands';
import sinon from 'sinon';
import { close, open, waitForOverlayRender, waitForScrollToFinish } from './helpers.js';
import { open, waitForOverlayRender, waitForScrollToFinish } from './helpers.js';

describe('events', () => {
let datePicker;
Expand All @@ -21,15 +21,15 @@ describe('events', () => {
it('should not be fired on programmatic value change when opened', async () => {
await open(datePicker);
datePicker.value = '2000-01-01';
await close(datePicker);
datePicker.close();
expect(changeSpy.called).to.be.false;
});

it('should not be fired on programmatic value change when having user input', async () => {
await sendKeys({ type: '1/2/2000' });
await waitForScrollToFinish(datePicker);
datePicker.value = '2000-01-01';
await close(datePicker);
datePicker.close();
expect(changeSpy.called).to.be.false;
});
});
Expand Down
9 changes: 1 addition & 8 deletions packages/date-picker/test/helpers.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { fire, listenOnce, makeSoloTouchEvent, nextRender } from '@vaadin/testing-helpers';
import { fire, makeSoloTouchEvent, nextRender } from '@vaadin/testing-helpers';
import { flush } from '@polymer/polymer/lib/utils/flush.js';
import { afterNextRender } from '@polymer/polymer/lib/utils/render-status.js';
import { isElementFocused } from '@vaadin/a11y-base/src/focus-utils.js';
Expand Down Expand Up @@ -60,13 +60,6 @@ export async function open(datePicker) {
await waitForOverlayRender();
}

export function close(datePicker) {
return new Promise((resolve) => {
listenOnce(datePicker.$.overlay, 'vaadin-overlay-close', resolve);
datePicker.close();
});
}

export function idleCallback() {
return new Promise((resolve) => {
if (window.requestIdleCallback) {
Expand Down
7 changes: 3 additions & 4 deletions packages/date-picker/test/keyboard-input.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { sendKeys } from '@web/test-runner-commands';
import sinon from 'sinon';
import { formatISODate, getAdjustedYear, parseDate } from '../src/vaadin-date-picker-helper.js';
import {
close,
getFocusableCell,
getFocusedCell,
idleCallback,
Expand Down Expand Up @@ -145,7 +144,7 @@ describe('keyboard', () => {

it('should select focused date on close', async () => {
await open(datePicker);
await close(datePicker);
datePicker.close();
expect(datePicker._selectedDate).to.equal(datePicker._focusedDate);
});
});
Expand Down Expand Up @@ -292,10 +291,10 @@ describe('keyboard', () => {
expect(spy.called).to.be.true;
});

it('should clear selection on close', async () => {
it('should clear selection on close', () => {
input.select();

await close(datePicker);
datePicker.close();
expect(input.selectionStart).to.equal(input.selectionEnd);
});

Expand Down
4 changes: 2 additions & 2 deletions packages/date-picker/test/validation.common.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { expect } from '@vaadin/chai-plugins';
import { enter, fixtureSync, nextRender, nextUpdate } from '@vaadin/testing-helpers';
import sinon from 'sinon';
import { close, open, setInputValue, waitForOverlayRender } from './helpers.js';
import { open, setInputValue, waitForOverlayRender } from './helpers.js';

class DatePicker2016 extends customElements.get('vaadin-date-picker') {
checkValidity() {
Expand Down Expand Up @@ -133,7 +133,7 @@ describe('validation', () => {
expect(datePicker.validate()).to.be.false;
await open(datePicker);
datePicker.value = '2000-02-01';
await close(datePicker);
datePicker.close();
expect(datePicker.invalid).to.be.false;
});

Expand Down
6 changes: 3 additions & 3 deletions packages/date-picker/test/wai-aria.common.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expect } from '@vaadin/chai-plugins';
import { fixtureSync, nextFrame, nextRender } from '@vaadin/testing-helpers';
import { activateScroller, close, getDefaultI18n, open } from './helpers.js';
import { activateScroller, getDefaultI18n, open } from './helpers.js';

describe('WAI-ARIA', () => {
describe('date picker', () => {
Expand All @@ -15,7 +15,7 @@ describe('WAI-ARIA', () => {
it('should toggle aria-expanded attribute on open', async () => {
await open(datePicker);
expect(input.getAttribute('aria-expanded')).to.equal('true');
await close(datePicker);
datePicker.close();
expect(input.getAttribute('aria-expanded')).to.equal('false');
});

Expand Down Expand Up @@ -113,7 +113,7 @@ describe('WAI-ARIA', () => {

it('should remove aria-hidden from other elements when overlay is closed', async () => {
await open(datePicker);
await close(datePicker);
datePicker.close();
expect(button.hasAttribute('aria-hidden')).to.be.false;
expect(input.hasAttribute('aria-hidden')).to.be.false;
});
Expand Down
8 changes: 4 additions & 4 deletions packages/field-highlighter/test/field-components.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import '@vaadin/radio-group';
import '@vaadin/select';
import '@vaadin/text-field';
import { html, render } from 'lit';
import { close, waitForOverlayRender } from '@vaadin/date-picker/test/helpers.js';
import { waitForOverlayRender } from '@vaadin/date-picker/test/helpers.js';
import { FieldHighlighter } from '../src/vaadin-field-highlighter.js';

async function waitForIntersectionObserver() {
Expand Down Expand Up @@ -93,7 +93,7 @@ describe('field components', () => {
field.focus();
await open(field);
await waitForOverlayRender();
await close(field);
field.close();
expect(hideSpy.callCount).to.equal(0);
});

Expand All @@ -102,7 +102,7 @@ describe('field components', () => {
await waitForOverlayRender();

field.focus();
await close(field);
field.close();

expect(hideSpy.callCount).to.equal(0);
});
Expand Down Expand Up @@ -477,7 +477,7 @@ describe('field components', () => {
// Focus date element and then time-picker
await date._overlayContent.focusDateElement();
time.focus();
await close(date);
date.close();

expect(hideSpy.callCount).to.equal(1);
expect(hideSpy.firstCall.args[0].detail.fieldIndex).to.equal(0);
Expand Down

0 comments on commit e415e90

Please sign in to comment.