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

fix: Navigate with full url #20846

Merged
merged 1 commit into from
Jan 17, 2025
Merged

fix: Navigate with full url #20846

merged 1 commit into from
Jan 17, 2025

Conversation

caalador
Copy link
Contributor

Fix navigating with full url
for instance when adding a
query parameter at the end.

Fixes #19580

Copy link

github-actions bot commented Jan 15, 2025

Test Results

1 165 files  + 2  1 165 suites  +2   1h 33m 28s ⏱️ + 4m 38s
7 623 tests + 2  7 567 ✅ + 2  56 💤 ±0  0 ❌ ±0 
7 992 runs  +16  7 927 ✅ +16  65 💤 ±0  0 ❌ ±0 

Results for commit 1c2d6ed. ± Comparison against base commit eff913f.

♻️ This comment has been updated with latest results.

mcollovati
mcollovati previously approved these changes Jan 17, 2025
Copy link
Collaborator

@mcollovati mcollovati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally and it works fine.

I just wonder if we should raise an error on the client side if the given URL is absolute and does not match document.baseURI.
With Vaadin router, an error is raised, so it would probably be good to have a similar behavior also with React.

image

Fix navigating with full url
for instance when adding a
query parameter at the end.

Fixes #19580
@caalador
Copy link
Contributor Author

I guess we should, but I wonder should it be part of this fix or should we have an enhancement ticket for it.

@mcollovati
Copy link
Collaborator

I guess we should, but I wonder should it be part of this fix or should we have an enhancement ticket for it.

Yeah, it definitely can be done separately. I'll create the ticket.

@caalador caalador merged commit e4f135a into main Jan 17, 2025
26 checks passed
@caalador caalador deleted the issues/19580-queryparam branch January 17, 2025 10:29
vaadin-bot pushed a commit that referenced this pull request Jan 17, 2025
Fix navigating with full url
for instance when adding a
query parameter at the end.

Fixes #19580
vaadin-bot added a commit that referenced this pull request Jan 17, 2025
Fix navigating with full url
for instance when adding a
query parameter at the end.

Fixes #19580

Co-authored-by: caalador <[email protected]>
@vaadin-bot
Copy link
Collaborator

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

Strange behavior with History -> replaceState(state,location) when react is enabled
3 participants