impr(nav): avoid duplicate browser history entries when re-clicking same nav button (@byseif21) #6624
Conversation
Miodec
added
the
waiting for review
| // only push to history if we're navigating to a different URL | ||
| const currentPath = | ||
| window.location.pathname + window.location.search + window.location.hash; | ||
| const targetUrl = new URL(url, window.location.origin); | ||
| const targetPath = targetUrl.pathname + targetUrl.search + targetUrl.hash; | ||
|
|
||
| if (currentPath !== targetPath) { | ||
| history.pushState(null, "", url); | ||
| } |
There was a problem hiding this comment.
Curious if there’s a reason we’re not directly comparing window.location.href with url — since the click handler calls this navigate function with target.href, it might work out cleanly and will simplify this. What do you think?
There was a problem hiding this comment.
Hi @pran01, I thought about using window.location.href !== url, but I went with this approach because url might be relative (like /settings), while href is always absolute. That mismatch could cause false negatives.
By comparing just the path, search, and hash, I make sure we only push when the actual navigable part changes—regardless of environment or base URL. It's a bit more verbose, but safer for typical SPA routing. that what I thought of.
There was a problem hiding this comment.
but rethinking this about simplifing thing, i think we could simplify this to be like that
const currentUrl = new URL(window.location.href);
const targetUrl = new URL(url, window.location.origin);
if (
currentUrl.pathname + currentUrl.search + currentUrl.hash !==
targetUrl.pathname + targetUrl.search + targetUrl.hash
) {
history.pushState(null, "", url);
}
Same behavior, just a little cleaner. I will wait for mio tho.
There was a problem hiding this comment.
but rethinking this about simplifing thing, i think we could simplify this to be like that
const currentUrl = new URL(window.location.href); const targetUrl = new URL(url, window.location.origin); if ( currentUrl.pathname + currentUrl.search + currentUrl.hash !== targetUrl.pathname + targetUrl.search + targetUrl.hash ) { history.pushState(null, "", url); }Same behavior, just a little cleaner. I will wait for mio tho.
Yeah i would prefer that.
Miodec
added
waiting for update
github-actions
bot
removed
the
waiting for update
Description
Closes #6623