refactor: handle user-selected date properly

[web-padawan]

Description

This PR updates the date-picker logic in preparation for fixing events being fired in the wrong order.

Related to #3379

Problem

There are two different ways of how vaadin-date-picker value can be updated:

1. Programmatically, by setting value property with JS,
2. User-selected (by typing or selecting from the overlay).

In the first case, value is set and then internal _selectedDate property is updated accordingly. This works fine.

In the second case, _selectedDate is set first, then value is updated and then _selectedDate is set again:

web-components/packages/date-picker/src/vaadin-date-picker-mixin.js

Line 672 in 4be73e8

this._handleDateChange('_selectedDate', value, oldValue);

This part of logic is problematic because of how internal flags are used. Currently, there are two of them:

• __userInputOccured - set when _focusedDate changes (cover changing date in the overlay month calendar),
• __dispatchChange - set inside the _selectedDateChanged observer if __userInputOccured is set to true.

The actual problem with the current implementation is how these flags are reset in the second observer run.

1. Right now, when firing change event inside of the _valueChaned observer, the flag is still there.
2. If we move change event to value-changed event listener, by that time the flag is already removed.

This is understandable because Polymer runs notify events after the property observers.

Solution

1. Removed using two-way data binding for _selectedDate property to use one-way data flow instead.
2. Added _selectDate method to be called on user interactions only (e.g. selecting date with mouse or keyboard).
3. Moved setting __dispatchChange flag to _selectDate method to not modify it in _selectedDateChanged.
4. Added internal date-selected event to be used for keys, similarly to how date-tap is used for mouse clicks.

These changes would make it easier to move the logic for change event (will be done in a separate PR).

Type of change

• Refactor

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
0.0% Duplication

[tomivirkki]

If we move change event to value-changed event listener, by that time the flag is already removed.

Would either of these (presumably smaller changes) work:

1. The flag removal logic is also moved in an internal "value-changed" event listener:

__onValueChangedEvent() {
  if (this.__dispatchChange) {
    this.dispatchEvent(new CustomEvent('change', { bubbles: true }));
    this.__dispatchChange = false;
  }
}

2. Defer the change event by microtask timing

_valueChanged(value, oldValue) {
  if (this.__dispatchChange) {
    queueMicrotask(() => {
      this.dispatchEvent(new CustomEvent('change', { bubbles: true }));
    });
    this.__dispatchChange = false;
  }
  this._handleDateChange('_selectedDate', value, oldValue);

  this._toggleHasValue(!!value);
}

[tomivirkki]

Well, this approach is still cleaner since it eliminates the hacky __userInputOccurred flag

[web-padawan]

I wanted to avoid microtask timings because it's not how our Polymer based components work in general.
Regarding value-changed event, it could be an option but IMO this approach is cleaner.

[vaadin-bot]

Hi @web-padawan , this commit cannot be picked to 22.0 by this bot, can you take a look and pick it manually?
Error Message: Error: Command failed: git cherry-pick 247ba84
error: could not apply 247ba84... refactor: handle user-selected date properly (#3530)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

[vaadin-bot]

This ticket/PR has been released with Vaadin 23.0.2.

[vaadin-bot]

This ticket/PR has been released with Vaadin 23.1.0.alpha1 and is also targeting the upcoming stable 23.1.0 version.