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

test: update date-picker tests to not use async close helper #8260

Merged
merged 1 commit into from
Dec 2, 2024

Conversation

web-padawan
Copy link
Member

@web-padawan web-padawan commented Dec 2, 2024

Description

UPD: actually there is a case with overlay blinking label click with the below changes, so I'll revert it for now.

The vaadin-overlay-close listener was added vaadin/vaadin-date-picker#498 - back then, date-picker has old clear button logic, which is no longer in use. As of today, the only case where that event actually contains sourceEvent is the "outside click" and the condition inside the listener appears to never be true.

Also, there is no need to wait for that event after changing close() method to set opened = false on the host (which is how other dropdown components work, e.g. vaadin-combo-box and vaadin-time-picker). In fact, unlike opening the overlay, closing is always synchronous so we can safely use datePicker.close() in tests.

Type of change

  • Refactor

@web-padawan web-padawan requested a review from vursen December 2, 2024 13:56
@web-padawan
Copy link
Member Author

UPD: did some testing and with this change the date-picker blinks on label click, so the listener actually makes sense.
I will revert that part of the PR and add a test to ensure overlay is not closed. Other changes can be preserved though.

@web-padawan web-padawan force-pushed the refactor/date-picker-close branch from e415e90 to 0bc8567 Compare December 2, 2024 14:06
@web-padawan web-padawan changed the title refactor: cleanup date-picker closing logic, simplify tests test: update date-picker tests to not use async close helper Dec 2, 2024
Copy link

sonarqubecloud bot commented Dec 2, 2024

@web-padawan
Copy link
Member Author

web-padawan commented Dec 2, 2024

Edited the PR to be only about test changes for now as the listener is actually useful.
I'll investigate if we can simplify handling label click taking the #7812 into account.

@web-padawan web-padawan merged commit 4f474c9 into main Dec 2, 2024
9 checks passed
@web-padawan web-padawan deleted the refactor/date-picker-close branch December 2, 2024 14:12
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.6.0.beta3 and is also targeting the upcoming stable 24.6.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.

3 participants