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

feat: add clear-button-visible API from <vaadin-text-field> #648

Merged
merged 1 commit into from
Mar 28, 2019

Conversation

platosha
Copy link
Contributor

@platosha platosha commented Mar 20, 2019

Fixes #645

BREAKING CHANGE: The clear button is now hidden by default. To make it
visible, set the clear-button-visible attribute or use
clearButtonVisible JS API property.


This change is Reviewable

@platosha platosha force-pushed the feat/clear-button-visible branch 2 times, most recently from a3ca555 to f836e92 Compare March 20, 2019 14:47
@Haprog
Copy link
Contributor

Haprog commented Mar 20, 2019

Should probably also bump vaadin-text-field dependency version in bower.json from ^2.1.2 to ^2.3.0 since v2.3.0 introduces the clear-button-visible API.

@platosha
Copy link
Contributor Author

@Haprog done

@Haprog
Copy link
Contributor

Haprog commented Mar 26, 2019

@platosha Looks like in this version when you click on the clear button, it always opens the date picker, regardless of if the date picker input was focused or not and if the overlay was open or not. This is different from current behaviour in released version.

If you test it here: https://cdn.vaadin.com/vaadin-date-picker/3.3.2/demo/#date-picker-basic-demos clicking on the clear button doesn't open or close the date-picker, it just clears the value and focuses the text field. So if you click the clear button when the overlay is open, it stays open. And if you click the clear button when the overlay is closed, it stays closed.

We should probably add tests for this so that the behaviour doesn't unintentionally change. So we should test what happens when the clear button is clicked in at least these cases:

Clear button is clicked when:

  1. date picker is not focused, overlay is closed
  2. date picker input is focused, overlay is closed (e.g. right after selecting a date from the overlay which closes the overlay but leaves the text input focused)
  3. date picker overlay is open

demo/date-picker-basic-demos.html Outdated Show resolved Hide resolved
test/dropdown.html Outdated Show resolved Hide resolved
src/vaadin-date-picker.html Show resolved Hide resolved
Fixes #645

BREAKING CHANGE: The clear button is now hidden by default. To make it
visible, set the `clear-button-visible` attribute or use
`clearButtonVisible` JS API property.
@platosha platosha merged commit 318d0fc into master Mar 28, 2019
@platosha platosha deleted the feat/clear-button-visible branch March 28, 2019 12:22
@jtomass jtomass added this to ✅ Done in vaadin-core Mar 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
vaadin-core
  
✅ Done
Development

Successfully merging this pull request may close these issues.

Integrate text-field's clearButtonVisible API
2 participants