chore: fix dashboard logo on collapse & make name of org text bigger #1019
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdjusted several dashboard UI styles and navbar/Logo behavior: increased mobile org name font, disabled initial sidebar width animation and added will-change styling, made Logo accept/use a viewBoxDimensions default, removed redundant flex-grow classes, added aside w-full, and added scrollbar-gutter fallback in CSS. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DesktopNav
participant Logo
User->>DesktopNav: Open dashboard / toggle sidebar
DesktopNav->>DesktopNav: compute sidebarCollapsed
alt Collapsed
DesktopNav->>Logo: render(viewBoxDimensions="0 0 40 40")
else Expanded
DesktopNav->>Logo: render(viewBoxDimensions="0 0 120 40")
end
Note right of Logo #ddebf7: Logo uses provided prop or its default\n(viewBoxDimensions = "0 0 120 40")
Logo-->>DesktopNav: SVG element with viewBox applied
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
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
🧹 Nitpick comments (3)
apps/web/app/(org)/dashboard/_components/MobileTab.tsx (1)
95-95: Text size bump looks fine; consider line-height and width tweak to avoid truncation jitter.At text-sm inside a 16px-tall bar, add a tighter leading and slightly larger max-w to reduce ellipsis frequency on common org names.
Apply:
- <p className="text-sm mr-2 text-gray-12 truncate w-fit max-w-[90px]"> + <p className="text-sm leading-tight mr-2 text-gray-12 truncate w-fit max-w-[110px]">packages/ui/src/components/icons/Logo.tsx (1)
7-9: Defaulted viewBox prop + direct binding — LGTM.The default
"0 0 120 40"and directviewBox={viewBoxDimensions}simplify behavior and align with Desktop’s conditional usage.If only two viewBoxes are intended, do you want a constrained helper to prevent typos?
+export const VIEWBOX = { + full: "0 0 120 40", + compact: "0 0 40 40", +} as const;Then use
typeof VIEWBOX[keyof typeof VIEWBOX]for the prop and passVIEWBOX.compact/fullat call sites.Also applies to: 21-23
apps/web/app/(org)/dashboard/_components/Navbar/Desktop.tsx (1)
56-59: Collapsed state still renders a 120px-wide SVG; set width conditionally to avoid letterboxing/overflow.With
viewBox="0 0 40 40"andpreserveAspectRatio="xMidYMid meet", keepingw-[120px]causes horizontal padding/overflow when the sidebar is 70px. Bind width to the state.Apply:
- <Logo - hideLogoName={sidebarCollapsed} - viewBoxDimensions={sidebarCollapsed ? "0 0 40 40" : "0 0 120 40"} - className="mx-auto w-[120px] h-[40px]" - /> + <Logo + hideLogoName={sidebarCollapsed} + viewBoxDimensions={sidebarCollapsed ? "0 0 40 40" : "0 0 120 40"} + className={clsx( + "mx-auto h-[40px]", + sidebarCollapsed ? "w-[40px]" : "w-[120px]", + )} + />Please verify in collapsed mode (70px width) that the icon is fully visible without horizontal scroll/clip.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/app/(org)/dashboard/_components/MobileTab.tsx(1 hunks)apps/web/app/(org)/dashboard/_components/Navbar/Desktop.tsx(1 hunks)packages/ui/src/components/icons/Logo.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for client-side server state and data fetching in the web app
Mutations should call Server Actions and perform precise cache updates with setQueryData/setQueriesData, avoiding broad invalidations
Prefer Server Components for initial data and pass initialData to client components for React Query hydration
Files:
apps/web/app/(org)/dashboard/_components/Navbar/Desktop.tsxapps/web/app/(org)/dashboard/_components/MobileTab.tsx
{apps/web,packages/ui}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps/web,packages/ui}/**/*.{ts,tsx}: Use Tailwind CSS exclusively for styling in the web app and shared React UI components
Component naming: React components in PascalCase; hooks in camelCase starting with 'use'
Files:
apps/web/app/(org)/dashboard/_components/Navbar/Desktop.tsxpackages/ui/src/components/icons/Logo.tsxapps/web/app/(org)/dashboard/_components/MobileTab.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; prefer shared types from packages
**/*.{ts,tsx}: Use Biome to format/lint TypeScript with a 2-space indent
TypeScript file names should be kebab-case (e.g., user-menu.tsx)
Files:
apps/web/app/(org)/dashboard/_components/Navbar/Desktop.tsxpackages/ui/src/components/icons/Logo.tsxapps/web/app/(org)/dashboard/_components/MobileTab.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
React/Solid components should be named using PascalCase
Files:
apps/web/app/(org)/dashboard/_components/Navbar/Desktop.tsxpackages/ui/src/components/icons/Logo.tsxapps/web/app/(org)/dashboard/_components/MobileTab.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). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
Summary by CodeRabbit
Style
Refactor
Chores