Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: remove modal component on route refresh #20540

Merged

Conversation

mcollovati
Copy link
Collaborator

Description

Modal components attached to the UI were not removed or replaced during self-navigation triggered by a route refresh.
This change updates navigation handler to ensure modal components are removed and adds a new navigation trigger for route refresh to differentiate programmatic navigations (e.g., forward actions).
It also modifies Hotswapper to require a full chain refresh when modal components are present.

Fixes #20473

Type of change

  • Bugfix
  • Feature

Checklist

  • I have read the contribution guide: https://vaadin.com/docs/latest/guide/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.

Additional for Feature type of change

  • Enhancement / new feature was discussed in a corresponding GitHub issue and Acceptance Criteria were created.

Copy link

github-actions bot commented Nov 25, 2024

Test Results

1 158 files  ± 0  1 158 suites  ±0   1h 33m 8s ⏱️ - 1m 37s
7 516 tests + 3  7 463 ✅ + 4  53 💤 ±0  0 ❌ ±0 
7 884 runs  +23  7 821 ✅ +24  63 💤 ±0  0 ❌ ±0 

Results for commit 2d0834e. ± Comparison against base commit 9e72f92.

♻️ This comment has been updated with latest results.

@mcollovati mcollovati marked this pull request as draft November 25, 2024 11:15
Modal components attached to the UI were not removed or replaced during
self-navigation triggered by a route refresh.
This change updates navigation handler to ensure modal components are
removed and adds a new navigation trigger for route refresh to differentiate
programmatic navigations (e.g., forward actions).
It also modifies Hotswapper to require a full chain refresh when modal
components are present.

Fixes #20473
@mcollovati mcollovati force-pushed the issues/20473-remove-modal-components-on-route-refresh branch from bc0379c to 4ef3417 Compare November 25, 2024 15:44
@mcollovati mcollovati marked this pull request as ready for review November 25, 2024 15:44
@mcollovati mcollovati merged commit 22d019b into main Nov 29, 2024
26 checks passed
@mcollovati mcollovati deleted the issues/20473-remove-modal-components-on-route-refresh branch November 29, 2024 11:01
vaadin-bot pushed a commit that referenced this pull request Nov 29, 2024
Modal components attached to the UI were not removed or replaced during
self-navigation triggered by a route refresh.
This change updates navigation handler to ensure modal components are
removed and adds a new navigation trigger for route refresh to differentiate
programmatic navigations (e.g., forward actions).
It also modifies Hotswapper to require a full chain refresh when modal
components are present.

Fixes #20473
vaadin-bot pushed a commit that referenced this pull request Nov 29, 2024
Modal components attached to the UI were not removed or replaced during
self-navigation triggered by a route refresh.
This change updates navigation handler to ensure modal components are
removed and adds a new navigation trigger for route refresh to differentiate
programmatic navigations (e.g., forward actions).
It also modifies Hotswapper to require a full chain refresh when modal
components are present.

Fixes #20473
vaadin-bot added a commit that referenced this pull request Nov 29, 2024
Modal components attached to the UI were not removed or replaced during
self-navigation triggered by a route refresh.
This change updates navigation handler to ensure modal components are
removed and adds a new navigation trigger for route refresh to differentiate
programmatic navigations (e.g., forward actions).
It also modifies Hotswapper to require a full chain refresh when modal
components are present.

Fixes #20473

Co-authored-by: Marco Collovati <marco@vaadin.com>
vaadin-bot added a commit that referenced this pull request Nov 29, 2024
Modal components attached to the UI were not removed or replaced during
self-navigation triggered by a route refresh.
This change updates navigation handler to ensure modal components are
removed and adds a new navigation trigger for route refresh to differentiate
programmatic navigations (e.g., forward actions).
It also modifies Hotswapper to require a full chain refresh when modal
components are present.

Fixes #20473

Co-authored-by: Marco Collovati <marco@vaadin.com>
@netsrotr
Copy link

netsrotr commented Dec 5, 2024

We have tested Vaadin 24.5.7 and this fix cause issues on our side with our hotswap -it close opened modal dialogs randomly and influence the browser refresh behavior (e.g. via key F5 in chrome).
It would really help if you would provide any documentation how to setup a working hotswap environment (favor eclipse IDE, but IntelliJ would also fit), where your fix really work. For now we stoped using hotswap because of that, any older version 24.5.x before was better than that, sorry.

@mcollovati
Copy link
Collaborator Author

@netsrotr do you mind creating a new ticket, providing a code example (or even better, a sample project) showing how you are using dialogs in the application? This would help us investigate further and see if we missed some use cases.

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

Successfully merging this pull request may close these issues.

Dialog is not updated when doing hotswap
4 participants