feat(frontend): add Sidebar component (T-128)#62
Merged
Conversation
Closes #43. - src/features/onboarding/components/Sidebar.tsx — collapsible left nav (240px <-> 64px) with 5 nav links, Lucide icons, i18n labels, Tooltip on hover when collapsed, 4 placeholder bottom pills. - Roving tabindex + ArrowUp/ArrowDown keyboard nav with wrap-around. - aria-label on all nav buttons so screen readers announce when collapsed (icon is aria-hidden), aria-current="page" on active. - New i18n keys sidebar.landmark / sidebar.navigation in en + fr. - 13 vitest cases / pnpm exec tsc -b / oxlint / oxfmt / vitest 80/80 all clean.
📝 WalkthroughWalkthroughThis PR introduces a new collapsible onboarding ChangesOnboarding Sidebar Component
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/features/onboarding/components/Sidebar.tsx (1)
32-44: ⚡ Quick winRemove duplicated
route/labelKeysource of truth.Line 35 duplicates the same domain already represented by
route, which can drift later. Prefer deriving labels directly fromroute.♻️ Proposed simplification
interface NavItem { route: SidebarRoute; icon: ComponentType<{ className?: string }>; - labelKey: "tasks" | "roadmap" | "insights" | "ideation" | "settings"; } const NAV_ITEMS: readonly NavItem[] = [ - { route: "tasks", icon: CheckSquare, labelKey: "tasks" }, - { route: "roadmap", icon: Map, labelKey: "roadmap" }, - { route: "insights", icon: MessageSquare, labelKey: "insights" }, - { route: "ideation", icon: Lightbulb, labelKey: "ideation" }, - { route: "settings", icon: Settings, labelKey: "settings" }, + { route: "tasks", icon: CheckSquare }, + { route: "roadmap", icon: Map }, + { route: "insights", icon: MessageSquare }, + { route: "ideation", icon: Lightbulb }, + { route: "settings", icon: Settings }, ] as const; -const label = translate(item.labelKey); +const label = translate(item.route);Also applies to: 100-101
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/onboarding/components/Sidebar.tsx` around lines 32 - 44, Remove the duplicated labelKey property: update the NavItem type to omit labelKey and change NAV_ITEMS to only include route and icon (keep the same route strings like "tasks", "roadmap", etc.), then change any usage of item.labelKey in the Sidebar rendering to derive the label key from item.route (e.g., use item.route as the translation key or a small helper like getLabelKeyFromRoute(route)), and apply the same removal/fix for the other duplicate array in this file that also defines route+labelKey; ensure all references now use the derived label key instead of the removed property.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/features/onboarding/components/Sidebar.tsx`:
- Around line 32-44: Remove the duplicated labelKey property: update the NavItem
type to omit labelKey and change NAV_ITEMS to only include route and icon (keep
the same route strings like "tasks", "roadmap", etc.), then change any usage of
item.labelKey in the Sidebar rendering to derive the label key from item.route
(e.g., use item.route as the translation key or a small helper like
getLabelKeyFromRoute(route)), and apply the same removal/fix for the other
duplicate array in this file that also defines route+labelKey; ensure all
references now use the derived label key instead of the removed property.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d5b659d1-3a1e-4fd3-8fa2-0b6fa3a0a6a6
📒 Files selected for processing (5)
CHANGELOG.mdsrc/features/onboarding/components/Sidebar.test.tsxsrc/features/onboarding/components/Sidebar.tsxsrc/shared/i18n/locales/en.jsonsrc/shared/i18n/locales/fr.json
Contributor
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #43.
Summary
src/features/onboarding/components/Sidebar.tsx+Sidebar.test.tsx— collapsible left-navSidebar(T-128, F-068, blocks T-130App.tsxmount).tasks/roadmap/insights/ideation/settings) with Lucide icons + i18n labels, Tooltip wrap when collapsed, 4 inert bottom indicator pills (placeholders for S5/S6).w-60 ↔ w-16viatransition-[width] duration-200 ease-in-out.Why
Sprint 1 deliverable per DESIGN S2 / PRD F-068. Visible app shell that subsequent sprints (T-130 mount, S5 indicators, S6 telemetry) depend on.
Changes
src/features/onboarding/components/Sidebar.tsx— props{ expanded, onToggle, activeRoute, onNavigate }, topPanelLeft/PanelLeftClosetoggle, roving tabindex (activeRouteitem istabIndex=0, others-1),ArrowDown/ArrowUpkeyboard nav with modulo wrap-around.src/features/onboarding/components/Sidebar.test.tsx— 13 vitest cases across 3 describe blocks (rendering / interactions / accessibility). Module-levelafterEach(cleanup)to prevent landmark bleed across consecutive renders.src/shared/i18n/locales/{en,fr}.json— new keyssidebar.landmark(Sidebar/Barre latérale) andsidebar.navigation(Main navigation/Navigation principale) for<aside aria-label>+<nav aria-label>.CHANGELOG.md—[Unreleased] / Addedentry.Accessibility
<aside aria-label>+<nav aria-label>translated landmarks.aria-current="page"on active nav item.aria-labelon every nav button so screen readers announce labels even when collapsed (icon isaria-hidden, no visible text in collapsed mode).aria-expandedon toggle button.focus-visible:ring-2inherited fromButtonprimitive.Testing
pnpm exec tsc -b— clean.pnpm exec oxlint src/features/onboarding— 0 warnings / 0 errors after fixingsort-imports(multi → single ordering with case-sensitive alphabetical),array-type(Array<X>→(X)[]),max-statements(split single describe with 13itinto 3 describes ≤6 each),id-length(useTtranslator aliased totranslate), andno-magic-numbers(extractedFOCUS_STEP_NEXT/FOCUS_STEP_PREV/TAB_INDEX_FOCUSED/TAB_INDEX_INERT).pnpm exec oxfmt --check— clean.pnpm exec vitest run— 80/80 pass (13 new for Sidebar).Acceptance criteria (from #43)
pnpm exec tsc -bno errorspnpm exec vitest run features/onboarding/SidebarpassesArrowUp/ArrowDownnavigates between itemsTest plan
App.tsxSummary by CodeRabbit