-
Notifications
You must be signed in to change notification settings - Fork 0
⚡ bolt: [performance improvement] #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,6 @@ | ||
| ## 2024-05-16 - [Frontend Performance: Caching getBoundingClientRect in Animation Loops] | ||
| **Learning:** Frequent calls to `getBoundingClientRect()` inside `requestAnimationFrame` loops or scroll handlers (like in `MagneticLetters`, `MobileTouchRepel`, and `ParallaxLayers`) cause significant layout thrashing and forced synchronous layouts, degrading rendering performance. | ||
| **Action:** Always pre-calculate and cache document-relative element positions during initialization (`init()`). Update this cache on window resize events and after custom fonts load (using `document.fonts.ready.then()`). During active animations (`animate()`, `update()`, etc.), calculate viewport-relative positions using the cached document coordinates minus the current scroll offset (`window.scrollX` / `window.scrollY`) instead of directly querying the DOM. | ||
| **Action:** Always pre-calculate and cache document-relative element positions during initialization (`init()`). Update this cache on window resize events and after custom fonts load (using `document.fonts.ready.then()`). During active animations (`animate()`, `update()`, etc.), calculate viewport-relative positions using the cached document coordinates minus the current scroll offset (`window.scrollX` / `window.scrollY`) instead of directly querying the DOM. | ||
| ## 2024-05-16 - [Performance Optimization: Use IntersectionObserver to Replace Synchronous getBoundingClientRect] | ||
| **Learning:** Checking element visibility synchronously via `getBoundingClientRect` (like in `PhotoCarousel.isInView()`) inside high-frequency event handlers (e.g. scroll, mousewheel, keydown) causes forced synchronous layout thrashing. | ||
| **Action:** Always prefer caching the visibility state asynchronously using `IntersectionObserver` when you only need to know if an element is in view, and read that cached boolean instead of calling `getBoundingClientRect`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -864,6 +864,8 @@ const PhotoCarousel = { | |
| currentIndex: 2, // Start with center card (index 2) | ||
| positions: ['far-left', 'left', 'center', 'right', 'far-right'], | ||
| isAnimating: false, | ||
| isVisible: false, // ⚡ bolt: cache visibility state to prevent layout thrashing | ||
| observer: null, | ||
|
|
||
| init() { | ||
| this.carousel = document.getElementById('photo-carousel'); | ||
|
|
@@ -916,6 +918,14 @@ const PhotoCarousel = { | |
| } | ||
| }, { passive: false }); | ||
|
|
||
| // ⚡ bolt: use intersectionobserver instead of getboundingclientrect | ||
| this.observer = new IntersectionObserver((entries) => { | ||
| entries.forEach(entry => { | ||
| this.isVisible = entry.isIntersecting; | ||
| }); | ||
| }, { threshold: 0.1 }); | ||
| this.observer.observe(this.carousel); | ||
|
|
||
| // Get the stack element early for all event handlers | ||
| const stack = this.carousel.querySelector('.carousel-stack'); | ||
|
|
||
|
|
@@ -1028,9 +1038,8 @@ const PhotoCarousel = { | |
| }, | ||
|
|
||
| isInView() { | ||
| if (!this.carousel) return false; | ||
| const rect = this.carousel.getBoundingClientRect(); | ||
| return rect.top < window.innerHeight && rect.bottom > 0; | ||
| // ⚡ bolt: return cached state instead of synchronous layout query | ||
| return this.isVisible; | ||
|
Comment on lines
1040
to
+1042
|
||
| }, | ||
|
|
||
| navigate(direction) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The previous
isInView()logic treated the carousel as “in view” when any part intersected the viewport (rect.top < innerHeight && rect.bottom > 0). UsingIntersectionObserverwiththreshold: 0.1changes that behavior (requires ~10% visibility), which can alter keyboard/wheel navigation activation near the viewport edges. To preserve existing behavior, use a threshold that matches the old condition (typically0, optionally with an appropriaterootMargin).