search

hotwired / turbo

The speed of a single-page web application without having to write any JavaScript
https://turbo.hotwired.dev
MIT License
6.55k stars 416 forks source link

scrollTo()/scrollIntoView() doesn't work after visit #354

Open VividVisions opened 2 years ago

VividVisions commented 2 years ago

I need to scroll to a certain element on a page right after it loads. I use a Stimulus controller which listens to the turbo:load event, then searches for the element and, if found, scrolls it into view.

This works when I call the page for the first time or refresh the browser. But whenever I reach the page through Turbo.visit(), it simply does not scroll although the controller function gets called.

(I've tried scrollTo(), scrollIntoView() and also location.href = '#anchorId';. The controller function finds the correct element and gets its correct position on the page.)

It seems that Turbo's scroll position restoration functions are interfering. Is there a way to prevent those? Or is there an event that fires after?

(Also happens with v7.0.0-rc.2.)

MichalSznajder commented 2 years ago

I have the same problem. I think this boils down to turbo:load being fired before scroll restore logic kicks in.

Related code: 'Visit.loadResponse()' calls 'Visit.render()'. In promise for Visit.render() Visit.complete() is called that dispatches turbo:load. Later Visit.render() calls 'performScroll()' that modifies scroll position so your Stimulus controller is no longer working.

Maybe turbo:load could be dispatched as last event in Turbo Drive?

Also #312 is related.

bep commented 2 years ago

I think @michalsznajder is right, but I'm not sure making turbo:load the last event would solve this problem.

You could get to something similar today by doing something like this in turbo:render:

requestAnimationFrame(() => { window.scrollTo(0, 123); })

The problem with that is that Turbo has already scrolled to the top so you get this flicker effect.

Ideally it would be cool to have a way to signal Turbo to not scroll, but I'm not sure how that would look like.

seanpdoyle commented 2 years ago

Do the change proposed in https://github.com/hotwired/turbo/pull/476 resolve this issue?

This issue might be related to https://github.com/hotwired/turbo/issues/400#issuecomment-979464687.

bep commented 2 years ago

@seanpdoyle I don't think so (but it's hard to be bombastic about it without testing it); I suspect that it will work on the functional level, but that it will cause a flicker effect.

seanpdoyle commented 2 years ago

I've added a test to https://github.com/hotwired/turbo/pull/476 to try and re-create the scenario outlined by this issue's initial comment: https://github.com/hotwired/turbo/pull/476/files#diff-b7c9a06f6b41512fee24c53728cacc1b98c47e90ac291d047e576a686ee86c60R123-R145

@bep I agree with your sentiment: adding coverage to guard against a regression will be valuable, but it might be challenging to programmattcally detect flicker from within the suite.

MichalSznajder commented 1 month ago

I think this issue can be closed.

On 7.3.0 I can perform scrollTo on turbo:load without any problems.