fix (design library): preview fixes #3606
Conversation
|
Warning Rate limit exceeded@kaeizen has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 41 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughTracks stylesheet load progress in the shadow root and passes it to the preview renderer; memoizes preview size in list items; tightens scale recompute logic with width/height guards; prefers requestIdleCallback for deferred previews (5000ms timeout in design-preview, per-item timeout in index); re-enables spinner z-index and background rules. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Item as DesignLibraryListItem
participant SR as useShadowRoot
participant PR as usePreviewRenderer
participant DOM as Shadow DOM
rect rgba(235,245,255,0.6)
Note over Item: Mount
Item->>SR: create shadow root, inject styles
SR-->>Item: { hostRef, shadowRoot, stylesLoaded }
Item->>PR: init renderer(shadowRoot, designPreviewSize, stylesLoaded)
end
rect rgba(240,255,240,0.6)
Note over SR,DOM: Style injection & load tracking
SR->>DOM: append style/link nodes (skip duplicates)
DOM-->>SR: onload/onerror → increment stylesLoaded
SR-->>Item: stylesLoaded updates
Item->>PR: stylesLoaded changed
PR->>PR: adjustScale(force, callCount) — check isEqualWidth/isEqualHeight
alt recompute == true
PR->>DOM: apply scale updates
else
PR-->>PR: skip DOM update
end
end
sequenceDiagram
autonumber
participant DP as DesignPreview
participant IDX as IndexScheduler
participant Browser as Window
Note over DP,IDX: Schedule deferred preview work
DP->>Browser: requestIdleCallback(callback, { timeout: 5000 }) or fallback setTimeout(designIndex * 20)
IDX->>Browser: requestIdleCallback(callback, { timeout: (designIndex + 1) * 500 }) or fallback setTimeout(...)
Browser-->>DP: idle callback / timeout fired
Browser-->>IDX: idle callback / timeout fired
DP-->>DP: run deferred render
IDX-->>IDX: mark shouldRender true for item
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🤖 Pull request artifacts
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/design-library-list/design-preview.js (1)
96-113: Consider simplifying the nested scheduling.The code nests
requestAnimationFrameandrequestIdleCallback, which adds complexity. SincerequestIdleCallbackalready schedules work during idle time, the additionalrequestAnimationFramewrapper may be unnecessary unless you specifically need to coordinate with the browser's paint cycle.Consider this simpler approach:
-if ( selectedTab !== 'pages' || designIndex < 9 ) { - // insert HTML for patterns and for the first 9 pages - wrapper.innerHTML = sanitizedHTML - requestAnimationFrame( () => { - ric( () => setIsLoading( false ) ) - } ) - return -} - -requestAnimationFrame( () => { - ric( () => { - wrapper.innerHTML = sanitizedHTML - updateShadowBodySize() - requestAnimationFrame( () => { - ric( () => setIsLoading( false ) ) - } ) - } ) -} ) +if ( selectedTab !== 'pages' || designIndex < 9 ) { + wrapper.innerHTML = sanitizedHTML + ric( () => setIsLoading( false ) ) + return +} + +ric( () => { + wrapper.innerHTML = sanitizedHTML + updateShadowBodySize() + ric( () => setIsLoading( false ) ) +} )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/design-library-list/design-library-list-item.js(6 hunks)src/components/design-library-list/design-preview.js(1 hunks)src/components/design-library-list/editor.scss(1 hunks)src/components/design-library-list/use-preview-renderer.js(6 hunks)src/components/design-library-list/use-shadow-root.js(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-25T08:17:15.543Z
Learnt from: kaeizen
PR: gambitph/Stackable#3601
File: src/components/design-library-list/use-preview-renderer.js:307-331
Timestamp: 2025-09-25T08:17:15.543Z
Learning: In the design library preview renderer hook (src/components/design-library-list/use-preview-renderer.js), selectedTab changes trigger content updates through a chain of effects rather than direct dependencies. The first useEffect (template processing) responds to selectedTab changes indirectly and updates content state, which then triggers the second useEffect (rendering/scaling) that depends on content. This eliminates the need to add selectedTab to the second effect's dependency array.
Applied to files:
src/components/design-library-list/use-preview-renderer.jssrc/components/design-library-list/design-library-list-item.js
🧬 Code graph analysis (3)
src/components/design-library-list/use-shadow-root.js (2)
src/components/design-library-list/util.js (2)
getAdditionalStylesForPreview(338-391)getAdditionalStylesForPreview(338-391)src/components/design-library-list/index.js (1)
shouldRender(77-77)
src/components/design-library-list/design-preview.js (1)
src/higher-order/with-block-wrapper-is-hovered/util.js (1)
window(8-11)
src/components/design-library-list/use-preview-renderer.js (3)
src/components/design-library-list/design-preview.js (1)
ref(21-21)src/components/design-library-list/use-shadow-root.js (3)
hostRef(11-11)shadowRoot(12-12)stylesLoaded(13-13)src/components/design-library-list/index.js (2)
selectedTab(79-87)shouldRender(77-77)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: build
🔇 Additional comments (6)
src/components/design-library-list/editor.scss (1)
249-254: Spinner layering fix looks correctRe-enabling the white backdrop with
z-index: 1gives the loading overlay proper coverage again, and dropping it to-1in the hidden state ensures interactions resume immediately after fade-out. Nice tidy alignment with the shadow-root style loading flow.src/components/design-library-list/design-library-list-item.js (2)
23-23: LGTM! Clean integration of stylesLoaded.The addition of
stylesLoadedfromuseShadowRootand its propagation tousePreviewRendererproperly coordinates style readiness with the rendering pipeline.Also applies to: 47-49, 59-59
65-76: LGTM! Memoization prevents unnecessary recalculations.The memoized
designPreviewSizeavoids redundant height calculations and the dependency array correctly captures all values that affect the computation.src/components/design-library-list/use-shadow-root.js (1)
31-34: LGTM! Duplicate prevention and load tracking well implemented.The
existingIdsSet prevents duplicate style injection, and the conditional guards ensure styles are only added once. The load/error handlers correctly track external stylesheets.Also applies to: 58-72, 74-97
src/components/design-library-list/use-preview-renderer.js (2)
87-148: LGTM! Recompute guard prevents redundant scale updates.The enhanced
adjustScalelogic withisEqualWidthandisEqualHeightchecks efficiently skips unnecessary scaling when dimensions are already aligned. Theforceparameter provides a retry mechanism, andcallCountprevents infinite loops.
349-359: LGTM! Style-load effect triggers necessary reflow.The new effect correctly triggers scale adjustment when styles finish loading, ensuring previews render at the correct size once all styles are available. The guards prevent execution when blocks aren't ready.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/design-library-list/design-preview.js (1)
93-94: Verify the scheduler fix addresses the previous concern.The ternary operator correctly fixes the logical OR issue from the previous review. Both branches now return functions accepting a callback, and the
requestIdleCallbacktimeout option is properly passed.However, consider explicitly using
window.setTimeoutfor consistency with howwindow.requestIdleCallbackis referenced, and to match patterns seen elsewhere in the codebase (e.g.,src/higher-order/with-block-wrapper-is-hovered/util.js).Optional refactor for explicitness:
-const ric = window.requestIdleCallback ? ( cb => window.requestIdleCallback( cb, { timeout: 1500 } ) ) - : ( cb => setTimeout( cb, designIndex * 20 ) ) +const ric = window.requestIdleCallback + ? ( cb => window.requestIdleCallback( cb, { timeout: 1500 } ) ) + : ( cb => window.setTimeout( cb, designIndex * 20 ) )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/design-library-list/design-preview.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/design-library-list/design-preview.js (1)
src/higher-order/with-block-wrapper-is-hovered/util.js (1)
window(8-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP latest
🔇 Additional comments (1)
src/components/design-library-list/design-preview.js (1)
84-115: Unable to verify loading–stylesLoaded coordination automatically
I couldn’t locate howsetIsLoadingindesign-preview.jsinteracts with thestylesLoadedcounter in your preview/render hooks. Please manually confirm that both the immediate‐insertion path (lines 97–104) and the deferred‐insertion path (lines 106–114) don’t race with style loading—i.e.,setIsLoading(false)only fires after all required CSS has been appended to the shadow root.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/design-library-list/index.js (1)
132-132: Simplify the idle callback to avoid unnecessary object allocation.The conditional returns an empty object
{}in the else branch. While functional, this is non-idiomatic and allocates an unnecessary object. Consider using a statement block or removing the else clause entirely.Apply this diff for a cleaner implementation:
- id = requestIdleCallback( () => ! shouldRender ? setShouldRender( true ) : {}, { timeout: 3000 } ) + id = requestIdleCallback( () => { + if ( ! shouldRender ) { + setShouldRender( true ) + } + }, { timeout: 3000 } )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/design-library-list/design-preview.js(1 hunks)src/components/design-library-list/index.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/design-library-list/design-preview.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 7.3 and WP latest
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/design-library-list/index.js (1)
126-145: Consider guarding the effect when shouldRender is already true.When
selectedTabswitches back to 'pages', this effect schedules an idle callback even ifshouldRenderis alreadytrue. While the callback itself checks! shouldRender, scheduling unnecessary idle callbacks can still occur.Consider adding an early return:
useEffect( () => { if ( selectedTab !== 'pages' ) { return } + if ( shouldRender ) { + return + } let id if ( typeof requestIdleCallback !== 'undefined' ) { - id = requestIdleCallback( () => ! shouldRender ? setShouldRender( true ) : {}, { timeout: ( designIndex + 1 ) * 500 } ) + id = requestIdleCallback( () => setShouldRender( true ), { timeout: ( designIndex + 1 ) * 500 } ) } else { // fallback - id = setTimeout( () => setShouldRender( true ), designIndex * 20 ) + id = setTimeout( () => setShouldRender( true ), ( designIndex + 1 ) * 500 ) } return () => { if ( typeof cancelIdleCallback !== 'undefined' ) { cancelIdleCallback( id ) } else { clearTimeout( id ) } } - }, [ selectedTab ] ) + }, [ selectedTab, shouldRender, designIndex ] )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/design-library-list/design-preview.js(1 hunks)src/components/design-library-list/index.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/design-library-list/design-preview.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: build
2 participants
Summary by CodeRabbit
Performance
Bug Fixes
Style