Skip to content

Conversation

kaeizen
Copy link
Contributor

@kaeizen kaeizen commented Oct 14, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Prevents duplicate preview renders, reducing flicker when switching tabs or selections.
    • Ensures preview scaling updates at the right time for consistent sizing.
    • Improves handling of theme changes, keeping previews in sync with color scheme updates.
  • Refactor

    • Streamlines the preview rendering flow to avoid unnecessary work, improving responsiveness and stability when navigating categories, tabs, and selections.

Copy link

coderabbitai bot commented Oct 14, 2025

Walkthrough

Introduces prevSelectedNumRef and new useEffect blocks to gate preview rendering based on selectedNum, shadowRoot, content, and color scheme changes. Adds early returns to prevent duplicate renders, coordinates renderPreview calls, and schedules scale adjustments via requestAnimationFrame. Maintains prevSelectedTabRef behavior while integrating new guard logic into the render/scale flow.

Changes

Cohort / File(s) Summary of edits
Preview render flow guards and sequencing
src/components/design-library-list/use-preview-renderer.js
Added prevSelectedNumRef; introduced effects to guard re-renders on selectedNum/shadowRoot/content changes; conditional renderPreview calls; coordinated scale adjustments via requestAnimationFrame; retained prevSelectedTabRef with updated sequencing and guards.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor C as Component
  participant H as usePreviewRenderer (hook)
  participant E1 as useEffect: selection/shadowRoot
  participant E2 as useEffect: render & scale
  participant DOM as shadowRoot
  participant R as renderPreview
  participant RAF as requestAnimationFrame

  C->>H: Invoke with props (selectedNum, content, colorScheme, shadowRoot)
  H->>E1: Monitor selectedNum, shadowRoot
  Note over E1: Guard transitions, update prevSelectedNumRef
  E1-->>R: Conditionally call renderPreview when content exists
  H->>E2: Monitor content, colorScheme, selection state
  Note over E2: Skip on guarded states
  E2->>R: Render preview when required
  R-->>DOM: Update preview DOM
  E2->>RAF: Schedule scale adjustment
  RAF-->>DOM: Apply scale changes
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

A nibble of state, a twitch of an ear,
I hop through effects to make previews clear.
Guarded I render, then scale with a spin,
A frame at a time—let smoothness begin.
Carrots of logic, no double-chew here—
The canvas looks crisp; the codepath sincere. 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly describes the primary change—ensuring the design library preview re-renders when switching categories—and matches the fix implemented in the changeset. It is clear, specific, and directly related to the added guard logic for preview rendering.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/design-library-selected-designs-are-not-rendering

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1e56ba3 and e0fd88b.

📒 Files selected for processing (1)
  • src/components/design-library-list/use-preview-renderer.js (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 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.564Z
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.
📚 Learning: 2025-09-25T08:17:15.564Z
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.564Z
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.js
🧬 Code graph analysis (1)
src/components/design-library-list/use-preview-renderer.js (2)
src/components/design-library-list/index.js (1)
  • shouldRender (77-77)
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.5.5
  • 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 latest
  • GitHub Check: PHP 7.3 and WP latest
  • GitHub Check: PHP 7.3 and WP 6.5.5
🔇 Additional comments (2)
src/components/design-library-list/use-preview-renderer.js (2)

67-67: LGTM!

The addition of prevSelectedNumRef follows the existing pattern established by prevSelectedTabRef for tracking previous prop values.


308-327: Simplify guard logic, remove redundancies, and verify category-change triggers

  • Extract the complex condition at line 317 into a named constant (e.g., shouldSkipRender) for clarity.
  • Remove the duplicated prevSelectedNumRef.current assignment and the redundant !content check.
  • Verify that changing category (while selectedNum stays the same) still calls renderPreview() (via this or the downstream effect).

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

🤖 Pull request artifacts

file commit
pr3627-stackable-3627-merge.zip e0fd88b

github-actions bot added a commit that referenced this pull request Oct 14, 2025
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.

1 participant