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!: align how clear button works with combo box behavior #6100

Merged
merged 14 commits into from
Jul 7, 2023

Conversation

DiegoCardoso
Copy link
Contributor

@DiegoCardoso DiegoCardoso commented Jul 4, 2023

Description

Make the behavior of clicking the clear button to be aligned with how <vaadin-combo-box> works.

This change:

  • adds a touchstart listener that prevent default, and clears the input element, BUT does not focus it
  • adds a mousedown listener that prevents default, focuses the input element, AND clears it
  • keep a click listener that does the same mousedown behavior, mostly used for programmatic clicks on the button

These changes also fix the issue described in vaadin/flow-components#5137, since it prevents the focusout event and the editor won't close after clicking the clear button.

Fixes #4130
Part of vaadin/flow-components#5137

Type of change

  • Bugfix
  • Feature

Make the behavior of clicking the clear button to be align with how `vaadin-combo-box` works.

This change:
- adds a touchstart listener that prevenst default, clears the input element, BUT does not focus it
- adds a mousedown listener that prevents default, focuses the input element, AND clears it
- keep a click listener that does the same mousedown behavior, mostly used for programmatically clicks on the button
vursen
vursen previously approved these changes Jul 5, 2023
packages/field-base/test/clear-button-mixin.test.js Outdated Show resolved Hide resolved
packages/field-base/test/clear-button-mixin.test.js Outdated Show resolved Hide resolved
@vursen vursen dismissed their stale review July 5, 2023 13:53

Approved by accident

Copy link
Contributor

@vursen vursen left a comment

Choose a reason for hiding this comment

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

I manually tested text-field, date-picker, time-picker, combo-box on iOS and their clear button functionality now works consistently.

@sonarcloud
Copy link

sonarcloud bot commented Jul 6, 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

@vaadin-bot
Copy link
Collaborator

Hi @DiegoCardoso , this commit cannot be picked to 24.0 by this bot, can you take a look and pick it manually?
Error Message: Error: Command failed: git cherry-pick b7823f9
error: could not apply b7823f9... fix!: align how clear button works with combo-box behavior (#6100)
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
Copy link
Collaborator

Hi @DiegoCardoso , 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 b7823f9
error: could not apply b7823f9... fix!: align how clear button works with combo-box behavior (#6100)
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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Align clear button focus behavior in field components
4 participants