Improve handling of anchor links and hashchange events #1125
Conversation
There was a problem hiding this comment.
The whole navigate-to-fragment stuff is pretty complex. Not only Turbo's code, but also browser behaviours. I'm pretty sure there's good reason for the conditionals here as it was well tested at the time, however it still throws up bugs in some situations.
So I think it may result in whack-a-mole style development.
In terms of the behaviour, I think our best option is to do something like: #592 (comment) where we create an <a> with the hash and programatically click it. That way we get scrolling, element focussing, and :target styling.
We'll still need to spec out the different cases when this is needed (e.g. a clicked link vs a programatic visit, vs a restoration visit), but hopefully this might tidy thing up a bit?
|
Interesting! I've never used the The above examples are relevant also to just clicking simple anchor links on the same page (without doing the navigate-to-fragment across separate pages). It's a bit sticky due to the way turbo/src/core/drive/navigator.js Lines 130 to 140 in 2a638e0 That value ( So using it here to determine " It's also the underlying cause of this issue: #876 (comment) (and related weird behaviours). |
|
Minimal repro/testing page for anchor links and hashchange events: <!DOCTYPE html>
<html>
<head>
<script src="https://cdn.jsdelivr.net/npm/@hotwired/[email protected]/dist/turbo.es2017-umd.min.js"></script>
<script type="text/javascript">
window.addEventListener("hashchange", (event) => {
console.log("hashchange: ", event)
document.getElementById("events").insertAdjacentHTML("beforeend",
`<p>Hashchange: { oldURL: ${event.oldURL}, newURL: ${event.newURL} }</p>`
)
});
window.addEventListener("turbo:before-fetch-request", (event) => {
console.log("turbo:before-fetch-request: ", event)
document.getElementById("events").insertAdjacentHTML("beforeend",
`<p>Fetch request: ${event.detail.url} (cancelled for demonstration)</p>`
)
event.preventDefault()
history.replaceState({}, "", event.detail.url)
});
function clearEvents() { document.getElementById("events").innerHTML = "" }
</script>
<meta content="no-cache" name="turbo-cache-control" />
</head>
<body>
<h1>Same-page anchors</h1>
<ul>
<li><a href="">No anchor</a></li>
<li><a href="#one">Anchor #one</a></li>
<li><a href="#two">Anchor #two</a></li>
<li><a href="#three">Anchor #three</a></li>
</ul>
<h2>Event log:</h2>
<div id="events">
</div>
<button onclick="clearEvents()">Clear events</button>
</body>
</html> |
Matt-Yorkley
changed the title
It can be used for just about anything you can do with CSS, including layout stuff. For example, it's a crucial part of our mobile menu on Omnipedia, which actually works if our JavaScript fails to execute because the link to open it just points to Anyways, I'm actually working on integrating Turbo into our site and ran across some of these issues with 7.3.0, notably the double Edit: Just had a chance to try this out in 8.0.0-beta.3 and it does seem to fix the issue we had with an unexpected page visit when just the hash changed. Good work! |
When navigating around between multiple anchor links on the same page there's a number of weird behaviours that can come up with how the navigations are handled and the subsequent
hashchangeevents that get fired:The
hashchangeevent's value forevent.oldURLoften contains an incorrect URL (wrong hash)Using the back or forwards buttons (restoration visits) causes two
hashchangeevents to be fired each time instead of one, and the duplicate contains incorrect informationMoving between multiple anchors on the same page can trigger an unexpected page visit in some cases, including a network request and repainting the DOM (
turbo:load) when it shouldn'tRelated issues/discussions here: #539 and here: #876 (comment)