fix(playground): resolve blank screen when clicking code & preview buttons
#348
Conversation
|
@SwasthK is attempting to deploy a commit to the Dodo Payments Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughLoads component list on mount, auto-selects a default pricing component via existing handler, groups items by category with adjusted styling, and centralizes panel toggles into a single togglePanel(signal) helper with a new "both" UI control. Changes
Sequence Diagram(s)(omitted — changes are UI/control-flow refinements that don't introduce a new multi-component sequential flow requiring a diagram) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/playground/playground.tsx (1)
116-144: Button active states cause visual confusion when both panels are shown.When "both" mode is active (
showCodePanel && showPreviewPanel), all three buttons will appear highlighted because:
- Code button:
showCodePanelis true → highlighted- Both button: both are true → highlighted
- Preview button:
showPreviewPanelis true → highlightedThis makes it unclear which mode is actually selected. The conditions should be mutually exclusive:
🔎 Proposed fix for exclusive active states
<Button size="sm" variant="ghost" onClick={() => togglePanel('code')} style={{ boxShadow: "inset 0 2px 4px rgba(202, 199, 199, 0.41)" }} className={cn( "text-muted-foreground hover:!bg-accent", - showCodePanel && "text-foreground bg-accent-foreground dark:bg-accent/50", + showCodePanel && !showPreviewPanel && "text-foreground bg-accent-foreground dark:bg-accent/50", )} > ... <Button size="sm" variant="ghost" onClick={() => togglePanel('preview')} style={{ boxShadow: "inset 0 2px 4px rgba(202, 199, 199, 0.41)" }} className={cn( "text-muted-foreground hover:!bg-accent", - showPreviewPanel && "text-foreground bg-accent-foreground dark:bg-accent/50" + showPreviewPanel && !showCodePanel && "text-foreground bg-accent-foreground dark:bg-accent/50" )} >
🧹 Nitpick comments (5)
src/components/playground/playground-header.tsx (3)
63-81: Missing dependencies in useEffect and no error handling forgetComponentList.The effect uses
handleComponentChangeandstate.selectedComponentbut the dependency array is empty. This will trigger ESLint exhaustive-deps warnings. Additionally, ifgetComponentList()throws an exception, the component will be stuck in the loading state.Consider wrapping
handleComponentChangeinuseCallbackand adding proper error handling:🔎 Proposed fix
+ const handleComponentChange = useCallback(async (componentId: string) => { + setIsLoadingComponent(true); + setLoadError(null); + // ... rest of implementation + }, [setSelectedComponent]); useEffect(() => { async function loadComponentList() { setIsLoading(true); - const list = await getComponentList(); - setComponentList(list); + try { + const list = await getComponentList(); + setComponentList(list); - // Default selection (pricing first item) - if (!state.selectedComponent) { - const pricingComponent = list.find((comp) => comp.category === "pricing"); - if (pricingComponent) { - await handleComponentChange(pricingComponent.id); + // Default selection (pricing first item) + if (!state.selectedComponent) { + const pricingComponent = list.find((comp) => comp.category === "pricing"); + if (pricingComponent) { + await handleComponentChange(pricingComponent.id); + } } + } catch (error) { + console.error("[PlaygroundHeader] Failed to load component list:", error); } setIsLoading(false); } loadComponentList(); - }, []); + }, [handleComponentChange, state.selectedComponent]);
27-27: Unused state setter forselectedCategory.
selectedCategoryis declared withuseStatebut the setter is destructured away and never used, making it effectively a constant. The filtering logic at lines 91-96 always uses"all".🔎 Proposed simplification
If category filtering isn't needed yet, simplify to a constant:
- const [selectedCategory] = useState<string>("all"); + const selectedCategory = "all";Or remove the
filteredComponentslogic entirely if all components should always be shown.
178-182: Dead code condition:category.id === "all"can never be true.The
categoriesarray (lines 83-89) does not contain an entry withid: "all", so the conditioncategory.id === "all"at line 181 will never evaluate to true.🔎 Proposed fix
{filteredComponents .filter( (comp: ComponentListItem) => - comp.category === category.id || category.id === "all", + comp.category === category.id, ) .map((component: ComponentListItem) => (src/components/playground/playground.tsx (2)
117-136: Extract repeated inline style to a constant.The
boxShadowstyle is duplicated across all three buttons. Consider extracting it to improve maintainability.🔎 Proposed refactor
+ const panelButtonStyle = { boxShadow: "inset 0 2px 4px rgba(202, 199, 199, 0.41)" }; - <Button ... style={{ boxShadow: "inset 0 2px 4px rgba(202, 199, 199, 0.41)" }} ...> + <Button ... style={panelButtonStyle} ...>
12-12: Consider renaming the Link icon import to avoid confusion.Importing
Linkfromlucide-reactcould cause confusion sinceLinktypically refers to Next.js's routing component. Consider using an alias for clarity.- import { Link, PanelLeft, PanelRight } from "lucide-react"; + import { Link as LinkIcon, PanelLeft, PanelRight } from "lucide-react";Then update line 133:
- <Link className={cn("rotate-45", ...)} /> + <LinkIcon className={cn("rotate-45", ...)} />
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/playground/playground-header.tsxsrc/components/playground/playground.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/playground/playground-header.tsx (1)
src/components/playground/auto-discovery.ts (2)
getComponentList(279-294)ComponentListItem(272-277)
🔇 Additional comments (2)
src/components/playground/playground-header.tsx (1)
174-194: Grouped category rendering looks good.The UI improvement to group components by category with clear labels enhances discoverability. The structure is clean and well-organized.
src/components/playground/playground.tsx (1)
24-39: LGTM!The
togglePanelfunction cleanly consolidates the panel visibility logic with a clear signal-based approach.
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/playground/playground.tsx (1)
116-162: Fix the button active state logic to prevent multiple buttons appearing active simultaneously.The current logic allows all three buttons (Code, Both, Preview) to appear active when both panels are visible, creating confusing UX where users cannot determine the current mode at a glance.
Current behavior:
- Both panels visible → All three buttons active ✗
- Code only → Code button active ✓
- Preview only → Preview button active ✓
Expected behavior:
- Both panels visible → Only "Both" button active
- Code only → Only Code button active
- Preview only → Only Preview button active
🔎 Proposed fix for mutually exclusive active states
<Button size="sm" variant="ghost" onClick={() => togglePanel("code")} style={{ boxShadow: "inset 0 2px 4px rgba(202, 199, 199, 0.41)" }} className={cn( "text-muted-foreground hover:!bg-accent", - showCodePanel && + showCodePanel && !showPreviewPanel && "text-foreground bg-accent-foreground dark:bg-accent/50", )} > <PanelLeft className="mr-1 h-4 w-4" /> Code </Button> <Button size="sm" variant="ghost" onClick={() => togglePanel("both")} style={{ boxShadow: "inset 0 2px 4px rgba(202, 199, 199, 0.41)" }} className={cn( "text-muted-foreground hover:!bg-accent", showCodePanel && showPreviewPanel && "text-foreground bg-accent-foreground dark:bg-accent/50", )} > <Link className={cn( "rotate-45", - showCodePanel && showPreviewPanel && "text-foreground", )} /> </Button> <Button size="sm" variant="ghost" onClick={() => togglePanel("preview")} style={{ boxShadow: "inset 0 2px 4px rgba(202, 199, 199, 0.41)" }} className={cn( "text-muted-foreground hover:!bg-accent", - showPreviewPanel && + !showCodePanel && showPreviewPanel && "text-foreground bg-accent-foreground dark:bg-accent/50", )} > <PanelRight className="mr-1 h-4 w-4" /> Preview </Button>
🧹 Nitpick comments (5)
src/components/playground/playground-header.tsx (2)
85-91: Move static categories array outside component.The
categoriesarray is constant data but is being recreated on every render. For better performance, define it outside the component body.🔎 Proposed fix
Move the array definition before the component:
+const CATEGORIES = [ + { id: "pricing", label: "Pricing Tables" }, + { id: "subscription", label: "Subscription" }, + { id: "payment", label: "Payment" }, + { id: "usage", label: "Usage & Billing" }, + { id: "ui", label: "UI Components" }, +] as const; + export function PlaygroundHeader() { const { state, setSelectedComponent, updateCode, updateStyles } = usePlayground(); const [selectedCategory] = useState<string>("all"); const [componentList, setComponentList] = useState<ComponentListItem[]>([]); const [isLoading, setIsLoading] = useState(true); const [isLoadingComponent, setIsLoadingComponent] = useState(false); const [loadError, setLoadError] = useState<string | null>(null); ... - const categories = [ - { id: "pricing", label: "Pricing Tables" }, - { id: "subscription", label: "Subscription" }, - { id: "payment", label: "Payment" }, - { id: "usage", label: "Usage & Billing" }, - { id: "ui", label: "UI Components" }, - ]; const filteredComponents = selectedCategory === "all" ? componentList : componentList.filter( (comp: ComponentListItem) => comp.category === selectedCategory, );Then update the reference in the JSX:
- {categories.map((category) => ( + {CATEGORIES.map((category) => ( <div key={category.id} className="flex flex-col gap-2 py-2">
93-98: Incomplete category filtering logic.The
selectedCategorystate is initialized to"all"(line 27) and never updated, making this filtering logic unreachable dead code. Additionally, line 183 checks forcategory.id === "all", but no category in the list has that ID.If category filtering is intended for future functionality, consider adding TODO comments. Otherwise, simplify by removing the unused filtering.
🔎 Simplification option
If category filtering is not needed yet, simplify to:
- const filteredComponents = - selectedCategory === "all" - ? componentList - : componentList.filter( - (comp: ComponentListItem) => comp.category === selectedCategory, - );And update line 182-183:
.filter( (comp: ComponentListItem) => - comp.category === category.id || category.id === "all", + comp.category === category.id, )src/components/playground/playground.tsx (3)
120-120: Replace hardcoded shadow colors with theme tokens.The inline
boxShadowstyle uses hardcoded RGBA values (rgba(202, 199, 199, 0.41)) that bypass the theme system. This can cause visual inconsistencies in dark mode or custom themes and makes the style harder to maintain since it's duplicated across three buttons.Consider extracting this to a CSS class using theme variables or a constant.
Also applies to: 134-134, 153-153
122-122: Consider removing!importantfrom hover styles.The
hover:!bg-accentclasses use Tailwind's!prefix (compiles to!important), which suggests there are conflicting styles requiring force override. This is generally a code smell and can make styles harder to maintain and debug.Consider investigating the root cause of style conflicts and resolving them through proper CSS specificity rather than using
!important.Also applies to: 136-136, 155-155
142-147: Remove redundant conditional styling on icon.The Link icon applies a conditional
text-foregroundclass when both panels are visible, but the button itself (lines 137-139) already applies the same class in the same condition. Icons inherit color from their parent button, making this redundant.🔎 Simplified version
<Link - className={cn( - "rotate-45", - showCodePanel && showPreviewPanel && "text-foreground", - )} + className="rotate-45" />
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/playground/playground-header.tsxsrc/components/playground/playground.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
src/components/playground/playground-header.tsx (1)
src/components/playground/auto-discovery.ts (2)
getComponentList(279-294)ComponentListItem(272-277)
src/components/playground/playground.tsx (1)
src/lib/utils.ts (1)
cn(4-6)
🔇 Additional comments (4)
src/components/playground/playground-header.tsx (2)
176-179: LGTM! Clear visual hierarchy for categories.The styling changes effectively create visual differentiation between category labels and component names, aligning with the PR objectives. The Tailwind classes are appropriate and create a clean, organized dropdown structure.
189-189: LGTM! Item indentation improves hierarchy.The left margin appropriately indents component items under their category labels, creating a clear parent-child visual relationship.
src/components/playground/playground.tsx (2)
12-12: LGTM!The Link icon import is correctly added and used for the new "both panels" toggle button.
24-39: LGTM!The centralized
togglePanelfunction cleanly consolidates the panel visibility logic and handles all three modes (code-only, preview-only, and both) correctly.
| // Load lightweight component list on mount (no actual imports) | ||
| useEffect(() => { | ||
| async function loadComponentList() { | ||
| setIsLoading(true); | ||
| const list = await getComponentList(); | ||
| setComponentList(list); | ||
|
|
||
| // Default selection (pricing first item) | ||
| if (!state.selectedComponent) { | ||
| const pricingComponent = list.find( | ||
| (comp) => comp.category === "pricing", | ||
| ); | ||
| if (pricingComponent) { | ||
| await handleComponentChange(pricingComponent.id); | ||
| } | ||
| } | ||
|
|
||
| setIsLoading(false); | ||
| } | ||
| loadComponentList(); | ||
| }, []); |
There was a problem hiding this comment.
Fix missing dependencies in useEffect.
The effect reads state.selectedComponent and calls handleComponentChange, but the dependency array is empty. This violates React's exhaustive-deps rule and can cause stale closure bugs where outdated values are used.
🔎 Recommended fix
Wrap handleComponentChange in useCallback and include all dependencies:
+import { useState, useEffect, useCallback } from "react";
...
- const handleComponentChange = async (componentId: string) => {
+ const handleComponentChange = useCallback(async (componentId: string) => {
setIsLoadingComponent(true);
setLoadError(null);
...
- };
+ }, [setSelectedComponent]);
// Load lightweight component list on mount (no actual imports)
useEffect(() => {
async function loadComponentList() {
setIsLoading(true);
const list = await getComponentList();
setComponentList(list);
// Default selection (pricing first item)
if (!state.selectedComponent) {
const pricingComponent = list.find(
(comp) => comp.category === "pricing",
);
if (pricingComponent) {
await handleComponentChange(pricingComponent.id);
}
}
setIsLoading(false);
}
loadComponentList();
- }, []);
+ }, [state.selectedComponent, handleComponentChange]);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/playground/playground-header.tsx around lines 63 to 83, the
useEffect reads state.selectedComponent and calls handleComponentChange but has
an empty dependency array; wrap handleComponentChange in useCallback (including
the variables it uses) so it is stable, then update the useEffect dependency
array to include handleComponentChange and any other external values used inside
(e.g., state.selectedComponent, getComponentList or props/state setters as
appropriate) to satisfy exhaustive-deps and avoid stale closures.
| <Button | ||
| size="sm" | ||
| variant="ghost" | ||
| onClick={togglePreviewPanel} | ||
| onClick={() => togglePanel("both")} | ||
| style={{ boxShadow: "inset 0 2px 4px rgba(202, 199, 199, 0.41)" }} | ||
| className={cn( | ||
| "text-muted-foreground hover:!bg-accent", | ||
| showCodePanel && | ||
| showPreviewPanel && | ||
| "text-foreground bg-accent-foreground dark:bg-accent/50", | ||
| )} | ||
| > | ||
| <Link | ||
| className={cn( | ||
| "rotate-45", | ||
| showCodePanel && showPreviewPanel && "text-foreground", | ||
| )} | ||
| /> | ||
| </Button> |
There was a problem hiding this comment.
Add accessible label to the icon-only button.
The middle button contains only a rotated Link icon without any visible text or aria-label, making it inaccessible to screen reader users who won't understand its purpose.
🔎 Proposed fix
<Button
size="sm"
variant="ghost"
onClick={() => togglePanel("both")}
style={{ boxShadow: "inset 0 2px 4px rgba(202, 199, 199, 0.41)" }}
+ aria-label="Show both code and preview panels"
className={cn(
"text-muted-foreground hover:!bg-accent",
showCodePanel &&
showPreviewPanel &&
"text-foreground bg-accent-foreground dark:bg-accent/50",
)}
>
<Link
className={cn(
"rotate-45",
showCodePanel && showPreviewPanel && "text-foreground",
)}
/>
</Button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Button | |
| size="sm" | |
| variant="ghost" | |
| onClick={togglePreviewPanel} | |
| onClick={() => togglePanel("both")} | |
| style={{ boxShadow: "inset 0 2px 4px rgba(202, 199, 199, 0.41)" }} | |
| className={cn( | |
| "text-muted-foreground hover:!bg-accent", | |
| showCodePanel && | |
| showPreviewPanel && | |
| "text-foreground bg-accent-foreground dark:bg-accent/50", | |
| )} | |
| > | |
| <Link | |
| className={cn( | |
| "rotate-45", | |
| showCodePanel && showPreviewPanel && "text-foreground", | |
| )} | |
| /> | |
| </Button> | |
| <Button | |
| size="sm" | |
| variant="ghost" | |
| onClick={() => togglePanel("both")} | |
| style={{ boxShadow: "inset 0 2px 4px rgba(202, 199, 199, 0.41)" }} | |
| aria-label="Show both code and preview panels" | |
| className={cn( | |
| "text-muted-foreground hover:!bg-accent", | |
| showCodePanel && | |
| showPreviewPanel && | |
| "text-foreground bg-accent-foreground dark:bg-accent/50", | |
| )} | |
| > | |
| <Link | |
| className={cn( | |
| "rotate-45", | |
| showCodePanel && showPreviewPanel && "text-foreground", | |
| )} | |
| /> | |
| </Button> |
🤖 Prompt for AI Agents
In src/components/playground/playground.tsx around lines 130 to 148, the middle
Button is icon-only and missing an accessible label; add an explicit accessible
name by adding aria-label (e.g., aria-label="Toggle both panels") to the Button,
mark the decorative Link icon as aria-hidden="true", and optionally use
aria-pressed or update the label dynamically to reflect the toggled state so
screen reader users understand the action.
1 participant
Summary
Fixes a blank screen issue on the
/playgroundpage when using theCode&Previewbuttons and improves visual clarity in thePlaygroundHeader& also adds a default component on page load.Changes
Code&PreviewbuttonPlaygroundHeaderCode&PreviewScreenshots/Recordings (if UI)
Before
before.mp4
After
after.mp4
How to Test
Steps to validate locally:
Checklist
Summary by CodeRabbit
New Features
UI/Style
✏️ Tip: You can customize this high-level summary in your review settings.