fix(sidebar): collapsible Workbench/Workspace nav sections#491
Open
lazmo88 wants to merge 2 commits intoThe-Vibe-Company:mainfrom
Open
fix(sidebar): collapsible Workbench/Workspace nav sections#491lazmo88 wants to merge 2 commits intoThe-Vibe-Company:mainfrom
lazmo88 wants to merge 2 commits intoThe-Vibe-Company:mainfrom
Conversation
added 2 commits
March 7, 2026 19:42
Repos initialized with `git init` but no commits have an "unborn" branch where HEAD is a symbolic ref that doesn't point to any commit. This caused session creation to fail with "Branch X does not exist" because: 1. `getRepoInfo()` used `rev-parse --abbrev-ref HEAD` which fails on unborn branches, falling back to "HEAD" — so the branch name check always triggered a checkout attempt. 2. `checkoutOrCreateBranch()` ran `git checkout <branch>` which fails on empty repos since no branches technically exist yet. Fix: - `getRepoInfo()`: Fall back to `symbolic-ref --short HEAD` which correctly returns the unborn branch name (e.g. "master" or "main"). - `checkoutOrCreateBranch()`: Detect empty repos via `rev-parse HEAD`. If already on the requested unborn branch, return success. If a different branch is requested, use `git checkout --orphan`. Closes The-Vibe-Company#487
The sidebar footer nav sections (Workbench: Prompts/Integrations/Terminal, Workspace: Environments/Agents/Settings) take ~300px of fixed vertical space on mobile (6 items × 44px min-height + headers/padding). This leaves very little room for the session list, making it nearly unusable. Fix: Make section headers clickable toggles with a chevron indicator. Sections default to collapsed on mobile (<768px) and expanded on desktop, following the same pattern used by Scheduled Runs and Agent Runs sections. Closes The-Vibe-Company#490
|
Someone is attempting to deploy a commit to the The Vibe Company Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
Greptile SummaryThis PR bundles two independent changes: (1) a UX improvement making the Workbench/Workspace/Resources nav sections in the sidebar collapsible (collapsed by default on mobile), and (2) a correctness fix in Key changes:
Issues found:
Confidence Score: 2/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Sidebar mounts] --> B{window.innerWidth < 768?}
B -- yes --> C[collapsedNavSections = all section IDs]
B -- no --> D[collapsedNavSections = empty Set]
C & D --> E[Render NAV_SECTIONS]
E --> F{section.id in collapsedNavSections?}
F -- yes / collapsed --> G[Render header button only\nchevron points right →]
F -- no / expanded --> H[Render header button + child nav items\nchevron points down ↓]
G --> I[User clicks header]
H --> I
I --> J[toggleNavSection: add/remove from Set]
J --> F
Last reviewed commit: 12e0c11 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+666
to
+672
| {NAV_SECTIONS.map((section) => { | ||
| const isCollapsed = collapsedNavSections.has(section.id); | ||
| return ( | ||
| <section key={section.id} className="rounded-lg border border-cc-border/30 bg-cc-card/20 p-0.5"> | ||
| <button | ||
| onClick={() => toggleNavSection(section.id)} | ||
| className="w-full flex items-center gap-1 px-2 py-0.5 cursor-pointer hover:text-cc-fg transition-colors" |
Contributor
There was a problem hiding this comment.
<h3> inside <button> is invalid HTML
The new section-header button wraps an <h3> element as a direct child. Heading elements (h1–h6) are "flow content" in the HTML5 spec, not "phrasing content", so they are not valid descendants of <button> (which requires phrasing content per spec). Browsers often silently recover by popping the heading outside the button in the parsed DOM, which would break the click handler. Screen readers also lose the button role or the heading role unexpectedly.
The original code had <h3> as a standalone sibling; make it a <span> (or plain text) inside the button to keep the semantics valid:
Suggested change
| {NAV_SECTIONS.map((section) => { | |
| const isCollapsed = collapsedNavSections.has(section.id); | |
| return ( | |
| <section key={section.id} className="rounded-lg border border-cc-border/30 bg-cc-card/20 p-0.5"> | |
| <button | |
| onClick={() => toggleNavSection(section.id)} | |
| className="w-full flex items-center gap-1 px-2 py-0.5 cursor-pointer hover:text-cc-fg transition-colors" | |
| <button | |
| onClick={() => toggleNavSection(section.id)} | |
| className="w-full flex items-center gap-1 px-2 py-0.5 cursor-pointer hover:text-cc-fg transition-colors" | |
| aria-expanded={!isCollapsed} | |
| > | |
| <svg viewBox="0 0 16 16" fill="currentColor" className={`w-2.5 h-2.5 text-cc-muted/60 transition-transform duration-150 ${isCollapsed ? "" : "rotate-90"}`}> | |
| <path d="M6 4l4 4-4 4" /> | |
| </svg> | |
| <span className="text-[9px] font-semibold uppercase tracking-[0.14em] text-cc-muted/75"> | |
| {section.label} | |
| </span> |
Prompt To Fix With AI
This is a comment left during a code review.
Path: web/src/components/Sidebar.tsx
Line: 666-672
Comment:
**`<h3>` inside `<button>` is invalid HTML**
The new section-header button wraps an `<h3>` element as a direct child. Heading elements (`h1`–`h6`) are "flow content" in the HTML5 spec, not "phrasing content", so they are not valid descendants of `<button>` (which requires phrasing content per spec). Browsers often silently recover by popping the heading outside the button in the parsed DOM, which would break the click handler. Screen readers also lose the button role or the heading role unexpectedly.
The original code had `<h3>` as a standalone sibling; make it a `<span>` (or plain text) inside the button to keep the semantics valid:
```suggestion
<button
onClick={() => toggleNavSection(section.id)}
className="w-full flex items-center gap-1 px-2 py-0.5 cursor-pointer hover:text-cc-fg transition-colors"
aria-expanded={!isCollapsed}
>
<svg viewBox="0 0 16 16" fill="currentColor" className={`w-2.5 h-2.5 text-cc-muted/60 transition-transform duration-150 ${isCollapsed ? "" : "rotate-90"}`}>
<path d="M6 4l4 4-4 4" />
</svg>
<span className="text-[9px] font-semibold uppercase tracking-[0.14em] text-cc-muted/75">
{section.label}
</span>
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
663
to
730
| {/* Footer */} | ||
| <div className="px-2 py-1.5 pb-safe bg-cc-sidebar-footer border-t border-cc-border/30"> | ||
| <nav className="flex flex-col gap-1.5" aria-label="Navigation"> | ||
| {NAV_SECTIONS.map((section) => ( | ||
| <section key={section.id} className="rounded-lg border border-cc-border/30 bg-cc-card/20 p-0.5"> | ||
| <h3 className="px-2 py-0.5 text-[9px] font-semibold uppercase tracking-[0.14em] text-cc-muted/75"> | ||
| {section.label} | ||
| </h3> | ||
| <div className="flex flex-col"> | ||
| {section.itemIds.map((itemId) => { | ||
| const item = NAV_ITEMS_BY_ID.get(itemId); | ||
| if (!item) return null; | ||
| const isActive = item.activePages | ||
| ? item.activePages.some((p) => route.page === p) | ||
| : route.page === item.id; | ||
| return ( | ||
| <button | ||
| key={item.id} | ||
| onClick={() => { | ||
| if (item.id !== "terminal") { | ||
| useStore.getState().closeTerminal(); | ||
| } | ||
| window.location.hash = item.hash; | ||
| // Close sidebar on mobile so the navigated page is visible | ||
| if (window.innerWidth < 768) { | ||
| useStore.getState().setSidebarOpen(false); | ||
| } | ||
| }} | ||
| title={item.label} | ||
| aria-current={isActive ? "page" : undefined} | ||
| className={`group flex min-h-[44px] md:min-h-[34px] w-full items-center gap-2 rounded-md px-2 py-1 md:py-0.5 text-left transition-colors duration-150 cursor-pointer focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cc-primary/60 ${ | ||
| isActive | ||
| ? "bg-cc-active text-cc-fg" | ||
| : "text-cc-muted hover:text-cc-fg hover:bg-cc-hover" | ||
| }`} | ||
| > | ||
| <span | ||
| aria-hidden | ||
| className={`h-4 w-0.5 shrink-0 rounded-full transition-colors ${ | ||
| isActive ? "bg-cc-primary" : "bg-transparent group-hover:bg-cc-border" | ||
| }`} | ||
| /> | ||
| <svg viewBox={item.viewBox} fill="currentColor" className="w-3.5 h-3.5 shrink-0"> | ||
| <path d={item.iconPath} fillRule={item.fillRule} clipRule={item.clipRule} /> | ||
| </svg> | ||
| <span className="min-w-0 flex-1 text-[12px] font-medium leading-tight">{item.label}</span> | ||
| </button> | ||
| ); | ||
| })} | ||
| </div> | ||
| </section> | ||
| ))} | ||
| {NAV_SECTIONS.map((section) => { | ||
| const isCollapsed = collapsedNavSections.has(section.id); | ||
| return ( | ||
| <section key={section.id} className="rounded-lg border border-cc-border/30 bg-cc-card/20 p-0.5"> | ||
| <button | ||
| onClick={() => toggleNavSection(section.id)} | ||
| className="w-full flex items-center gap-1 px-2 py-0.5 cursor-pointer hover:text-cc-fg transition-colors" | ||
| > | ||
| <svg viewBox="0 0 16 16" fill="currentColor" className={`w-2.5 h-2.5 text-cc-muted/60 transition-transform duration-150 ${isCollapsed ? "" : "rotate-90"}`}> | ||
| <path d="M6 4l4 4-4 4" /> | ||
| </svg> | ||
| <h3 className="text-[9px] font-semibold uppercase tracking-[0.14em] text-cc-muted/75"> | ||
| {section.label} | ||
| </h3> | ||
| </button> | ||
| {!isCollapsed && ( | ||
| <div className="flex flex-col"> | ||
| {section.itemIds.map((itemId) => { | ||
| const item = NAV_ITEMS_BY_ID.get(itemId); | ||
| if (!item) return null; | ||
| const isActive = item.activePages | ||
| ? item.activePages.some((p) => route.page === p) | ||
| : route.page === item.id; | ||
| return ( | ||
| <button | ||
| key={item.id} | ||
| onClick={() => { | ||
| if (item.id !== "terminal") { | ||
| useStore.getState().closeTerminal(); | ||
| } | ||
| window.location.hash = item.hash; | ||
| // Close sidebar on mobile so the navigated page is visible | ||
| if (window.innerWidth < 768) { | ||
| useStore.getState().setSidebarOpen(false); | ||
| } | ||
| }} | ||
| title={item.label} | ||
| aria-current={isActive ? "page" : undefined} | ||
| className={`group flex min-h-[44px] md:min-h-[34px] w-full items-center gap-2 rounded-md px-2 py-1 md:py-0.5 text-left transition-colors duration-150 cursor-pointer focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cc-primary/60 ${ | ||
| isActive | ||
| ? "bg-cc-active text-cc-fg" | ||
| : "text-cc-muted hover:text-cc-fg hover:bg-cc-hover" | ||
| }`} | ||
| > | ||
| <span | ||
| aria-hidden | ||
| className={`h-4 w-0.5 shrink-0 rounded-full transition-colors ${ | ||
| isActive ? "bg-cc-primary" : "bg-transparent group-hover:bg-cc-border" | ||
| }`} | ||
| /> | ||
| <svg viewBox={item.viewBox} fill="currentColor" className="w-3.5 h-3.5 shrink-0"> | ||
| <path d={item.iconPath} fillRule={item.fillRule} clipRule={item.clipRule} /> | ||
| </svg> | ||
| <span className="min-w-0 flex-1 text-[12px] font-medium leading-tight">{item.label}</span> | ||
| </button> | ||
| ); | ||
| })} | ||
| </div> | ||
| )} | ||
| </section> | ||
| ); | ||
| })} | ||
| </nav> | ||
| <div className="mt-1.5 rounded-lg border border-cc-border/30 bg-cc-card/20 px-1.5 py-0.5"> | ||
| <div className="flex items-center justify-between"> |
Contributor
There was a problem hiding this comment.
Missing tests for new collapsible behavior
The PR adds significant new behavior — section toggle on click, chevron rotation, and mobile-default-collapsed state derived from window.innerWidth — but Sidebar.test.tsx gains zero new test cases. The existing suite only asserts that the section labels are present (getByText("Workbench")); it does not cover:
- Clicking a section header hides its child nav items (collapsed state)
- Clicking again re-shows them (expanded state)
- On a viewport width < 768 the sections initialize as collapsed
Per the project's frontend-testing rule, behavior changes in web/src/components/**/*.tsx require matching coverage in Sidebar.test.tsx. Please add tests for at least the toggle (click-to-collapse / click-to-expand) and the mobile-default path (mock window.innerWidth = 375 before render and assert items are not in the DOM).
Rule Used: CLAUDE.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: web/src/components/Sidebar.tsx
Line: 663-730
Comment:
**Missing tests for new collapsible behavior**
The PR adds significant new behavior — section toggle on click, chevron rotation, and mobile-default-collapsed state derived from `window.innerWidth` — but `Sidebar.test.tsx` gains zero new test cases. The existing suite only asserts that the section labels are present (`getByText("Workbench")`); it does not cover:
- Clicking a section header hides its child nav items (collapsed state)
- Clicking again re-shows them (expanded state)
- On a viewport width < 768 the sections initialize as collapsed
Per the project's frontend-testing rule, behavior changes in `web/src/components/**/*.tsx` require matching coverage in `Sidebar.test.tsx`. Please add tests for at least the toggle (click-to-collapse / click-to-expand) and the mobile-default path (mock `window.innerWidth = 375` before render and assert items are not in the DOM).
**Rule Used:** CLAUDE.md ([source](https://app.greptile.com/review/custom-context?memory=666e8656-df2f-4480-b76a-67f089c8461a))
How can I resolve this? If you propose a fix, please make it concise.
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.
Summary
Problem
On mobile viewports, the sidebar footer nav sections take ~300px of fixed vertical space (6 nav items ×
min-h-[44px]+ section headers + padding), leaving the session list nearly unusable. Users have to scroll past Prompts, Integrations, Terminal, Environments, Agents, and Settings just to see their sessions.Fix
Added collapsible toggle behavior to
NAV_SECTIONSrendering, following the same pattern already used by "Scheduled Runs", "Agent Runs", and "Archived" sections in the sidebar:<button>with a chevron iconcollapsedNavSectionsstate tracks which sections are collapsedwindow.innerWidth— collapsed by default on mobileTest plan
Closes #490