fix (design library): improve scrolling #3601
Conversation
WalkthroughReworks list/item rendering: items memoized and no longer ref-forwarded; introduces idle- and visibility-gated shouldRender propagated to useShadowRoot and usePreviewRenderer; preview rendering batched/staged with updateShadowBodySize and rAF/idle DOM writes; spinner and modal/editor styles adjusted; pages prefetch added. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Parent as DesignLibraryList
participant ItemWrap as Item wrapper
participant Item as DesignLibraryListItem
participant SR as useShadowRoot
participant PR as usePreviewRenderer
participant DP as DesignPreview
Parent->>ItemWrap: render item (shouldRender=false)
ItemWrap->>ItemWrap: schedule setShouldRender(true) (requestIdleCallback) / observe visibility (IntersectionObserver)
alt shouldRender becomes true
ItemWrap->>Item: mount with shouldRender, designIndex, presetMarks
Item->>SR: useShadowRoot(shouldRender)
SR-->>Item: { hostRef, shadowRoot }
Item->>PR: usePreviewRenderer(props, shouldRender, spacingSize, ref, hostRef, shadowRoot, setIsLoading)
PR-->>Item: { blocks, previewSize, updateShadowBodySize, onClickDesign, ... }
Item->>DP: mount DesignPreview (passes updateShadowBodySize, setIsLoading)
DP->>DP: write innerHTML into wrapperRef (direct or staged via rAF/idle)
DP->>PR: updateShadowBodySize()
DP->>Item: setIsLoading(false)
else shouldRender false
Item-->>Parent: render lightweight placeholder (spinner hidden)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/design-library-list/use-shadow-root.js (1)
25-75: Prevent duplicate style injection when shouldRender toggles.Every time the item re-enters the viewport, this effect runs and appends the same styles again into the shadow root, causing unbounded duplication and performance/memory issues.
Apply a guard to run only once per host (or dedupe by IDs):
- useEffect( () => { - if ( shouldRender && hostRef.current ) { + useEffect( () => { + if ( ! shouldRender || ! hostRef.current || shadowRoot ) { + return + } const shadow = hostRef.current.shadowRoot || hostRef.current.attachShadow( { mode: 'open' } ) @@ - styleNodes.forEach( node => { + styleNodes.forEach( node => { if ( node.textContent ) { // we use :host in the shadow DOM to target the root node.textContent = node.textContent.replace( /:root/g, ':host' ) } - shadow.appendChild( node ) + // Avoid duplicates by ID + if ( node.id && shadow.getElementById( node.id ) ) { + return + } + shadow.appendChild( node ) } ) @@ - setShadowRoot( shadow ) - } - }, [ shouldRender ] ) + setShadowRoot( shadow ) + } + }, [ shouldRender, shadowRoot ] )
🧹 Nitpick comments (7)
src/design-library/index.js (1)
66-69: Avoid blocking on prefetch; fire-and-forget and align version.Awaiting the prefetch will unnecessarily delay returning pages. Also, use the same reset/version for consistency.
- // pre-fetch patterns - if ( type === 'pages' ) { - await fetchDesignLibrary() - } + // Prefetch patterns non-blocking (use same reset/version) + if ( type === 'pages' ) { + fetchDesignLibrary( reset, LATEST_API_VERSION, 'patterns' ) + .catch( () => {} ) + }src/components/design-library-list/editor.scss (1)
235-241: Verify spinner height matches JS placeholder height to avoid jump.Item-pages placeholder height defaults to 413px in JS, but spinner container is 343px. Consider aligning to prevent layout jank.
- height: 343px; + height: 413px;If 343px is intentional, update getCardHeight’s fallback for pages accordingly for consistency.
src/components/design-library-list/index.js (1)
97-111: Avoid global querySelector; derive root from the wrapper to reduce coupling.Document-level query can pick the wrong container if multiple modals exist. Prefer closest ancestor or pass the container ref.
- const rootEl = document.querySelector( '.ugb-modal-design-library__designs' ) + const rootEl = wrapperRef.current?.closest( '.ugb-modal-design-library__designs' )Alternatively, thread the parent’s containerRef down as a prop and use it directly.
src/components/design-library-list/design-library-list-item.js (1)
71-73: Minor: use destructured selectedNum for consistency.Nitpick only; no behavior change.
- if ( ! shouldRender && ! props.selectedNum ) { + if ( ! shouldRender && ! selectedNum ) { return <div style={ { height: `${ getCardHeight() }px` } } /> }src/components/design-library-list/use-preview-renderer.js (3)
253-261: Guard against missing design content in pages aggregation.If a fetched design lacks template/content, replacePlaceholders will throw. Coalesce to empty string.
- const designsContentForPreview = designs.map( ( design, i ) => - replacePlaceholders( design.template || design.content, categorySlugs[ i ], false ) + const designsContentForPreview = designs.map( ( design, i ) => { + const dc = design.template || design.content || '' + return replacePlaceholders( dc, categorySlugs[ i ], false ) + } ).join( '\n' ) @@ - ? designs.map( ( design, i ) => - replacePlaceholders( design.template || design.content, categorySlugs[ i ], true ) - ) + ? designs.map( ( design, i ) => { + const dc = design.template || design.content || '' + return replacePlaceholders( dc, categorySlugs[ i ], true ) + } ) : designsContentForPreview
293-305: Handle initialization errors to avoid stuck loading state.Catch and clear loading on failures to prevent a permanent spinner.
- initialize().then( () => { + initialize().then( () => { @@ - renderedTemplate.current = template - } ) + renderedTemplate.current = template + } ).catch( () => { + setIsLoading( false ) + } )
145-147: Nit: variable naming.Use lowerCamelCase for local variables.
- const CardHeightKey = enableBackground ? 'background' : 'noBackground' - newCardHeight[ CardHeightKey ] = cardRect.height + const cardHeightKey = enableBackground ? 'background' : 'noBackground' + newCardHeight[ cardHeightKey ] = cardRect.height
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/components/design-library-list/design-library-list-item.js(4 hunks)src/components/design-library-list/editor.scss(1 hunks)src/components/design-library-list/index.js(5 hunks)src/components/design-library-list/use-preview-renderer.js(9 hunks)src/components/design-library-list/use-shadow-root.js(3 hunks)src/design-library/index.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/components/design-library-list/use-preview-renderer.js (6)
src/components/design-library-list/design-library-list-item.js (3)
props(27-34)spacingSize(38-38)ref(44-44)src/components/design-library-list/index.js (2)
props(22-30)shouldRender(95-95)src/components/design-library-list/design-preview.js (1)
ref(18-18)src/components/design-library-list/use-shadow-root.js (2)
hostRef(11-11)shadowRoot(12-12)src/components/design-library-list/util.js (11)
blocks(235-235)getCategorySlug(325-335)getCategorySlug(325-335)replacePlaceholders(183-193)replacePlaceholders(183-193)cleanParse(15-47)cleanParse(15-47)addBackgroundScheme(106-152)addBackgroundScheme(106-152)adjustPatternSpacing(154-181)adjustPatternSpacing(154-181)src/design-library/index.js (4)
designs(7-7)fetchDesign(41-46)fetchDesign(41-46)design(102-102)
src/components/design-library-list/use-shadow-root.js (2)
src/components/design-library-list/design-library-list-item.js (1)
useShadowRoot(42-42)src/components/design-library-list/index.js (1)
shouldRender(95-95)
src/components/design-library-list/design-library-list-item.js (3)
src/components/design-library-list/index.js (2)
props(22-30)shouldRender(95-95)src/components/design-library-list/use-preview-renderer.js (4)
props(41-53)usePreviewRenderer(37-377)usePreviewRenderer(37-377)cardHeight(59-59)src/components/design-library-list/use-shadow-root.js (4)
useShadowRoot(10-77)useShadowRoot(10-77)hostRef(11-11)shadowRoot(12-12)
src/components/design-library-list/index.js (3)
src/components/modal-design-library/modal.js (1)
selectedTab(62-62)src/components/design-library-list/design-library-list-item.js (2)
DesignLibraryListItem(26-164)props(27-34)src/components/design-library-list/use-preview-renderer.js (1)
props(41-53)
⏰ 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 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP 6.5.5
🔇 Additional comments (3)
src/components/design-library-list/index.js (3)
29-30: Prop added: selectedTab. Ensure all callers pass it.This component now depends on selectedTab for item behavior and sizing.
If any call sites omit it, default to 'patterns' to preserve prior behavior.
65-72: Good: stable keys and prop spreading.Using design.id/designId as key and spreading preview props improves stability and clarity.
119-121: LGTM: child-level render gating via shouldRender.This simplifies the parent and reduces placeholder churn.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/components/design-library-list/use-preview-renderer.js (1)
301-313: Confirm spinner lifecycle is intentional.You only call
setIsLoading(false)here, never setting it back totruefor subsequent loads unless adopting the refactor above. Ensure this matches the intended UX; otherwise, spinners won’t appear for later template fetches.src/components/design-library-list/design-library-list-item.js (1)
114-117: Default scale to 1 to avoid invalid CSS.When
previewSize.scaleis undefined (e.g., very early render),scale(undefined)is invalid. Fallback to 1.- transform: `scale(${ selectedNum && selectedData ? selectedData.selectedPreviewSize.scale : previewSize?.scale })`, + transform: `scale(${ selectedNum && selectedData ? selectedData.selectedPreviewSize.scale : ( previewSize?.scale ?? 1 ) })`,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/design-library-list/design-library-list-item.js(5 hunks)src/components/design-library-list/design-preview.js(2 hunks)src/components/design-library-list/index.js(4 hunks)src/components/design-library-list/use-preview-renderer.js(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/design-library-list/index.js
🧰 Additional context used
🧬 Code graph analysis (3)
src/components/design-library-list/design-preview.js (2)
src/components/design-library-list/use-auto-scroll.js (1)
NOOP(6-6)src/components/design-library-list/use-preview-renderer.js (2)
blocks(55-55)updateShadowBodySize(76-86)
src/components/design-library-list/use-preview-renderer.js (5)
src/components/design-library-list/design-library-list-item.js (3)
props(27-34)spacingSize(38-38)ref(44-44)src/components/design-library-list/index.js (2)
props(22-30)shouldRender(95-95)src/components/design-library-list/use-shadow-root.js (3)
hostRef(11-11)shadowRoot(12-12)useSelect(14-14)src/components/design-library-list/util.js (11)
blocks(235-235)getCategorySlug(325-335)getCategorySlug(325-335)replacePlaceholders(183-193)replacePlaceholders(183-193)cleanParse(15-47)cleanParse(15-47)addBackgroundScheme(106-152)addBackgroundScheme(106-152)adjustPatternSpacing(154-181)adjustPatternSpacing(154-181)src/design-library/index.js (4)
designs(7-7)fetchDesign(41-46)fetchDesign(41-46)design(102-102)
src/components/design-library-list/design-library-list-item.js (3)
src/components/design-library-list/index.js (2)
props(22-30)shouldRender(95-95)src/components/design-library-list/use-preview-renderer.js (5)
props(41-53)usePreviewRenderer(37-390)usePreviewRenderer(37-390)cardHeight(59-59)updateShadowBodySize(76-86)src/components/design-library-list/use-shadow-root.js (4)
useShadowRoot(10-77)useShadowRoot(10-77)hostRef(11-11)shadowRoot(12-12)
⏰ 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). (5)
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 7.3 and WP latest
🔇 Additional comments (5)
src/components/design-library-list/use-preview-renderer.js (2)
314-316: Good fix: keep prevSelectedTabRef in sync.This addresses the earlier bug where the “tab didn’t change” optimization broke.
145-158: Card height initialization looks good.Initial run sets both background/noBackground heights; later runs update the active one only. Matches the intended behavior.
src/components/design-library-list/design-preview.js (1)
54-75: Event listener scoping is correct.Listeners are attached only when relevant (non-pages) and properly cleaned up.
src/components/design-library-list/design-library-list-item.js (2)
67-70: Operator precedence fix in getCardHeight looks good.This resolves the earlier ternary/coalescing bug.
72-74: Efficient placeholder rendering before shouldRender.Lightweight placeholder avoids work while preserving layout.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/components/design-library-list/use-preview-renderer.js (3)
59-61: Initialize state objects with empty objects instead of empty arrays.React compares dependency arrays using shallow comparison, and these state variables are used as objects throughout the hook. The initialization should match their expected structure.
- const [ cardHeight, setCardHeight ] = useState( {} ) - const [ previewSize, setPreviewSize ] = useState( {} ) + const [ cardHeight, setCardHeight ] = useState( {} ) + const [ previewSize, setPreviewSize ] = useState( {} )Actually, the current initialization is correct - these are objects, not arrays.
260-277: Optimize batch fetching and processing of designs.The change from individual processing to batch fetching and processing is a good performance improvement. However, there are potential issues with the current implementation.
#!/bin/bash # Verify if fetchDesign handles concurrent calls properly to avoid race conditions echo "Searching for fetchDesign implementation to check caching behavior..." fd -t f -e js fetchDesign | head -5 | xargs rg -A 10 -B 2 "fetchDesign.*async" echo -e "\nChecking if there's any concurrency handling in the design fetching..." rg -A 5 -B 5 "Promise\.all.*fetchDesign"
323-347: Improve dependency array and effect logic.The dependency array includes multiple objects and complex dependencies that could cause unnecessary re-renders. The effect logic has proper guards but could be optimized.
ESLint warns when your effect uses variables not listed in the dependency array, and omitting dependencies might use stale values. Consider splitting this effect or using more specific dependencies:
#!/bin/bash # Check if any of the dependencies in this useEffect are objects that get recreated frequently echo "Checking for object dependencies that might cause unnecessary re-renders..." rg -A 3 -B 3 "containerScheme|backgroundScheme" --type js echo -e "\nLooking for where these scheme objects are created..." ast-grep --pattern 'containerScheme = $_' ast-grep --pattern 'backgroundScheme = $_'The current dependency list mixes primitive values with potentially complex objects, which could lead to performance issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/design-library-list/design-preview.js(2 hunks)src/components/design-library-list/use-preview-renderer.js(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/design-library-list/design-preview.js
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/design-library-list/use-preview-renderer.js (5)
src/components/design-library-list/index.js (2)
props(22-30)shouldRender(95-95)src/components/design-library-list/design-library-list-item.js (3)
props(27-34)spacingSize(38-38)ref(44-44)src/components/design-library-list/use-shadow-root.js (3)
hostRef(11-11)shadowRoot(12-12)useSelect(14-14)src/components/design-library-list/util.js (11)
blocks(235-235)getCategorySlug(325-335)getCategorySlug(325-335)replacePlaceholders(183-193)replacePlaceholders(183-193)cleanParse(15-47)cleanParse(15-47)addBackgroundScheme(106-152)addBackgroundScheme(106-152)adjustPatternSpacing(154-181)adjustPatternSpacing(154-181)src/design-library/index.js (4)
designs(7-7)fetchDesign(41-46)fetchDesign(41-46)design(102-102)
⏰ 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 latest
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 7.3 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 8.2 and WP 6.7.2
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: build
🔇 Additional comments (9)
src/components/design-library-list/use-preview-renderer.js (9)
37-53: Function signature change aligns with new architecture.The updated function signature properly destructures props and adds
shouldRenderas a dedicated parameter. This matches the architectural changes across the PR for implementing an idle-gated rendering system.
76-86: LGTM on the new updateShadowBodySize function.The function correctly extracts shadow body size calculation into a reusable utility. The parameter handling and property extraction are well-structured.
127-140: Fix the preview size initialization logic.The condition
Object.keys( prev ).length === 0correctly identifies when the state is uninitialized (empty object), but this appears to address a previous issue mentioned in the past review comments.
145-158: Similar initialization pattern for cardHeight needs consistency check.The cardHeight initialization uses
! Object.keys( prev ).lengthwhich is equivalent toObject.keys( prev ).length === 0. This is consistent with the previewSize logic above.
230-237: LGTM on template-specific state management.The early return guard and state reset logic properly handles template changes. The spinner re-enabling and ref clearing ensures clean state between different templates.
315-317: Track template changes properly for optimization.The
renderedTemplate.current = templateassignment correctly tracks which template has been rendered, enabling the early return optimization in the useEffect.
319-321: Separate useEffect for selectedTab tracking is correct.This addresses the previous review comment about
prevSelectedTabRefnot being updated. The separate effect ensures the ref stays in sync with selectedTab changes.
350-363: Handle background changes with proper shouldRender gating.The separate effect for background changes is well-structured and properly gated with
shouldRender. The comparison logic prevents unnecessary re-renders when the background state hasn't changed.
392-394: Return object includes new properties for external coordination.The expanded return object now includes
previewSize,cardHeight, andupdateShadowBodySize, which enables better coordination with parent components for sizing operations.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/design-library-list/index.js (1)
95-121: Remove unused ref to avoid noisewrapperRef isn’t used. Drop the ref and its declaration.
-const DesignLibraryItem = props => { - const wrapperRef = useRef( null ) +const DesignLibraryItem = props => { const [ shouldRender, setShouldRender ] = useState( false ) @@ - return ( - <div ref={ wrapperRef }> + return ( + <div> <DesignLibraryListItem { ...props } shouldRender={ shouldRender } /> </div> )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/design-library-list/design-library-list-item.js(5 hunks)src/components/design-library-list/design-preview.js(2 hunks)src/components/design-library-list/index.js(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/components/design-library-list/design-preview.js (2)
src/components/design-library-list/use-auto-scroll.js (1)
NOOP(6-6)src/components/design-library-list/use-preview-renderer.js (2)
blocks(55-55)updateShadowBodySize(76-86)
src/components/design-library-list/design-library-list-item.js (3)
src/components/design-library-list/index.js (2)
props(22-30)shouldRender(96-96)src/components/design-library-list/use-preview-renderer.js (5)
props(41-53)usePreviewRenderer(37-395)usePreviewRenderer(37-395)cardHeight(59-59)updateShadowBodySize(76-86)src/components/design-library-list/use-shadow-root.js (4)
useShadowRoot(10-77)useShadowRoot(10-77)hostRef(11-11)shadowRoot(12-12)
src/components/design-library-list/index.js (3)
src/components/modal-design-library/modal.js (1)
selectedTab(62-62)src/components/design-library-list/design-library-list-item.js (2)
DesignLibraryListItem(26-167)props(27-34)src/components/design-library-list/use-preview-renderer.js (1)
props(41-53)
⏰ 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 6.7.2
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 8.2 and WP 6.5.5
- GitHub Check: PHP 7.3 and WP 6.5.5
🔇 Additional comments (4)
src/components/design-library-list/design-preview.js (1)
83-105: Unify update timing, add Safari-safe idle fallback, avoid redundant writes, and clean up scheduled tasks
- Always call updateShadowBodySize whenever content changes (also in the immediate path).
- Use a setTimeout fallback for requestIdleCallback instead of requestAnimationFrame to avoid nested rAF.
- Skip DOM writes when the HTML hasn’t changed.
- Add cleanup to cancel scheduled callbacks.
- Include selectedTab, designIndex, and updateShadowBodySize in deps to prevent stale closures.
useEffect( () => { const wrapper = wrapperRef.current if ( ! wrapper ) { return } - const ric = window.requestIdleCallback || window.requestAnimationFrame - const sanitizedHTML = safeHTML( blocks ) + const ric = typeof window.requestIdleCallback !== 'undefined' + ? window.requestIdleCallback + : ( cb => setTimeout( cb, 0 ) ) + const newHTML = safeHTML( blocks ) + let idleId + let rafId + + const apply = () => { + if ( wrapper.innerHTML !== newHTML ) { + wrapper.innerHTML = newHTML + updateShadowBodySize() + } + } - if ( selectedTab !== 'pages' || designIndex < 9 ) { - // insert HTML for patterns and for the first 9 pages - wrapper.innerHTML = sanitizedHTML - return - } + if ( selectedTab !== 'pages' || designIndex < 9 ) { + // Patterns and first 9 pages: update immediately if changed + apply() + return + } - requestAnimationFrame( () => { - ric( () => { - wrapper.innerHTML = sanitizedHTML - updateShadowBodySize() - } ) - } ) - }, [ blocks ] ) + rafId = requestAnimationFrame( () => { + idleId = ric( apply ) + } ) + + return () => { + if ( rafId ) cancelAnimationFrame( rafId ) + if ( typeof cancelIdleCallback !== 'undefined' && idleId ) { + cancelIdleCallback( idleId ) + } else if ( idleId ) { + clearTimeout( idleId ) + } + } + }, [ blocks, selectedTab, designIndex, updateShadowBodySize ] )src/components/design-library-list/index.js (1)
95-114: Idle-gated rendering is a solid improvementThe lazy mount via requestIdleCallback with a safe timeout fallback is appropriate and will reduce initial load. Cleanup logic looks correct.
src/components/design-library-list/design-library-list-item.js (2)
42-53: ShadowRoot + preview renderer integration looks goodPassing shouldRender to useShadowRoot and wiring updateShadowBodySize through to DesignPreview aligns with the new rendering model.
72-74: Placeholder short-circuit avoids unnecessary workEarly return when not shouldRender and unselected is a clean way to prevent heavy initialization.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/components/design-library-list/editor.scss (1)
191-198: Opacity transition looks good; add reduced-motion guard.To respect users’ motion preferences, disable the opacity transition when prefers-reduced-motion is set.
.stk-block-design__host-container { position: relative; transition: opacity 0.3s cubic-bezier(0.2, 0.6, 0.4, 1); will-change: opacity; + @media (prefers-reduced-motion: reduce) { + transition: none; + } &.stk-is-loading { opacity: 0; } &.stk-show { opacity: 1; } }src/components/design-library-list/design-preview.js (1)
16-20: Default setIsLoading to NOOP to avoid undefined calls.Prevents runtime errors if the prop isn’t passed.
onMouseDown = NOOP, updateShadowBodySize = NOOP, - setIsLoading, + setIsLoading = NOOP,src/components/design-library-list/design-library-list-item.js (1)
108-114: Expose hidden state to assistive tech.
While opacity toggles visibility, focusable descendants could still be reachable. Mark the host container aria-hidden when loading/not ready.- <div - className={ `stk-block-design__host-container ${ ! shadowRoot || isLoading ? 'stk-is-loading' : 'stk-show' }` } + <div + className={ `stk-block-design__host-container ${ ! shadowRoot || isLoading ? 'stk-is-loading' : 'stk-show' }` } + aria-hidden={ !shadowRoot || isLoading }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/design-library-list/design-library-list-item.js(4 hunks)src/components/design-library-list/design-preview.js(2 hunks)src/components/design-library-list/editor.scss(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/components/design-library-list/design-preview.js (2)
src/components/design-library-list/use-auto-scroll.js (1)
NOOP(6-6)src/components/design-library-list/use-preview-renderer.js (2)
blocks(55-55)updateShadowBodySize(76-86)
src/components/design-library-list/design-library-list-item.js (4)
src/components/design-library-list/index.js (2)
props(22-30)shouldRender(96-96)src/components/design-library-list/use-preview-renderer.js (8)
props(41-53)usePreviewRenderer(37-395)usePreviewRenderer(37-395)shadowBodySizeRef(65-65)previewSize(60-60)cardHeight(59-59)blocks(55-55)updateShadowBodySize(76-86)src/components/design-library-list/use-shadow-root.js (4)
useShadowRoot(10-77)useShadowRoot(10-77)hostRef(11-11)shadowRoot(12-12)src/components/design-library-list/design-preview.js (3)
ref(21-21)DesignPreview(12-124)DesignPreview(12-124)
⏰ 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 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP 6.6.2
- GitHub Check: PHP 8.2 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: PHP 8.2 and WP 6.7.2
🔇 Additional comments (5)
src/components/design-library-list/editor.scss (1)
244-249: Confirm parent height; 100% may collapse without an explicit container height.If the parent of .stk-spinner-container doesn’t have a definite height, the spinner may not center. Verify this in all breakpoints/tabs.
src/components/design-library-list/design-preview.js (1)
119-121: Pointer-events guard on wrapper is a nice safety.
Prevents accidental interactions with previewed blocks.src/components/design-library-list/design-library-list-item.js (3)
42-45: Good: gating shadow root creation with shouldRender.
Reduces work for offscreen/untoggled items.
49-53: Plumbing updateShadowBodySize and setIsLoading through hooks/components is solid.
Keeps sizing and loading state controlled from the source.Please confirm that updateShadowBodySize is invoked for all page previews (including early ones) so auto‑scroll has correct dimensions.
63-66: Use computed enableBackground and nullish coalescing to respect 0 heights.
This avoids mixing props.enableBackground with the hook value and prevents 0 from falling through.- const key = props.enableBackground ? 'background' : 'noBackground' - return cardHeight?.[ key ] || ( props.selectedTab === 'pages' ? 413 : 250 ) + const key = enableBackground ? 'background' : 'noBackground' + return cardHeight?.[ key ] ?? ( selectedTab === 'pages' ? 413 : 250 )
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/design-library-list/index.js (1)
38-40: Guard against null ref before scrolling to topPrevents a potential runtime error if the ref isn’t set yet.
useEffect( () => { - containerRef.current.scrollTop = 0 + if ( containerRef.current ) { + containerRef.current.scrollTop = 0 + } }, [ designs ] )src/components/design-library-list/design-library-list-item.js (1)
110-113: Default scale to 1 to avoid invalid CSSpreviewSize?.scale can be undefined briefly; fall back to 1.
- transform: `scale(${ selectedNum && selectedData ? selectedData.selectedPreviewSize.scale : previewSize?.scale })`, + transform: `scale(${ selectedNum && selectedData ? selectedData.selectedPreviewSize.scale : ( previewSize?.scale ?? 1 ) })`,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/design-library-list/design-library-list-item.js(5 hunks)src/components/design-library-list/design-preview.js(2 hunks)src/components/design-library-list/index.js(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/components/design-library-list/design-preview.js (2)
src/components/design-library-list/use-auto-scroll.js (1)
NOOP(6-6)src/components/design-library-list/use-preview-renderer.js (2)
blocks(55-55)updateShadowBodySize(76-86)
src/components/design-library-list/design-library-list-item.js (5)
src/components/design-library-list/index.js (2)
props(22-30)shouldRender(96-96)src/components/design-library-list/use-preview-renderer.js (8)
props(41-53)usePreviewRenderer(37-395)usePreviewRenderer(37-395)shadowBodySizeRef(65-65)previewSize(60-60)cardHeight(59-59)blocks(55-55)updateShadowBodySize(76-86)src/components/design-library-list/use-shadow-root.js (4)
useShadowRoot(10-77)useShadowRoot(10-77)hostRef(11-11)shadowRoot(12-12)src/components/design-library-list/design-preview.js (3)
ref(21-21)DesignPreview(12-125)DesignPreview(12-125)src/components/design-library-list/use-auto-scroll.js (3)
useAutoScroll(8-147)useAutoScroll(8-147)onMouseDown(140-140)
src/components/design-library-list/index.js (3)
src/components/modal-design-library/modal.js (1)
selectedTab(62-62)src/components/design-library-list/design-library-list-item.js (2)
props(27-34)DesignLibraryListItem(26-164)src/components/design-library-list/use-preview-renderer.js (1)
props(41-53)
⏰ 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.7.2
- GitHub Check: PHP 7.3 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: build
🔇 Additional comments (5)
src/components/design-library-list/index.js (2)
63-73: Prop wiring looks goodPassing stable key, selectedTab, designIndex, and preview props into each item is correct and aligns with downstream hooks.
96-114: Idle-gated shouldRender init is soundStaggered requestIdleCallback/setTimeout with cleanup is appropriate.
src/components/design-library-list/design-library-list-item.js (2)
63-66: Use computed enableBackground and local selectedTabAvoid mixing props and computed values; prevents desync and respects 0 heights with ??.
const getCardHeight = () => { - const key = props.enableBackground ? 'background' : 'noBackground' - return cardHeight?.[ key ] ?? ( props.selectedTab === 'pages' ? 413 : 250 ) + const key = enableBackground ? 'background' : 'noBackground' + return cardHeight?.[ key ] ?? ( selectedTab === 'pages' ? 413 : 250 ) }
116-124: Preview props pass-through looks correctshadowRoot guard and passing updateShadowBodySize/setIsLoading are appropriate.
src/components/design-library-list/design-preview.js (1)
84-112: Ensure loading clears for early path, avoid redundant writes, and fix effect depsEarly branch never clears loading; spinners can stay visible for patterns and first 9 pages. Also, avoid needless DOM writes and include selectedTab/designIndex in deps so logic reruns on tab/index changes.
useEffect( () => { const wrapper = wrapperRef.current if ( ! wrapper || ! blocks ) { return } const ric = window.requestIdleCallback || ( cb => setTimeout( cb, designIndex * 20 ) ) const sanitizedHTML = safeHTML( blocks ) if ( selectedTab !== 'pages' || designIndex < 9 ) { // insert HTML for patterns and for the first 9 pages - wrapper.innerHTML = sanitizedHTML - // No need to call setIsLoading(false) here; isLoading is already false at this point. + if ( wrapper.innerHTML !== sanitizedHTML ) { + wrapper.innerHTML = sanitizedHTML + } + // Keep sizes in sync for early pages. + if ( selectedTab === 'pages' ) { + updateShadowBodySize() + } + // Reveal after write. + ric( () => setIsLoading( false ) ) return } setIsLoading( true ) requestAnimationFrame( () => { ric( () => { - wrapper.innerHTML = sanitizedHTML + if ( wrapper.innerHTML !== sanitizedHTML ) { + wrapper.innerHTML = sanitizedHTML + } updateShadowBodySize() requestAnimationFrame( () => { ric( () => setIsLoading( false ) ) } ) } ) } ) -}, [ blocks ] ) // Only depend on blocks; selectedTab and designIndex changes will cause blocks to update +}, [ blocks, selectedTab, designIndex ] )
- apply useCallbacks, useMemo, and memo - fix preview when loading
…hub.com/gambitph/Stackable into fix/design-library-full-pages-scrolling
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/components/modal-design-library/index.js (1)
28-35: Stabilize callback deps to satisfy exhaustive-deps and avoid stale closuresInclude the setter in the dependency array. It’s stable, but this silences lint and documents intent.
-const onChangeApiVersion = useCallback( v => setApiVersion( v ), [] ) +const onChangeApiVersion = useCallback( v => setApiVersion( v ), [ setApiVersion ] )src/components/design-library-list/index.js (2)
95-121: Cap Safari idle fallback delay to avoid multi‑second stalls
setTimeout(…, designIndex * 20)can defer rendering by several seconds for large lists on browsers withoutrequestIdleCallback(e.g., Safari). Use a small constant delay.- if ( typeof requestIdleCallback !== 'undefined' ) { - id = requestIdleCallback( () => ! shouldRender ? setShouldRender( true ) : {} ) - } else { - // fallback - id = setTimeout( () => setShouldRender( true ), designIndex * 20 ) - } + if ( typeof requestIdleCallback !== 'undefined' ) { + id = requestIdleCallback( () => { if ( ! shouldRender ) setShouldRender( true ) } ) + } else { + id = setTimeout( () => { if ( ! shouldRender ) setShouldRender( true ) }, 16 ) + }
132-147: Remove unsupported IntersectionObserver option
scrollMarginis not a valid observer option; it has no effect. KeeprootMargin.- }, { - root: rootEl, - rootMargin: '500px', - scrollMargin: '500px', - threshold: 0, - } ) + }, { + root: rootEl, + rootMargin: '500px', + threshold: 0, + } )src/components/design-library-list/design-library-list-item.js (1)
106-124: Spinner visibility depends on setIsLoading from DesignPreview early pathThis relies on the preview setting
isLoadingto false; ensure the DesignPreview early branch clears it (see linked comment).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/block/design-library/edit.js(3 hunks)src/components/design-library-list/design-library-list-item.js(2 hunks)src/components/design-library-list/design-preview.js(2 hunks)src/components/design-library-list/index.js(5 hunks)src/components/modal-design-library/editor.scss(1 hunks)src/components/modal-design-library/index.js(2 hunks)src/components/modal-design-library/modal.js(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
src/components/modal-design-library/modal.js (1)
src/components/design-library-list/util.js (1)
blocksForSubstitution(213-213)
src/components/design-library-list/index.js (2)
src/components/design-library-list/design-library-list-item.js (2)
props(28-36)DesignLibraryListItem(27-164)src/hooks/use-preset-controls.js (1)
getPresetMarks(114-126)
src/components/design-library-list/design-library-list-item.js (5)
src/components/design-library-list/index.js (4)
props(23-31)props(96-96)presetMarks(102-102)shouldRender(98-98)src/components/design-library-list/use-preview-renderer.js (8)
props(41-53)usePreviewRenderer(37-395)usePreviewRenderer(37-395)shadowBodySizeRef(65-65)previewSize(60-60)cardHeight(59-59)blocks(55-55)updateShadowBodySize(76-86)src/components/design-library-list/use-shadow-root.js (4)
useShadowRoot(10-77)useShadowRoot(10-77)hostRef(11-11)shadowRoot(12-12)src/components/design-library-list/design-preview.js (3)
ref(21-21)DesignPreview(12-125)DesignPreview(12-125)src/components/design-library-list/use-auto-scroll.js (3)
useAutoScroll(10-149)useAutoScroll(10-149)onMouseDown(142-142)
src/block/design-library/edit.js (2)
src/components/design-library-list/util.js (2)
disabledBlocks(212-212)blocksForSubstitution(213-213)src/util/blocks.js (1)
disabledBlocks(555-555)
src/components/modal-design-library/index.js (2)
src/deprecated/v2/components/design-library-control/index.js (1)
apiVersion(30-30)src/deprecated/v2/components/modal-design-library/index.js (1)
apiVersion(45-45)
src/components/design-library-list/design-preview.js (4)
src/components/design-library-list/use-auto-scroll.js (1)
NOOP(8-8)src/components/design-library-list/index.js (1)
wrapperRef(97-97)src/components/design-library-list/use-preview-renderer.js (2)
blocks(55-55)updateShadowBodySize(76-86)src/components/design-library-list/use-shadow-root.js (1)
shadowRoot(12-12)
⏰ 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 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 6.5.5
- GitHub Check: PHP 7.3 and WP latest
- GitHub Check: PHP 8.2 and WP 6.5.5
🔇 Additional comments (8)
src/components/modal-design-library/editor.scss (1)
46-49: Explicit row span looks goodLocking the full-pages designs pane to
1 / 4keeps it consuming every track in the three-row grid after dropping the custom template override. This should help the content stay flush with the footer. Nicely done.src/block/design-library/edit.js (2)
331-332: LGTM on memoized onCloseStable and side‑effect free.
402-404: LGTM on passing memoized handlersPassing memoized onClose/onSelect helps avoid unnecessary re-renders downstream.
src/components/modal-design-library/modal.js (1)
432-433: LGTM on consolidating selection handlerPassing a single, memoized
onSelectMultistreamlines list item behavior and prevents prop churn.src/components/design-library-list/index.js (2)
66-74: LGTM on stable keys and passing new propsGood use of
design.id || design.designIdand forwardingselectedTab/designIndex.
151-152: LGTM on deferring render via shouldRender + preset marksClean placeholder approach; avoids heavy work offscreen.
src/components/design-library-list/design-library-list-item.js (1)
63-66: Use computed enableBackground and nullish coalescing for heightAligns with preview renderer state and respects 0 heights.
-const getCardHeight = () => { - const key = props.enableBackground ? 'background' : 'noBackground' - return cardHeight?.[ key ] ?? ( props.selectedTab === 'pages' ? 413 : 250 ) -} +const getCardHeight = () => { + const key = enableBackground ? 'background' : 'noBackground' + return cardHeight?.[ key ] ?? ( selectedTab === 'pages' ? 413 : 250 ) +}src/components/design-library-list/design-preview.js (1)
84-113: Early branch leaves spinner visible; Safari fallback delays are excessive; missing size update for early pages
isLoadingstarts true in the item and isn’t cleared on the early path.setTimeout(cb, designIndex * 20)can delay long lists by seconds on Safari (no rIC).- Not updating sizes for the first 9 pages can break auto-scroll.
Apply:
- const ric = window.requestIdleCallback || ( cb => setTimeout( cb, designIndex * 20 ) ) + const ric = window.requestIdleCallback || ( cb => setTimeout( cb, 16 ) ) const sanitizedHTML = safeHTML( blocks ) - if ( selectedTab !== 'pages' || designIndex < 9 ) { - // insert HTML for patterns and for the first 9 pages - wrapper.innerHTML = sanitizedHTML - // No need to call setIsLoading(false) here; isLoading is already false at this point. - return - } + if ( selectedTab !== 'pages' || designIndex < 9 ) { + // Insert HTML for patterns and early pages + if ( wrapper.innerHTML !== sanitizedHTML ) { + wrapper.innerHTML = sanitizedHTML + } + if ( selectedTab === 'pages' ) { + updateShadowBodySize() + } + ric( () => setIsLoading( false ) ) + return + } @@ - }, [ blocks, shadowRoot ] ) // Only depend on blocks and shadowRoot; selectedTab and designIndex changes will cause blocks to update + }, [ blocks, shadowRoot, selectedTab, designIndex, updateShadowBodySize ] )
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/components/design-library-list/design-preview.js (1)
16-20: Default setIsLoading to NOOP to prevent hard crashes if omitted.Prevents runtime errors if a caller forgets to pass setIsLoading.
designIndex, onMouseDown = NOOP, updateShadowBodySize = NOOP, - setIsLoading, + setIsLoading = NOOP,src/components/design-library-list/index.js (1)
141-145: Remove unsupported IntersectionObserver option.scrollMargin is not a valid IntersectionObserver option; it’s a CSS property and is ignored here.
}, { root: rootEl, rootMargin: '500px', - scrollMargin: '500px', threshold: 0, } )src/components/design-library-list/design-library-list-item.js (1)
108-112: Default scale to 1 to avoid invalid CSS during first render.Prevents transform: scale(undefined) before previewSize is computed.
- transform: `scale(${ selectedNum && selectedData ? selectedData.selectedPreviewSize.scale : previewSize?.scale })`, + transform: `scale(${ selectedNum && selectedData ? selectedData.selectedPreviewSize.scale : ( previewSize?.scale ?? 1 ) })`,
📜 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(3 hunks)src/components/design-library-list/design-preview.js(2 hunks)src/components/design-library-list/editor.scss(1 hunks)src/components/design-library-list/index.js(5 hunks)src/components/design-library-list/use-preview-renderer.js(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/design-library-list/editor.scss
🧰 Additional context used
🧬 Code graph analysis (4)
src/components/design-library-list/design-preview.js (3)
src/components/design-library-list/use-auto-scroll.js (1)
NOOP(8-8)src/components/design-library-list/use-preview-renderer.js (2)
blocks(55-55)updateShadowBodySize(75-85)src/components/design-library-list/use-shadow-root.js (1)
shadowRoot(12-12)
src/components/design-library-list/use-preview-renderer.js (4)
src/components/design-library-list/design-library-list-item.js (3)
props(28-36)spacingSize(38-38)ref(44-44)src/components/design-library-list/index.js (3)
props(23-31)props(96-96)shouldRender(98-98)src/components/design-library-list/util.js (11)
blocks(235-235)getCategorySlug(325-335)getCategorySlug(325-335)replacePlaceholders(183-193)replacePlaceholders(183-193)cleanParse(15-47)cleanParse(15-47)addBackgroundScheme(106-152)addBackgroundScheme(106-152)adjustPatternSpacing(154-181)adjustPatternSpacing(154-181)src/design-library/index.js (4)
designs(7-7)fetchDesign(41-46)fetchDesign(41-46)design(102-102)
src/components/design-library-list/design-library-list-item.js (5)
src/components/design-library-list/index.js (4)
props(23-31)props(96-96)presetMarks(102-102)shouldRender(98-98)src/components/design-library-list/use-preview-renderer.js (7)
props(41-53)usePreviewRenderer(37-379)usePreviewRenderer(37-379)shadowBodySizeRef(64-64)previewSize(59-59)blocks(55-55)updateShadowBodySize(75-85)src/components/design-library-list/use-shadow-root.js (4)
useShadowRoot(10-77)useShadowRoot(10-77)hostRef(11-11)shadowRoot(12-12)src/components/design-library-list/design-preview.js (3)
ref(21-21)DesignPreview(12-127)DesignPreview(12-127)src/components/design-library-list/use-auto-scroll.js (3)
useAutoScroll(10-149)useAutoScroll(10-149)onMouseDown(142-142)
src/components/design-library-list/index.js (3)
src/components/design-library-list/design-library-list-item.js (2)
props(28-36)DesignLibraryListItem(27-163)src/components/modal-design-library/modal.js (1)
selectedTab(64-64)src/hooks/use-preset-controls.js (1)
getPresetMarks(114-126)
⏰ 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 7.3 and WP 6.5.5
- GitHub Check: PHP 8.2 and WP latest
- GitHub Check: PHP 7.3 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 8.2 and WP 6.5.5
🔇 Additional comments (3)
src/components/design-library-list/use-preview-renderer.js (2)
126-139: Preview size init fix looks good.The switch to checking for an empty prev object ensures both heights are initialized on first run.
303-305: Good: prevSelectedTabRef kept in sync.This addresses the stale-ref issue for tab-change logic.
src/components/design-library-list/design-preview.js (1)
84-114: Ensure effect reacts to tab/index changes and avoid redundant writes; update size for early pages.The effect doesn’t depend on selectedTab/designIndex, so branch changes won’t run when blocks are unchanged. Also, skip redundant innerHTML writes and update size for first 9 pages to keep auto‑scroll sizing accurate.
Apply:
- useEffect( () => { + useEffect( () => { const wrapper = wrapperRef.current if ( ! wrapper || ! blocks ) { return } setIsLoading( true ) const ric = window.requestIdleCallback || ( cb => setTimeout( cb, designIndex * 20 ) ) const sanitizedHTML = safeHTML( blocks ) if ( selectedTab !== 'pages' || designIndex < 9 ) { // insert HTML for patterns and for the first 9 pages - wrapper.innerHTML = sanitizedHTML + if ( wrapper.innerHTML !== sanitizedHTML ) { + wrapper.innerHTML = sanitizedHTML + } + if ( selectedTab === 'pages' ) { + updateShadowBodySize() + } requestAnimationFrame( () => { ric( () => setIsLoading( false ) ) } ) return } requestAnimationFrame( () => { ric( () => { - wrapper.innerHTML = sanitizedHTML + if ( wrapper.innerHTML !== sanitizedHTML ) { + wrapper.innerHTML = sanitizedHTML + } updateShadowBodySize() requestAnimationFrame( () => { ric( () => setIsLoading( false ) ) } ) } ) } ) - }, [ blocks, shadowRoot ] ) // Only depend on blocks and shadowRoot; selectedTab and designIndex changes will cause blocks to update + }, [ blocks, shadowRoot, selectedTab, designIndex ] )
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/components/modal-design-library/modal.js (1)
432-432: Ensure onSelectMulti is referentially stable to preserve list item memoizationPassing a re-created handler on every selection can force a re-render of the memoized list. After applying the functional update refactor and memoizing addDesign, onSelectDesign’s identity will be stable across selections.
Please profile DesignLibraryList re-renders (React DevTools Profiler) while selecting/deselecting items and switching tabs. Expect only affected items to re-render, not the entire list.
src/components/design-library-list/design-library-list-item.js (2)
105-105: Hide spinner when not rendering to reduce visual noiseShow spinner only while actively rendering and loading.
-<div className={ `stk-spinner-container ${ isLoading || ! shouldRender ? '' : 'stk-hide-spinner' }` }><Spinner /></div> +<div className={ `stk-spinner-container ${ isLoading && shouldRender ? '' : 'stk-hide-spinner' }` }><Spinner /></div>
109-111: Default scale to 1 to avoid invalid CSSpreviewSize?.scale can be undefined initially, producing an invalid transform.
- transform: `scale(${ selectedNum && selectedData ? selectedData.selectedPreviewSize.scale : previewSize?.scale })`, + transform: `scale(${ selectedNum && selectedData ? selectedData.selectedPreviewSize.scale : ( previewSize?.scale ?? 1 ) })`,src/components/design-library-list/use-preview-renderer.js (1)
212-301: Add cancellation guard for async initialize to prevent stale updatesAvoid setState after unmount/template change and stop spinner reliably on errors.
useEffect( () => { if ( ! shouldRender || renderedTemplate.current === template ) { return } + let isCancelled = false + // Reset per-template state and show spinner setIsLoading( true ) categoriesRef.current = [] hasBackgroundTargetRef.current = false @@ - initialize().then( () => { + initialize().then( () => { + if ( isCancelled ) return // We need to parse the content because this is what we use to insert the blocks in the Editor const [ parsedBlocks, blocksForSubstitution ] = parseDisabledBlocks( _parsedBlocks ) const parsedBlocksForInsertion = _parsedBlocksForInsertion ? parseDisabledBlocks( _parsedBlocksForInsertion )[ 0 ] : null blocksForSubstitutionRef.current = blocksForSubstitution setContent( parsedBlocks ) setContentForInsertion( parsedBlocksForInsertion ) setIsLoading( false ) renderedTemplate.current = template - } ) + } ).catch( () => { + if ( ! isCancelled ) { + setIsLoading( false ) + } + } ) -}, [ template, shouldRender ] ) + return () => { + isCancelled = true + } +}, [ template, shouldRender ] )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/design-library-list/design-library-list-item.js(3 hunks)src/components/design-library-list/use-preview-renderer.js(8 hunks)src/components/modal-design-library/modal.js(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/components/modal-design-library/modal.js (1)
src/components/design-library-list/util.js (1)
blocksForSubstitution(213-213)
src/components/design-library-list/use-preview-renderer.js (5)
src/components/design-library-list/design-library-list-item.js (3)
props(28-36)spacingSize(38-38)ref(44-44)src/components/design-library-list/index.js (3)
props(23-31)props(96-96)shouldRender(98-98)src/components/design-library-list/use-shadow-root.js (3)
hostRef(11-11)shadowRoot(12-12)useSelect(14-14)src/components/design-library-list/util.js (11)
blocks(235-235)getCategorySlug(325-335)getCategorySlug(325-335)replacePlaceholders(183-193)replacePlaceholders(183-193)cleanParse(15-47)cleanParse(15-47)addBackgroundScheme(106-152)addBackgroundScheme(106-152)adjustPatternSpacing(154-181)adjustPatternSpacing(154-181)src/design-library/index.js (4)
designs(7-7)fetchDesign(41-46)fetchDesign(41-46)design(102-102)
src/components/design-library-list/design-library-list-item.js (5)
src/components/design-library-list/use-preview-renderer.js (7)
props(41-53)usePreviewRenderer(37-379)usePreviewRenderer(37-379)shadowBodySizeRef(64-64)previewSize(59-59)blocks(55-55)updateShadowBodySize(75-85)src/components/design-library-list/index.js (4)
props(23-31)props(96-96)presetMarks(102-102)shouldRender(98-98)src/components/design-library-list/use-shadow-root.js (4)
useShadowRoot(10-77)useShadowRoot(10-77)hostRef(11-11)shadowRoot(12-12)src/components/design-library-list/design-preview.js (3)
ref(21-21)DesignPreview(12-127)DesignPreview(12-127)src/components/design-library-list/use-auto-scroll.js (3)
useAutoScroll(10-149)useAutoScroll(10-149)onMouseDown(142-142)
⏰ 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.5.5
- 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.7.2
- GitHub Check: PHP 7.3 and WP latest
🔇 Additional comments (2)
src/components/modal-design-library/modal.js (2)
32-34: LGTM on importing useCallbackImport looks correct and aligns with new memoized handler usage.
134-166: Stabilize onSelectDesign and use functional state updates to prevent stale closures and excess re-rendersCurrent deps include arrays, recreating the callback on every selection and defeating list item memoization; also non‑functional setState risks lost updates under rapid clicks. Use functional updates and depend only on selectedTab and a memoized addDesign.
Apply within this hunk:
-const onSelectDesign = useCallback( ( designId, category, parsedBlocks, blocksForSubstitution, selectedPreviewSize ) => { - if ( selectedTab === 'pages' ) { - const selectedDesign = [ { - designId, category, designData: parsedBlocks, blocksForSubstitution, selectedPreviewSize, - } ] - addDesign( selectedDesign ) - - return - } - - const newSelectedDesigns = [ ...selectedDesignIds ] - // We also get the design data from displayDesigns - // already instead of after clicking the "Add - // Designs" button since displayDesigns can change - // when the user is switching tabs (block/ui - // kits/wireframes) and the data can be lost. - const newSelectedDesignData = [ ...selectedDesignData ] - - if ( newSelectedDesigns.includes( designId ) ) { - const i = newSelectedDesigns.indexOf( designId ) - newSelectedDesigns.splice( i, 1 ) - setSelectedDesignIds( newSelectedDesigns ) - newSelectedDesignData.splice( i, 1 ) - setSelectedDesignData( newSelectedDesignData ) - } else { - newSelectedDesigns.push( designId ) - setSelectedDesignIds( newSelectedDesigns ) - newSelectedDesignData.push( { - designId, category, designData: parsedBlocks, blocksForSubstitution, selectedPreviewSize, - } ) - setSelectedDesignData( newSelectedDesignData ) - } -}, [ selectedTab, selectedDesignIds, selectedDesignData ] ) +const onSelectDesign = useCallback( ( designId, category, parsedBlocks, blocksForSubstitution, selectedPreviewSize ) => { + if ( selectedTab === 'pages' ) { + addDesign( [ { designId, category, designData: parsedBlocks, blocksForSubstitution, selectedPreviewSize } ] ) + return + } + setSelectedDesignIds( prevIds => { + const idx = prevIds.indexOf( designId ) + if ( idx !== -1 ) { + setSelectedDesignData( prevData => { + const nextData = prevData.slice() + nextData.splice( idx, 1 ) + return nextData + } ) + const nextIds = prevIds.slice() + nextIds.splice( idx, 1 ) + return nextIds + } + setSelectedDesignData( prevData => ( [ + ...prevData, + { designId, category, designData: parsedBlocks, blocksForSubstitution, selectedPreviewSize }, + ] ) ) + return [ ...prevIds, designId ] + } ) +}, [ selectedTab, addDesign ] )Additionally (outside this hunk), memoize addDesign so onSelectDesign can safely depend on it:
// Replace current addDesign with: const addDesign = useCallback( designs => { setIsMultiSelectBusy( true ) const cb = () => setIsMultiSelectBusy( false ) props.onSelect( designs, cb, selectedTab ) }, [ props.onSelect, selectedTab ] )
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/design-library-list/editor.scss(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
Summary by CodeRabbit
New Features
Performance
UI/UX
Reliability