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

refactor: extract overlay auto add controller #7028

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

sissbruecker
Copy link
Contributor

@sissbruecker sissbruecker commented Jan 10, 2025

This is another attempt at extracting common behavior for automatically adding overlay-based components to the UI. The previous PR (#6620) slightly modified the auto-removal behavior by running it in beforeClientResponse. That in turn caused a regression in Dialog, so the previous PR was reverted.

The reason for delaying the auto-close behavior was that:

  • Flow always syncs properties first before running event listeners
  • That means the opened property change listener in OverlayAutoAddController runs first and removes the component from the UI
  • Event listeners are not run if the element is not in the UI
  • In ConfirmDialog that can be a problem because clicking confirm / cancel buttons closes the dialog from the client (opened property change) and also sends confirm / cancel events which are then not be processed

As an alternative, this PR changes ConfirmDialog to:

  • Not sync the opened property from client to server
  • Instead use an opened-changed listener to change the opened property on the server
  • With that other event listeners are still run, even if the opened-changed listener removes the component from the UI - the UI check only happens once before running all listeners
  • Behavior should still be the same as before, where opened-changed also auto-removed the component

@sissbruecker sissbruecker marked this pull request as ready for review February 7, 2025 11:14
# Conflicts:
#	vaadin-confirm-dialog-flow-parent/vaadin-confirm-dialog-flow/src/main/java/com/vaadin/flow/component/confirmdialog/ConfirmDialog.java
Copy link

sonarqubecloud bot commented Feb 7, 2025

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

Successfully merging this pull request may close these issues.

2 participants