feat(ui): Normalize typography hierarchy and improve sidebar active indicator#548
Conversation
…ndicator - EcosystemSection H2: font-bold tracking-tight → font-extrabold tracking-[-0.02em] - CapabilitiesSection H3: text-base → text-lg for better visual hierarchy - Sidebar: exact match for /dashboard route to prevent false-positive active state - Sidebar: hide left-border pseudo-element when collapsed to prevent overlap - Sidebar: add transition-colors duration-150 for smooth active state animation - Sidebar: scale-110 on active icon for visual weight in collapsed mode
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThree components were modified: Sidebar.tsx now uses specialized active state detection with exact match for dashboard and conditional transition styling; CapabilitiesSection.tsx increased heading font size from text-base to text-lg; EcosystemSection.tsx updated h2 styling from font-bold to font-extrabold with adjusted tracking. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.16.4)apps/web/src/components/dashboard/Sidebar.tsx┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m apps/web/src/components/landing/CapabilitiesSection.tsx┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m apps/web/src/components/landing/EcosystemSection.tsx┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m 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 |
Project Scorecard |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/src/components/dashboard/Sidebar.tsx (2)
73-75: Preferclsxfor this active-class composition.Lines 73-75 are getting difficult to maintain due to nested template composition. This is a good spot to switch to
clsx.Refactor sketch
+import clsx from 'clsx'; ... -const activeCls = isActive - ? `bg-brand/15 text-brand-light hover:bg-brand/20 transition-colors duration-150${collapsed ? '' : ' before:absolute before:left-0 before:top-1.5 before:bottom-1.5 before:w-0.5 before:rounded-full before:bg-brand'}` - : 'text-text-secondary hover:bg-surface-2 hover:text-text-primary transition-colors duration-150'; +const activeCls = clsx( + 'transition-colors duration-150', + isActive + ? 'bg-brand/15 text-brand-light hover:bg-brand/20' + : 'text-text-secondary hover:bg-surface-2 hover:text-text-primary', + isActive && + !collapsed && + 'before:absolute before:left-0 before:top-1.5 before:bottom-1.5 before:w-0.5 before:rounded-full before:bg-brand' +);As per coding guidelines,
apps/web/src/components/**/*.{ts,tsx}: Use Radix UI + Tailwind CSS + shadcn/ui component pattern with class-variance-authority and clsx for styling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/dashboard/Sidebar.tsx` around lines 73 - 75, The inline template for activeCls is brittle; refactor it to use clsx: import clsx from 'clsx' and replace the template string assigned to activeCls with a clsx call that conditionally includes the base classes, the active state classes (bg-brand/15 text-brand-light hover:bg-brand/20 transition-colors duration-150), the hover/secondary/default classes when not active, and the before:... utility classes only when isActive && !collapsed; reference the activeCls variable, and the isActive and collapsed booleans to build the conditional class list so the composition is clearer and maintainable.
88-88: Smooth the active icon scale animation.Line 88 applies
scale-110but only transitions color; adding transform transition will avoid a snap effect.Optional polish
-className={`h-[18px] w-[18px] flex-shrink-0 transition-colors ${isActive ? 'text-brand-light scale-110' : ''}`} +className={`h-[18px] w-[18px] flex-shrink-0 transition-colors transition-transform duration-150 ${isActive ? 'text-brand-light scale-110' : ''}`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/dashboard/Sidebar.tsx` at line 88, The active icon currently only transitions colors, causing the `scale-110` to snap; update the className used for the icon (the JSX that builds the icon element in Sidebar.tsx where className includes `transition-colors` and `${isActive ? 'text-brand-light scale-110' : ''}`) to also transition transforms (e.g., add `transition-transform` or replace with `transition-colors transition-transform` / `transition-all` and optionally set a duration like `duration-150`) so the scale change animates smoothly when `isActive` toggles.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/dashboard/Sidebar.tsx`:
- Around line 69-72: The current isActive check in Sidebar.tsx uses
pathname.startsWith(item.href) which can falsely match prefixes (e.g., /admin
matching /admin-something); update the logic around the isActive calculation to
perform a segment-boundary-safe check: consider a route active only if pathname
=== item.href or pathname starts with item.href followed by a slash (i.e.,
item.href + '/'), so use that boundary-aware condition when computing isActive
for the nav item (refer to the isActive variable, item.href, and pathname).
---
Nitpick comments:
In `@apps/web/src/components/dashboard/Sidebar.tsx`:
- Around line 73-75: The inline template for activeCls is brittle; refactor it
to use clsx: import clsx from 'clsx' and replace the template string assigned to
activeCls with a clsx call that conditionally includes the base classes, the
active state classes (bg-brand/15 text-brand-light hover:bg-brand/20
transition-colors duration-150), the hover/secondary/default classes when not
active, and the before:... utility classes only when isActive && !collapsed;
reference the activeCls variable, and the isActive and collapsed booleans to
build the conditional class list so the composition is clearer and maintainable.
- Line 88: The active icon currently only transitions colors, causing the
`scale-110` to snap; update the className used for the icon (the JSX that builds
the icon element in Sidebar.tsx where className includes `transition-colors` and
`${isActive ? 'text-brand-light scale-110' : ''}`) to also transition transforms
(e.g., add `transition-transform` or replace with `transition-colors
transition-transform` / `transition-all` and optionally set a duration like
`duration-150`) so the scale change animates smoothly when `isActive` toggles.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 27056ed0-01bd-47b8-8daf-28da6fbdc00b
📒 Files selected for processing (3)
apps/web/src/components/dashboard/Sidebar.tsxapps/web/src/components/landing/CapabilitiesSection.tsxapps/web/src/components/landing/EcosystemSection.tsx
| const isActive = | ||
| item.href === '/dashboard' | ||
| ? pathname === item.href | ||
| : pathname.startsWith(item.href); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nav_file="$(fd -a 'navigation\.(ts|tsx)$' apps/web/src/components/dashboard | head -n1)"
echo "Navigation file: $nav_file"
# Inspect dashboard href declarations
rg -nP "href\s*:\s*['\"]/dashboard[^'\"]*['\"]" "$nav_file"
# Detect potential prefix-collision pairs in declared hrefs
python - "$nav_file" <<'PY'
import re, sys, pathlib
p = pathlib.Path(sys.argv[1])
text = p.read_text(encoding="utf-8")
hrefs = sorted(set(re.findall(r"href\s*:\s*['\"]([^'\"]+)['\"]", text)))
print("\nFound hrefs:")
for h in hrefs:
print(" -", h)
print("\nPotential prefix collisions (a is prefix of b without '/' boundary):")
found = False
for a in hrefs:
for b in hrefs:
if a == b:
continue
if b.startswith(a) and not b.startswith(a + "/"):
found = True
print(f" - {a} vs {b}")
if not found:
print(" - none")
PYRepository: Forge-Space/siza
Length of output: 487
🏁 Script executed:
#!/bin/bash
# Get the file size first
wc -l apps/web/src/components/dashboard/Sidebar.tsx
# Show the relevant section with more context
sed -n '60,80p' apps/web/src/components/dashboard/Sidebar.tsx
# Check if there are any nested sub-routes beyond top-level in the router/app config
fd -e 'tsx' -e 'ts' apps/web/src/app | head -20Repository: Forge-Space/siza
Length of output: 1534
🏁 Script executed:
#!/bin/bash
# Check app directory structure for nested routes
fd . apps/web/src/app --type f -e tsx -e ts | head -30
# Also check if there are any dynamic route segments or nested structure
find apps/web/src/app -type d 2>/dev/null | head -20Repository: Forge-Space/siza
Length of output: 2345
Refactor active route matching to segment-boundary safe approach.
No prefix collisions exist in the current navigation structure, but the route matching should be hardened to prevent false positives in future additions. Line 72 currently uses pathname.startsWith(item.href), which would incorrectly activate a /admin item if a route like /admin-something were added. Use a segment-boundary check instead.
Proposed fix
const isActive =
item.href === '/dashboard'
? pathname === item.href
- : pathname.startsWith(item.href);
+ : pathname === item.href || pathname.startsWith(`${item.href}/`);📝 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.
| const isActive = | |
| item.href === '/dashboard' | |
| ? pathname === item.href | |
| : pathname.startsWith(item.href); | |
| const isActive = | |
| item.href === '/dashboard' | |
| ? pathname === item.href | |
| : pathname === item.href || pathname.startsWith(`${item.href}/`); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/dashboard/Sidebar.tsx` around lines 69 - 72, The
current isActive check in Sidebar.tsx uses pathname.startsWith(item.href) which
can falsely match prefixes (e.g., /admin matching /admin-something); update the
logic around the isActive calculation to perform a segment-boundary-safe check:
consider a route active only if pathname === item.href or pathname starts with
item.href followed by a slash (i.e., item.href + '/'), so use that
boundary-aware condition when computing isActive for the nav item (refer to the
isActive variable, item.href, and pathname).
1 participant
Summary
EcosystemSectionH2:font-bold tracking-tight→font-extrabold tracking-[-0.02em](matchesCapabilitiesSectionandCTASection)CapabilitiesSectionH3:text-base→text-lgfor better visual weight on capability cards/dashboardroute (previously matched all sub-routes like/dashboard/settings)transition-colors duration-150to both active/inactive states for smooth animationscale-110to active icon for visual weight in collapsed modeTasks covered
task_siza_typography_hierarchytask_siza_sidebar_active_indicatorSummary by CodeRabbit
Release Notes