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

ELEMENTS-1450: compute validity based on the current value #495

Merged

Conversation

mnixo
Copy link
Member

@mnixo mnixo commented Dec 17, 2021

Context

  • On an auto search, the results are updated as the user tweaks the values of the widgets in the search form layout.
  • When value is changed, the form is validated and, if successful, the appropriate results are shown.
  • When using a <nuxeo-date-picker> the user is able to set a date and the results are updated automatically.
  • However, when changing the date to another value, the elements shows itself as invalid and therefore the results are not updated.
  • This seems to be caused to the way <vaadin-date-picker> checks the validity of its value:
    • As the user picks a second date, the change is picked up by the form layout and Web UI automatically tries to validate the form to present the new results.
    • While validating the form, <vaadin-date-picker> does its own validation but doesn't seem to be ready, as at this moment there's still a difference between value (same as this._inputValue, still the first date the user picked) and this._selectedDate (which is already the second date picked).

Proposed Solution

  • By passing this.value in this.$.date.validate() it ensures that the comparison is done with the most up to date value, instead of relying on the outdated this._inputValue.
  • Since the comparison is done with formatted date values and to ensure a smooth outcome, we should be using the same formatting that <vaadin-date-picker> uses (relies on locales), but since <nuxeo-date-picker> has no access to the Vaadin global variable and its helpers, I propose using the moment formatting (also based on locales) that is already used throughout <nuxeo-date-picker> we should just use this.$.date.i18n.formatDate().

@mnixo mnixo requested a review from a team as a code owner December 17, 2021 17:03
@nuxeojenkins
Copy link
Contributor

View issue in JIRA: ELEMENTS-1450: Fix auto-search when changing the select in nuxeo-date-picker

Gabez0r
Gabez0r previously approved these changes Dec 20, 2021
Copy link
Collaborator

@Gabez0r Gabez0r left a comment

Choose a reason for hiding this comment

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

LGTM.

ui/widgets/nuxeo-date-picker.js Outdated Show resolved Hide resolved
richardsd
richardsd previously approved these changes Dec 21, 2021
@mnixo mnixo dismissed stale reviews from richardsd and Gabez0r via f75a353 December 21, 2021 16:51
@mnixo mnixo force-pushed the ELEMENTS-1450-fix-date-picker-validation-3.0.x branch from 30d6d44 to f75a353 Compare December 21, 2021 16:51
richardsd
richardsd previously approved these changes Dec 21, 2021
Gabez0r
Gabez0r previously approved these changes Dec 22, 2021
semisse
semisse previously approved these changes Dec 30, 2021
@mnixo
Copy link
Member Author

mnixo commented Jan 11, 2022

At last, 🔵 pipeline run.

@Gabez0r Gabez0r dismissed stale reviews from semisse, richardsd, and themself via b9a1828 February 10, 2022 14:54
@Gabez0r Gabez0r force-pushed the ELEMENTS-1450-fix-date-picker-validation-3.0.x branch from f75a353 to b9a1828 Compare February 10, 2022 14:54
@Gabez0r
Copy link
Collaborator

Gabez0r commented Feb 11, 2022

@Gabez0r Gabez0r merged commit c25597f into maintenance-3.0.x Feb 11, 2022
@Gabez0r Gabez0r deleted the ELEMENTS-1450-fix-date-picker-validation-3.0.x branch February 11, 2022 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants