Refine dashboard shell and simplify local dev setup#16
Conversation
- Move settings into an in-page section nav - Remove panel slide wrappers and simplify sidebar states - Smooth shared button and input styling
d76c315 to
e93d289
Compare
Greptile SummaryThis PR refines the dashboard shell by moving settings section navigation from a left sidebar into a horizontal tab bar inside the settings workspace itself, and applies a consistent visual polish pass (rounded corners, ring-based active states, removed
Confidence Score: 4/5Safe to merge — all changes are UI-layer restructuring and visual polish with no data or state logic altered. The settings nav relocation is wired correctly end-to-end, the showContextSidebar guard is sound, and the shared component updates (button, input) are purely cosmetic. The only loose ends are the orphaned SettingsModuleSidebar export and a missing aria-current on the new tab bar, both of which are style-level issues that don't affect runtime behaviour. dashboard-module-sidebar-admin.tsx still exports SettingsModuleSidebar with no remaining consumers; settings-workspace.tsx has the accessibility gap on the new tab buttons. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[DashboardSidebarChrome] -->|activeView === settings| B{showContextSidebar}
B -->|false| C[Hide divider + ModuleSidebar]
B -->|true| D[Show ModuleSidebar]
D --> E[ModuleSidebar]
E -->|settings view| F[return null]
E -->|other views| G[roadmap / changelog / feedback / etc.]
A --> H[AmendDashboardMainWorkspace]
H -->|activeView === settings| I[SettingsWorkspace]
I --> J[SettingsSectionNav]
J -->|onSectionChange| K[uiState.setActiveSettingsSection]
K -->|activeSection prop| J
Reviews (1): Last reviewed commit: "Refine dashboard shell and settings navi..." | Re-trigger Greptile |
| const SETTINGS_SECTIONS: Array<{ | ||
| icon: ReactElement; | ||
| id: SettingsSection; | ||
| label: string; | ||
| }> = [ | ||
| { id: "general", icon: <Settings />, label: "General" }, | ||
| { id: "services", icon: <GitPullRequestArrow />, label: "Services" }, | ||
| { id: "portal", icon: <Globe />, label: "Portal" }, | ||
| { id: "automation", icon: <DatabaseZap />, label: "Automation" }, | ||
| { id: "accounts", icon: <Users />, label: "Accounts" }, | ||
| ]; |
There was a problem hiding this comment.
JSX elements stored in a module-level constant
SETTINGS_SECTIONS initialises ReactElement objects (e.g. <Settings />) once at module load time. While Lucide icons carry no context and this works today, the pattern means the same element object is shared across every render; if any icon ever needs a className, size, or theme-derived prop, callers can't pass it without replacing the whole array. Storing icon components (the function/class references) is the more idiomatic and flexible approach.
Confidence this is an issue: 2/5
| const SETTINGS_SECTIONS: Array<{ | |
| icon: ReactElement; | |
| id: SettingsSection; | |
| label: string; | |
| }> = [ | |
| { id: "general", icon: <Settings />, label: "General" }, | |
| { id: "services", icon: <GitPullRequestArrow />, label: "Services" }, | |
| { id: "portal", icon: <Globe />, label: "Portal" }, | |
| { id: "automation", icon: <DatabaseZap />, label: "Automation" }, | |
| { id: "accounts", icon: <Users />, label: "Accounts" }, | |
| ]; | |
| const SETTINGS_SECTIONS: Array<{ | |
| icon: ComponentType; | |
| id: SettingsSection; | |
| label: string; | |
| }> = [ | |
| { id: "general", icon: Settings, label: "General" }, | |
| { id: "services", icon: GitPullRequestArrow, label: "Services" }, | |
| { id: "portal", icon: Globe, label: "Portal" }, | |
| { id: "automation", icon: DatabaseZap, label: "Automation" }, | |
| { id: "accounts", icon: Users, label: "Accounts" }, | |
| ]; |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| className={cn( | ||
| "inline-flex h-10 shrink-0 items-center gap-2 rounded-xl px-3 text-sm font-semibold transition-colors duration-150 ease-linear active:opacity-75 [&_svg]:size-3.5 [&_svg]:shrink-0", | ||
| activeSection === section.id | ||
| ? "bg-[#232327] text-foreground shadow-[inset_0_1px_0_rgb(255_255_255/0.05)] ring-1 ring-white/[0.06]" | ||
| : "text-muted-foreground hover:bg-foreground/[0.045] hover:text-foreground", | ||
| )} | ||
| onClick={() => onSectionChange(section.id)} |
There was a problem hiding this comment.
Missing
aria-current on active settings nav tab
Screen readers have no signal for which tab is currently selected. Adding aria-current="page" (or aria-selected with a role="tab") on the active button fixes this with a one-liner. This applies to all five tab buttons.
Confidence this is an issue: 4/5
| className={cn( | |
| "inline-flex h-10 shrink-0 items-center gap-2 rounded-xl px-3 text-sm font-semibold transition-colors duration-150 ease-linear active:opacity-75 [&_svg]:size-3.5 [&_svg]:shrink-0", | |
| activeSection === section.id | |
| ? "bg-[#232327] text-foreground shadow-[inset_0_1px_0_rgb(255_255_255/0.05)] ring-1 ring-white/[0.06]" | |
| : "text-muted-foreground hover:bg-foreground/[0.045] hover:text-foreground", | |
| )} | |
| onClick={() => onSectionChange(section.id)} | |
| aria-current={activeSection === section.id ? "page" : undefined} | |
| className={cn( | |
| "inline-flex h-10 shrink-0 items-center gap-2 rounded-xl px-3 text-sm font-semibold transition-colors duration-150 ease-linear active:opacity-75 [&_svg]:size-3.5 [&_svg]:shrink-0", | |
| activeSection === section.id | |
| ? "bg-[#232327] text-foreground shadow-[inset_0_1px_0_rgb(255_255_255/0.05)] ring-1 ring-white/[0.06]" | |
| : "text-muted-foreground hover:bg-foreground/[0.045] hover:text-foreground", | |
| )} | |
| onClick={() => onSectionChange(section.id)} |
Summary
amend/docs.amendportless layout.convex devsetup.Testing
bun run readinessbun run check-typesbun run build