qmd-lab / closeread

https://closeread.dev
MIT License
128 stars 5 forks source link

Changing browser zoom breaks Scrollama progress updates #101

Closed jimjam-slam closed 3 weeks ago

jimjam-slam commented 1 month ago

On https://closeread.netlify.app/gallery/demos/ojs-variables, scroll part way through the OJS animation, then change browser zoom and continue to scroll. The Scrollama progress variables no longer update.

Perhaps Scrollama needs to be rebooted or some such when browser zoom changes?

Observed on Safari and Chrome (and I've just realised that the OJS demo fails entirely on Chrome! 🪦)

andrewpbray commented 1 month ago

Oy! Indeed, on Chrome:

Screenshot 2024-10-19 at 5 48 41 PM

Do you think this is a browser update that broke this? I swear this one was working fine on chrome a week ago..

jimjam-slam commented 1 month ago

Interesting! I wonder if the Quarto team are getting any reports... should be on in just over an hour to get into it!

jimjam-slam commented 1 month ago

I've split the GeoJSON error out to a separate issue (but it looks like it's no longer a problem on the preview docs!).

For the animation, I can confirm that the step progress and step enter events don't appear to be firing properly. See attached video:

https://github.com/user-attachments/assets/2de75b11-55a7-4e80-b28d-34b7da34f577

I might see if I can refactor our code very slightly so that the Scrollama event listeners are "rebooted" when the screen zoom changes!

jimjam-slam commented 1 month ago

I think this might be https://github.com/russellsamora/scrollama/issues/145. I've attempted a small change based on a comment in that thread:

# after the scroller setup code

window.addEventListener("resize", (event) => {
    console.log(">>> WINDOW RESIZE")
    setTimeout(1000, triggerScroller.resize)
    setTimeout(1000, progressBlockScroller.resize)
  })

I used setTimeout instead of debounce from the throttleDebounce library on the assumption that it's purely a performance consideration to not want to call it many times. But I'm not seeing any success (the resize event fires, but scroller progress event handlers are not improving).

Not sure if there's a race condition, if this just isn't the problem or something in the middle.

EDIT: nevermind, I had the code wrong using setTimeout! This works!

window.addEventListener("resize", (event) => {
    setTimeout(() => triggerScroller.resize(), 1000)
    setTimeout(() => progressBlockScroller.resize(), 1000)
  })
jimjam-slam commented 1 month ago

Not sure if it's necessary to bring in throttle-debounce and use that — performance doesn't seem to be massively affected when manually resizing the window, and a browser zoom only seems to cause one resize event. (That said, I'd like to get more au faix with ES Modules so we can start leaning on a few more things :) )