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.

Sign up for GitHub

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

Jump to bottom

fix: commit focused date on outside click #5670

Merged
merged 7 commits into from
Mar 16, 2023
Merged

fix: commit focused date on outside click #5670

merged 7 commits into from
Mar 16, 2023

Conversation

ugur-vaadin
Copy link
Contributor

@ugur-vaadin ugur-vaadin commented Mar 14, 2023 •
edited

Description

Currently, on selection via Enter key, Space key, or Today button, the selection happens instantaneously. A possible duplication of the selection and validation on overlay close is avoided by setting __userConfirmedDate. When Space is used to deselect an already selected item, the absence of a __userConfirmedDate is not properly communicated. This PR sets __userConfirmedDate based on whether the event is a selection or a deselection.

Fixes #5640

Type of change

  • Bugfix
  • Feature

Checklist

  • I have read the contribution guide: https://vaadin.com/docs/latest/contributing/overview
  • I have added a description following the guideline.
  • The issue is created in the corresponding repository and I have referenced it.
  • I have added tests to ensure my change is effective and works as intended.
  • New and existing tests are passing locally with my change.
  • I have performed self-review and corrected misspellings.

@ugur-vaadin ugur-vaadin linked an issue Mar 14, 2023 that may be closed by this pull request
[date-picker] The input element is not cleared when closing the dropdown after deselecting a date #5640
Closed
@ugur-vaadin ugur-vaadin marked this pull request as draft March 14, 2023 14:57
@ugur-vaadin ugur-vaadin marked this pull request as ready for review March 14, 2023 15:06
@ugur-vaadin ugur-vaadin changed the title fix: apply input value on outside click fix: commit focused date on outside click Mar 15, 2023
@vursen
Copy link
Contributor

vursen commented Mar 16, 2023 •
edited

It seems that the reason the user input doesn't get committed on overlay close is because deselecting a date with Space doesn't reset __userConfirmedDate, so it wrongly goes to this branch:

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

Lines 910 to 912 in 0e72706

if (this.__userConfirmedDate) {
this.__userConfirmedDate = false;
} else {

@ugur-vaadin ugur-vaadin removed the request for review from bwajtr March 16, 2023 09:23
@sonarcloud
Copy link

sonarcloud bot commented Mar 16, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ugur-vaadin ugur-vaadin merged commit 8dd1860 into main Mar 16, 2023
@ugur-vaadin ugur-vaadin deleted the 5640-date-picker-the-input-element-is-not-cleared-when-closing-the-dropdown-after-deselecting-a-date branch March 16, 2023 10:24
vaadin-bot pushed a commit that referenced this pull request Mar 16, 2023
@ugur-vaadin @vaadin-bot
fix: commit focused date on outside click (#5670)
1e3586b 
* fix: apply input value on outside click

* fix: select focused date on outside click except input element click

* fix: use default selection behaviour

* fix: reset userConfirmedDate if a date is deselected

* test: add deselect test

* test: move test to dropdown tests

* test: convert arrowdown to arrowright and change test date
@vaadin-bot
Copy link
Collaborator

Hi @ugur-vaadin , this commit cannot be picked to 23.3 by this bot, can you take a look and pick it manually?
Error Message: Error: Command failed: git cherry-pick 8dd1860
error: could not apply 8dd1860... fix: commit focused date on outside click (#5670)
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".

@ugur-vaadin ugur-vaadin changed the title fix: commit focused date on outside click fix: set userConfirmedDate based on whether the event is a selection or a deselection Mar 16, 2023
ugur-vaadin added a commit that referenced this pull request Mar 17, 2023
@vaadin-bot @ugur-vaadin
fix: commit focused date on outside click (#5670) (#5676)
67984fc 
* fix: apply input value on outside click

* fix: select focused date on outside click except input element click

* fix: use default selection behaviour

* fix: reset userConfirmedDate if a date is deselected

* test: add deselect test

* test: move test to dropdown tests

* test: convert arrowdown to arrowright and change test date

Co-authored-by: Ugur Saglam <106508695+ugur-vaadin@users.noreply.github.com>
@ugur-vaadin ugur-vaadin mentioned this pull request Mar 17, 2023
Merged
8 tasks
@ugur-vaadin ugur-vaadin changed the title fix: set userConfirmedDate based on whether the event is a selection or a deselection fix: commit focused date on outside click Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vursen vursen vursen approved these changes

Assignees
No one assigned
Labels
cherry-picked-24.0 need to pick manually 23.3 target/23.3 target/24.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[date-picker] The input element is not cleared when closing the dropdown after deselecting a date
3 participants
@ugur-vaadin @vursen @vaadin-bot