Playground: Fix theme toggle UI when sidebar is collapsed #7853
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
WalkthroughReplaces ThemeToggle's Button-based UI with a SidebarMenu and ClientOnly-wrapped Skeletons for icons/label; inserts a SidebarSeparator before ThemeToggle; changes submenu chevron icon and rotation and adds guards to skip empty menus; reduces sidebar width constants from 19rem to 18rem. No public API changes. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant UI as ThemeToggle (SidebarMenu)
participant TM as Theme Manager
participant C as ClientOnly/Skeleton
U->>UI: Click theme menu item
UI->>TM: Toggle theme (light ⇄ dark)
TM-->>UI: Current theme
UI->>C: Hydrate and render icon/label
C-->>U: Show skeleton pre-hydration, icon/label post-hydration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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). (8)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
the
Playground
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7853 +/- ##
=======================================
Coverage 56.34% 56.34%
=======================================
Files 905 905
Lines 58834 58834
Branches 4150 4150
=======================================
Hits 33151 33151
Misses 25577 25577
Partials 106 106
🚀 New features to boost your workflow:
|
MananTank
force-pushed
the
08-13-playground_fix_theme_toggle_ui_when_sidebar_is_collapsed
branch
from
47d755c to
961e142
Compare
size-limit report 📦
|
![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
🧹 Nitpick comments (2)
apps/playground-web/src/components/ThemeToggle.tsx (2)
34-36: Optional: Clarify label when theme is set to "system"If you want the label to match the icon/effective mode, show resolvedTheme or annotate system with the effective value.
Option A (show effective mode):
- <ClientOnly ssr={<Skeleton className="w-16 h-4" />}> - <span>{theme}</span> - </ClientOnly> + <ClientOnly ssr={<Skeleton className="w-16 h-4" />}> + <span>{(resolvedTheme ?? theme) ?? "system"}</span> + </ClientOnly>Option B (preserve "system" but add effective mode):
- <ClientOnly ssr={<Skeleton className="w-16 h-4" />}> - <span>{theme}</span> - </ClientOnly> + <ClientOnly ssr={<Skeleton className="w-16 h-4" />}> + <span> + {theme === "system" ? `system (${resolvedTheme ?? "light"})` : theme} + </span> + </ClientOnly>
20-21: Nit: Add a title for better UX in collapsed sidebarA title attribute helps show a tooltip in collapsed mode and improves accessibility.
- <SidebarMenuButton + <SidebarMenuButton aria-label="Toggle theme" className="w-full text-muted-foreground hover:text-foreground px-2 py-1.5 text-left justify-start gap-2 capitalize h-auto font-normal" + title="Toggle theme"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/playground-web/src/components/ThemeToggle.tsx(1 hunks)apps/playground-web/src/components/blocks/full-width-sidebar-layout.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Files:
apps/playground-web/src/components/blocks/full-width-sidebar-layout.tsxapps/playground-web/src/components/ThemeToggle.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
apps/playground-web/src/components/blocks/full-width-sidebar-layout.tsxapps/playground-web/src/components/ThemeToggle.tsx
apps/{dashboard,playground-web}/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
apps/{dashboard,playground-web}/**/*.{ts,tsx}: Import UI primitives from@/components/ui/*(Button, Input, Select, Tabs, Card, Sidebar, Badge, Separator) in dashboard and playground apps
UseNavLinkfor internal navigation with automatic active states in dashboard and playground apps
Use Tailwind CSS only – no inline styles or CSS modules
Usecn()from@/lib/utilsfor conditional class logic
Use design system tokens (e.g.,bg-card,border-border,text-muted-foreground)
Server Components (Node edge): Start files withimport "server-only";
Client Components (browser): Begin files with'use client';
Always callgetAuthToken()to retrieve JWT from cookies on server side
UseAuthorization: Bearerheader – never embed tokens in URLs
Return typed results (e.g.,Project[],User[]) – avoidany
Wrap client-side data fetching calls in React Query (@tanstack/react-query)
Use descriptive, stablequeryKeysfor React Query cache hits
ConfigurestaleTime/cacheTimein React Query based on freshness (default ≥ 60s)
Keep tokens secret via internal API routes or server actions
Never importposthog-jsin server components
Files:
apps/playground-web/src/components/blocks/full-width-sidebar-layout.tsxapps/playground-web/src/components/ThemeToggle.tsx
🧠 Learnings (1)
📚 Learning: 2025-07-18T19:20:32.530Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*.{tsx,jsx} : Layouts should reuse `SidebarLayout` / `FullWidthSidebarLayout` (`@/components/blocks/SidebarLayout`).
Applied to files:
apps/playground-web/src/components/blocks/full-width-sidebar-layout.tsx
⏰ 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). (8)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Unit Tests
- GitHub Check: Build Packages
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
apps/playground-web/src/components/blocks/full-width-sidebar-layout.tsx (2)
129-129: LGTM: Separator improves visual groupingAdding SidebarSeparator between footer links and ThemeToggle aligns with the new SidebarMenu-based toggle and improves scannability.
123-131: FullWidthSidebarLayout always receivesfooterSidebarLinksin playground-webWe verified that every usage of
FullWidthSidebarLayoutin apps/playground-web (notably inapps/playground-web/src/app/AppSidebar.tsx) passes a definedfooterSidebarLinksarray, so under current conditions theThemeTogglealready renders.If you’d like to future-proof the component against omitted footer links, you can refactor as follows to keep the separator conditional but always show the toggle:
- {footerSidebarLinks && ( - <SidebarFooter className="pb-3 pt-0"> - <RenderSidebarMenu - links={footerSidebarLinks} - fullPath={props.fullPath} - /> - <SidebarSeparator /> - <ThemeToggle /> - </SidebarFooter> - )} + <SidebarFooter className="pb-3 pt-0"> + {footerSidebarLinks && ( + <> + <RenderSidebarMenu + links={footerSidebarLinks} + fullPath={props.fullPath} + /> + <SidebarSeparator /> + </> + )} + <ThemeToggle /> + </SidebarFooter>apps/playground-web/src/components/ThemeToggle.tsx (2)
6-10: LGTM: Correctly moved to SidebarMenu primitivesImports now use the sidebar primitives instead of Button, consistent with the new sidebar-based UI.
17-33: Use resolvedTheme for icon rendering and toggle logic; add type="button"When theme is "system", the icon currently shows MoonIcon and toggle always flips to "dark", which can be surprising. Using resolvedTheme keeps the icon and toggle behavior consistent with the effective theme. Adding type="button" avoids accidental form submissions if this is ever placed inside a form.
[seduce_essential_refactor -> suggest_essential_refactor]
Apply:export function ThemeToggle() { - const { setTheme, theme } = useTheme(); + const { setTheme, theme, resolvedTheme } = useTheme(); return ( <SidebarMenu> <SidebarMenuItem> <SidebarMenuButton aria-label="Toggle theme" className="w-full text-muted-foreground hover:text-foreground px-2 py-1.5 text-left justify-start gap-2 capitalize h-auto font-normal" - onClick={() => { - setTheme(theme === "dark" ? "light" : "dark"); - }} + type="button" + onClick={() => { + setTheme((resolvedTheme ?? theme) === "dark" ? "light" : "dark"); + }} > - <ClientOnly ssr={<Skeleton className="size-4" />}> - {theme === "light" ? ( + <ClientOnly ssr={<Skeleton className="size-4" />}> + {(resolvedTheme ?? theme) === "light" ? ( <SunIcon className="size-4" /> ) : ( <MoonIcon className="size-4" /> )} </ClientOnly>
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
apps/playground-web/src/components/blocks/full-width-sidebar-layout.tsx (1)
123-132: ThemeToggle is unintentionally hidden when footerSidebarLinks is undefinedThemeToggle (and the new SidebarSeparator) are rendered only if footerSidebarLinks is present. This regresses UX by removing the theme switcher entirely on pages without footer links. Render the footer and ThemeToggle unconditionally; only guard the footer menu + separator.
Apply this diff:
- {footerSidebarLinks && ( - <SidebarFooter className="pb-3 pt-0"> - <RenderSidebarMenu - links={footerSidebarLinks} - fullPath={props.fullPath} - /> - <SidebarSeparator /> - <ThemeToggle /> - </SidebarFooter> - )} + <SidebarFooter className="pb-3 pt-0"> + {footerSidebarLinks && ( + <> + <RenderSidebarMenu + links={footerSidebarLinks} + fullPath={props.fullPath} + /> + <SidebarSeparator /> + </> + )} + <ThemeToggle /> + </SidebarFooter>
🧹 Nitpick comments (3)
apps/playground-web/src/components/blocks/full-width-sidebar-layout.tsx (3)
311-314: Match icon size and color inheritance for consistencyThe chevron should:
- Use the same size as other icons here (size-4).
- Inherit color from the parent button (currentColor) so hover/active styles apply consistently. Hard-coding text-foreground prevents this.
Apply this diff:
- <ChevronRightIcon - data-chevron - className="transition-transform ml-auto text-foreground" - /> + <ChevronRightIcon + data-chevron + className="ml-auto size-4 transition-transform" + />
292-297: Avoid mixing controlled and uncontrolled Collapsible propsYou pass both open (controlled) and defaultOpen (uncontrolled) with the same value. This is redundant and can confuse readers. Keep it controlled and remove defaultOpen.
Apply this diff:
<Collapsible className="group/collapsible [&[data-state=open]>button>svg[data-chevron]]:rotate-90" - defaultOpen={open} open={open} onOpenChange={setOpen} >
283-286: Remove explanatory comments that restate the codeThe “Add null check for links” comments restate what the code does and add noise. The guard clauses are self-explanatory.
Apply these diffs:
- // Add null check for links if (!props.links || props.links.length === 0) { return null; }- // Add null check for links if (!props.links || props.links.length === 0) { return null; }Also applies to: 382-385
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/playground-web/src/components/ThemeToggle.tsx(1 hunks)apps/playground-web/src/components/blocks/full-width-sidebar-layout.tsx(4 hunks)apps/playground-web/src/components/ui/sidebar.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/playground-web/src/components/ui/sidebar.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/playground-web/src/components/ThemeToggle.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
Files:
apps/playground-web/src/components/blocks/full-width-sidebar-layout.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
apps/playground-web/src/components/blocks/full-width-sidebar-layout.tsx
apps/{dashboard,playground-web}/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
apps/{dashboard,playground-web}/**/*.{ts,tsx}: Import UI primitives from@/components/ui/*(Button, Input, Select, Tabs, Card, Sidebar, Badge, Separator) in dashboard and playground apps
UseNavLinkfor internal navigation with automatic active states in dashboard and playground apps
Use Tailwind CSS only – no inline styles or CSS modules
Usecn()from@/lib/utilsfor conditional class logic
Use design system tokens (e.g.,bg-card,border-border,text-muted-foreground)
Server Components (Node edge): Start files withimport "server-only";
Client Components (browser): Begin files with'use client';
Always callgetAuthToken()to retrieve JWT from cookies on server side
UseAuthorization: Bearerheader – never embed tokens in URLs
Return typed results (e.g.,Project[],User[]) – avoidany
Wrap client-side data fetching calls in React Query (@tanstack/react-query)
Use descriptive, stablequeryKeysfor React Query cache hits
ConfigurestaleTime/cacheTimein React Query based on freshness (default ≥ 60s)
Keep tokens secret via internal API routes or server actions
Never importposthog-jsin server components
Files:
apps/playground-web/src/components/blocks/full-width-sidebar-layout.tsx
🧠 Learnings (5)
📚 Learning: 2025-07-18T19:20:32.530Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*.{tsx,jsx} : Layouts should reuse `SidebarLayout` / `FullWidthSidebarLayout` (`@/components/blocks/SidebarLayout`).
Applied to files:
apps/playground-web/src/components/blocks/full-width-sidebar-layout.tsx
📚 Learning: 2025-07-31T16:17:42.753Z
Learnt from: MananTank
PR: thirdweb-dev/js#7768
File: apps/playground-web/src/app/navLinks.ts:1-1
Timestamp: 2025-07-31T16:17:42.753Z
Learning: Configuration files that import and reference React components (like icon components from lucide-react) need the "use client" directive, even if they primarily export static data, because the referenced components need to be executed in a client context when used by other client components.
Applied to files:
apps/playground-web/src/components/blocks/full-width-sidebar-layout.tsx
📚 Learning: 2025-07-07T21:21:47.488Z
Learnt from: saminacodes
PR: thirdweb-dev/js#7543
File: apps/portal/src/app/pay/page.mdx:4-4
Timestamp: 2025-07-07T21:21:47.488Z
Learning: In the thirdweb-dev/js repository, lucide-react icons must be imported with the "Icon" suffix (e.g., ExternalLinkIcon, RocketIcon) as required by the new linting rule, contrary to the typical lucide-react convention of importing without the suffix.
Applied to files:
apps/playground-web/src/components/blocks/full-width-sidebar-layout.tsx
📚 Learning: 2025-07-18T19:20:32.530Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*client.tsx : Interactive UI that relies on hooks (`useState`, `useEffect`, React Query, wallet hooks).
Applied to files:
apps/playground-web/src/components/blocks/full-width-sidebar-layout.tsx
📚 Learning: 2025-07-18T19:20:32.530Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-18T19:20:32.530Z
Learning: Applies to dashboard/**/*.{tsx,jsx} : Icons come from `lucide-react` or the project-specific `…/icons` exports – never embed raw SVG.
Applied to files:
apps/playground-web/src/components/blocks/full-width-sidebar-layout.tsx
🧬 Code Graph Analysis (1)
apps/playground-web/src/components/blocks/full-width-sidebar-layout.tsx (1)
apps/playground-web/src/components/ui/sidebar.tsx (1)
SidebarSeparator(771-771)
⏰ 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: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Size
- GitHub Check: Unit Tests
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apps/playground-web/src/components/blocks/full-width-sidebar-layout.tsx (2)
3-3: ChevronRightIcon import is correct and consistent with repo linting rulesUsing the Icon-suffixed lucide import matches the repository convention noted in our learnings. No issues.
293-297: Chevron rotation behavior matches new right-pointing iconSwitching to rotate-90 on open for a right-pointing chevron is semantically correct and improves visual clarity. Looks good.
Merge activity
|
<!--
## title your PR with this format: "[SDK/Dashboard/Portal] Feature/Fix: Concise title for the changes"
If you did not copy the branch name from Linear, paste the issue tag here (format is TEAM-0000):
## Notes for the reviewer
Anything important to call out? Be sure to also clarify these in your comments.
## How to test
Unit tests, playground, etc.
-->
<!-- start pr-codex -->
---
## PR-Codex overview
This PR focuses on refining the sidebar component in the application by adjusting styles, modifying the theme toggle functionality, and enhancing the UI elements for better user experience.
### Detailed summary
- Updated `SIDEBAR_WIDTH` and `SIDEBAR_WIDTH_MOBILE` from `"19rem"` to `"18rem"`.
- Removed `ChevronDownIcon` and replaced it with `ChevronRightIcon` in `RenderSidebarSubmenu`.
- Added `<SidebarSeparator />` in `FullWidthSidebarLayoutInner`.
- Replaced `Button` with `SidebarMenu`, `SidebarMenuButton`, and `SidebarMenuItem` in `ThemeToggle`.
> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`
<!-- end pr-codex -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
- New Features
- Theme toggle now appears as a sidebar menu item with improved loading placeholders for icon and label.
- UI/UX
- Sidebar width reduced from 19rem to 18rem for a more compact layout.
- Chevron indicators updated: right-chevron rotates 90° when open for clearer state.
- Added a visual separator in the sidebar footer for better grouping.
- Bug Fixes
- Prevented rendering of empty sidebar menus/submenus.
- Refactor
- Streamlined imports and removed unused variants; maintained existing toggle behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
graphite-app
bot
force-pushed
the
08-13-playground_fix_theme_toggle_ui_when_sidebar_is_collapsed
branch
from
961e142 to
857d4df
Compare
graphite-app
bot
deleted the
08-13-playground_fix_theme_toggle_ui_when_sidebar_is_collapsed
branch
PR-Codex overview
This PR focuses on refining the layout and functionality of the sidebar and theme toggle components in the application. It includes adjustments to dimensions, class names, and component structure for improved UI/UX.
Detailed summary
SIDEBAR_WIDTHandSIDEBAR_WIDTH_MOBILEfrom"19rem"to"18rem"insidebar.tsx.ChevronDownIconand replaced it withChevronRightIconinRenderSidebarSubmenu.rotate-180torotate-90.<SidebarSeparator />inFullWidthSidebarLayoutInner.ThemeToggleto useSidebarMenu,SidebarMenuItem, andSidebarMenuButton.Summary by CodeRabbit
New Features
UI/UX
Bug Fixes
Refactor