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.

Sign up for GitHub

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

Jump to bottom

Navigating to the current route but with different query params does not update the URL when using @PreserveOnRefresh #19701

Closed
jameskerrtaskize opened this issue Jul 17, 2024 · 6 comments · Fixed by #19718
Closed

Navigating to the current route but with different query params does not update the URL when using @PreserveOnRefresh #19701

jameskerrtaskize opened this issue Jul 17, 2024 · 6 comments · Fixed by #19718
Assignees
@mcollovati
Labels
BFP Bugfix priority, also known as Warranty bug Impact: Low navigation prerelease version for Vaadin 24.5.0 preserve-on-refresh Released with Vaadin 24.4.8 Released with Vaadin 24.5.0.alpha6 Severity: Minor workaround

Comments

@jameskerrtaskize
Copy link

Description of the bug

In my repro example there is a route called TreeView ("/tree"). It contains 3 buttons. If you click the first button it navigated to /tree?param=one. If you click the second, it navigates to /tree?param=two etc.

My demo app has @PreserveOnRefresh on the TreeView route. When clicking the buttons, the URL does not update.
If you remove @PreserveOnRefresh, the buttons do cause the URL to update.

The actual navigation is fine but the URL does not change correctly.

We had previously worked around this by calling ui.getPage().getHistory().replaceState(..) to set the correct URL. However, we are now unable to upgrade to 24.4 due to #19613
I don't really want to be calling replaceState so I'd like to get this bug fixed so we don't have to.

Expected behavior

URL should change when the buttons are pressed

Minimal reproducible example

preserveonrefresh-nav-issue.zip

Versions

  • Vaadin / Flow version: 24.4 (was present in 23)
  • Java version: 17
  • OS version: osx
@mcollovati
Copy link
Collaborator

It looks like a regression introduced by #15331
For programmatic navigation, Push history is refreshed only if the trigger is UI_NAVIGATE. The mentioned PR added a new trigger REFRESH for preserve on refresh, to allow distinguishing navigation events.

If you replace the buttons with RouterLinks you get the expected behavior.

@mcollovati mcollovati moved this from 🆕 Needs triage to 🔖 Normal Priority (P2) in Vaadin Flow bugs & maintenance (Vaadin 10+) Jul 19, 2024
@mcollovati mcollovati added workaround BFP Bugfix priority, also known as Warranty labels Jul 19, 2024
@mcollovati mcollovati moved this from 🔖 Normal Priority (P2) to 🔖 High Priority (P1) in Vaadin Flow bugs & maintenance (Vaadin 10+) Jul 19, 2024
@mcollovati
Copy link
Collaborator

The issue was triaged and currently added to the backlog priority queue for further investigation

@mcollovati mcollovati self-assigned this Jul 24, 2024
@mcollovati
Copy link
Collaborator

The issue was assigned to a developer and is currently being investigated

@mcollovati mcollovati moved this from 🔖 High Priority (P1) to 🏗 WIP in Vaadin Flow bugs & maintenance (Vaadin 10+) Jul 24, 2024
mcollovati added a commit that referenced this issue Jul 24, 2024
@mcollovati
fix: update browser URL on preserve on refresh navigation
408a853 
Programmatic self navigation with query parameters on view with preserve on
refresh do not update the URL in the browser.
This change fixes the regression introduced by #15331, updating push
history also for navigation events with trigger REFRESH.

Fixes #19701
@mcollovati mcollovati mentioned this issue Jul 24, 2024
Merged
9 tasks
@mcollovati mcollovati moved this to 🔎Iteration reviews in Vaadin Flow ongoing work (Vaadin 10+) Jul 24, 2024
mcollovati added a commit that referenced this issue Jul 24, 2024
@mcollovati
fix: update browser URL on preserve on refresh navigation (#19718)
763a530 
Programmatic self navigation with query parameters on view with preserve on
refresh do not update the URL in the browser.
This change fixes the regression introduced by #15331, updating push
history also for navigation events with trigger REFRESH.

Fixes #19701
vaadin-bot pushed a commit that referenced this issue Jul 24, 2024
@mcollovati @vaadin-bot
fix: update browser URL on preserve on refresh navigation (#19718)
07f2599 
Programmatic self navigation with query parameters on view with preserve on
refresh do not update the URL in the browser.
This change fixes the regression introduced by #15331, updating push
history also for navigation events with trigger REFRESH.

Fixes #19701
vaadin-bot pushed a commit that referenced this issue Jul 24, 2024
@mcollovati @vaadin-bot
fix: update browser URL on preserve on refresh navigation (#19718)
d9b5973 
Programmatic self navigation with query parameters on view with preserve on
refresh do not update the URL in the browser.
This change fixes the regression introduced by #15331, updating push
history also for navigation events with trigger REFRESH.

Fixes #19701
vaadin-bot added a commit that referenced this issue Jul 24, 2024
@vaadin-bot @mcollovati
fix: update browser URL on preserve on refresh navigation (#19718) (C…
5896dda 
…P: 24.4) (#19719)

* fix: update browser URL on preserve on refresh navigation (#19718)

Programmatic self navigation with query parameters on view with preserve on
refresh do not update the URL in the browser.
This change fixes the regression introduced by #15331, updating push
history also for navigation events with trigger REFRESH.

Fixes #19701

* restore removed imports

---------

Co-authored-by: Marco Collovati <[email protected]>
vaadin-bot added a commit that referenced this issue Jul 24, 2024
@vaadin-bot @mcollovati
fix: update browser URL on preserve on refresh navigation (#19718) (C…
94b67dd 
…P: 24.3) (#19720)

* fix: update browser URL on preserve on refresh navigation (#19718)

Programmatic self navigation with query parameters on view with preserve on
refresh do not update the URL in the browser.
This change fixes the regression introduced by #15331, updating push
history also for navigation events with trigger REFRESH.

Fixes #19701

* restore removed imports

---------

Co-authored-by: Marco Collovati <[email protected]>
@mcollovati
Copy link
Collaborator

The PR fixing the issue was merged and changes will be delivered in the following versions: Flow 24.4.5 (Vaadin 24.4.8), 24.3.14 (Vaadin 24.3.17)

@vaadin-bot
Copy link
Collaborator

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

@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.4.8.

@jameskerrtaskize jameskerrtaskize mentioned this issue Aug 21, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mcollovati mcollovati

Labels
BFP Bugfix priority, also known as Warranty bug Impact: Low navigation prerelease version for Vaadin 24.5.0 preserve-on-refresh Released with Vaadin 24.4.8 Released with Vaadin 24.5.0.alpha6 Severity: Minor workaround
Projects
Vaadin Flow bugs & maintenance (Vaadin 10+)
Status: ✅ Closed
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: update browser URL on preserve on refresh navigation vaadin/flow
3 participants
@mcollovati @vaadin-bot @jameskerrtaskize