Skip to content

Fix window repaint on JS scroll events - move to CSS (#63) - #184

Open
wakqasahmed wants to merge 1 commit into
sunnah-com:masterfrom
wakqasahmed:fix/issue-63-scroll-repaint-to-css
Open

Fix window repaint on JS scroll events - move to CSS (#63)#184
wakqasahmed wants to merge 1 commit into
sunnah-com:masterfrom
wakqasahmed:fix/issue-63-scroll-repaint-to-css

Conversation

@wakqasahmed

Copy link
Copy Markdown

Fixes #63

Problem

The sticky/compact-header effect (shrinking the banner, hiding the toolbar, fixing the header to the top, and repositioning the side panel) was driven by a $(window).scroll() jQuery handler that ran on every scroll event. On each tick it:

  • wrote several inline styles (position, top, display, bottom) unconditionally, even when the state hadn't changed, and
  • called $(".mainContainer").position(), which forces a synchronous layout read.

That combination produces a forced synchronous layout + full repaint on every scroll pixel, which is exactly what the attached Chrome DevTools Performance profile in the issue shows: a Long Task dominated by Painting/Rendering time while scrolling sunnah.com/riyadussalihin/18.

Verified the same $(window).scroll(...) handler (public/js/sunnah.js, previously lines 462-485) is still present, unchanged, in current master.

Fix

  • Replaced the scroll listener with an IntersectionObserver on #toolbar, which only fires when the header actually crosses the same ~25px threshold the old code polled for on every tick — not on every scroll event.
  • Moved the visual states (fixed header, hidden toolbar, shrunk banner, repositioned search box/side panel) into a single .header-stuck CSS class (public/css/all.css) instead of ad-hoc inline style writes.
  • The side panel's dynamic left offset (previously recomputed via a layout-forcing .position() call on every scroll event) is now computed once per state transition and on resize, via a CSS custom property (--sidePanel-left).
  • Kept the back-to-top button toggle but guarded it so it only touches the class when the 750px threshold is actually crossed, instead of every scroll tick.

No visual behavior change intended — same thresholds, same classes/styles, same effect — only removed the per-scroll-tick DOM writes and forced layout reads.

Test plan

  • node --check public/js/sunnah.js passes (syntax valid)
  • Manual/visual verification on a running instance (no local dev environment available in this sandbox — please verify header/side-panel/back-to-top behavior on a hadith page like /riyadussalihin/18 before merging)

…-com#63)

The sticky/compact-header effect was implemented as a $(window).scroll()
handler that wrote several inline styles (position, top, display, bottom)
and read $(".mainContainer").position() on every single scroll event,
forcing a synchronous layout and repaint of the page on every scroll tick
(visible as a Long Task dominated by Painting/Rendering in the DevTools
Performance panel).

Replace it with an IntersectionObserver watching #toolbar, which only
fires on the actual threshold crossing, and drive the visual states with
a single .header-stuck CSS class instead of per-tick inline style writes.
The sidePanel's dynamic left offset is now computed once per state
transition (and on resize) via a CSS custom property rather than on every
scroll event.
@wakqasahmed

Copy link
Copy Markdown
Author

Hi @ahadith @Yugi-2 — noticed this PR doesn't have a reviewer assigned yet — it's been about 4 days, CI is green and it's mergeable. Would you (or whoever's best placed) be able to take a look when you get a chance, or point me to who should? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix issue with window repaint on JS scroll events - move to CSS

1 participant