Skip to content

UI: use hover capability detection for sidebar actions and add collapsible nav#548

Open
whefter wants to merge 2 commits intoThe-Vibe-Company:mainfrom
whefter:fix/sidebar-tablet-hover-collapsible-nav
Open

UI: use hover capability detection for sidebar actions and add collapsible nav#548
whefter wants to merge 2 commits intoThe-Vibe-Company:mainfrom
whefter:fix/sidebar-tablet-hover-collapsible-nav

Conversation

@whefter
Copy link
Copy Markdown

@whefter whefter commented Mar 15, 2026

Summary

  • Replace sm:group-hover: breakpoint logic with @media (hover: hover) via a Tailwind v4 @custom-variant can-hover so session action buttons (archive, three-dot menu) are always visible on touch devices like tablets
  • Add a collapsible toggle to the sidebar footer navigation with localStorage persistence, freeing vertical space on constrained viewports

Why

  • Tablet bug: The sidebar used screen width (sm: = 640px) to decide whether to show action buttons on hover. Tablets exceed 640px but are touch devices with no hover capability, making archive/rename completely inaccessible (only double-tap rename worked).
  • Mobile space: The footer nav (~400px with 7 items at 44px touch targets + section headers + resources) consumed most of the vertical space on phones and short viewports, leaving barely any room for the session list.

Changes

  • web/src/index.css — new @custom-variant can-hover (@media (hover: hover))
  • web/src/components/SessionItem.tsxsm:can-hover: on archive + three-dot buttons
  • web/src/components/Sidebar.tsx — collapsible footer nav with "Navigation" toggle, default expanded, state in localStorage
  • web/src/components/Sidebar.test.tsx — updated class assertions + 4 new toggle tests

Testing

  • All 4336 tests pass, typecheck passes, extended tests
  • Verified on actual tablet

Review provenance

  • Implemented using Claude Code with Opus 4.6, Sparring - Research - Plan - Implement HITL approach
chrome_screenshot_Mar 15, 2026 7_44_30 AM GMT+01_00

…ollapsible nav

Replace sm:group-hover: breakpoint logic with @media (hover: hover) custom
variant so session action buttons (archive, three-dot menu) are always visible
on touch devices like tablets. Add a collapsible toggle to the sidebar footer
navigation with localStorage persistence, freeing vertical space on
constrained viewports.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 15, 2026

@whefter is attempting to deploy a commit to the The Vibe Company Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 4 files


Since this is your first cubic review, here's how it works:

  • cubic automatically reviews your code and comments on bugs and improvements
  • Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
  • Add one-off context when rerunning by tagging @cubic-dev-ai with guidance or docs links (including llms.txt)
  • Ask questions if you need clarification on any suggestion

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 15, 2026

Greptile Summary

This PR makes two independent UI improvements: it fixes a real tablet accessibility bug by swapping sm: breakpoint guards for a can-hover: (@media (hover: hover)) custom Tailwind variant on session action buttons, and it adds a collapsible footer navigation with localStorage persistence to recover vertical space on constrained viewports.

Key observations:

  • Tablet fix is correct and well-scoped — the @custom-variant can-hover declaration in index.css is the right Tailwind v4 primitive, and the class changes on SessionItem.tsx correctly make the three-dot menu always visible on touch devices while hiding the archive quick-action (which is reachable via the menu).
  • Six other components still use the old sm:group-hover: patternEnvManager.tsx, CronManager.tsx, Composer.tsx, HomePage.tsx, PromptsPage.tsx, and SandboxManager.tsx all have sm:opacity-0 sm:group-hover:opacity-100 on action buttons and will remain broken on touch tablets. This PR does not need to fix them all, but a follow-up issue is worth tracking.
  • Collapsible nav implementation is solid — lazy useState initializer with try/catch, useCallback for the toggle, correct aria-expanded / aria-label on the toggle button, and localStorage.removeItem in beforeEach to prevent test bleed. Minor gaps: the toggle button lacks aria-controls, and the chevron SVG uses fill on an unclosed path (renders as a filled triangle rather than a line chevron).
  • Test coverage is good — existing class assertions updated, four new toggle tests cover default, collapse, expand, and restore-from-storage flows. The archive button's own class change (sm:group-hover:can-hover:group-hover:) has no direct class assertion test, though the button's behaviour is exercised by other archive flow tests.

Confidence Score: 4/5

  • Safe to merge — no functional regressions, logic is correct, tests pass. Minor a11y and style gaps noted.
  • The core logic is sound: the hover-media-query approach is the correct fix for the tablet bug, the collapsible nav state management is well-guarded, and the new tests cover the meaningful paths. Score is 4 rather than 5 due to the stale comment on the archive button, the missing aria-controls linkage, and the open-path fill rendering of the chevron icon — none of which are regressions or functional bugs, but they slightly reduce polish and accessibility conformance.
  • No files require special attention for merge safety. The six unchanged components still using sm:group-hover:opacity-100 (EnvManager, CronManager, Composer, HomePage, PromptsPage, SandboxManager) represent a known follow-up gap but are out of scope for this PR.

Important Files Changed

Filename Overview
web/src/index.css Adds @custom-variant can-hover (@media (hover: hover)) — correct Tailwind v4 syntax, clean single-line change with no side effects.
web/src/components/SessionItem.tsx Replaces sm: breakpoint guards with can-hover: media-query variant on both action buttons. The three-dot menu is correctly always-visible on touch; the archive quick button is permanently hidden on touch devices (intentional, but the stale comment and test gap for its class change are worth addressing).
web/src/components/Sidebar.tsx Adds collapsible footer nav with localStorage persistence. State initialization, toggle logic, and try/catch guards are solid. Minor issues: open-path fill on the chevron SVG, and missing aria-controls on the toggle button.
web/src/components/Sidebar.test.tsx Correctly cleans localStorage in beforeEach, updates existing class assertions, and adds 4 well-structured tests covering default state, collapse, expand, and localStorage restoration.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Device renders SessionItem] --> B{can-hover media query?}

    B -- "No - touch only" --> C[Three-dot menu: always visible]
    B -- "No - touch only" --> D[Archive button: always hidden]
    C --> E[User taps menu to access Archive]

    B -- "Yes - pointer device" --> F{Group hover active?}
    F -- No --> G[Both buttons hidden]
    F -- Yes --> H[Three-dot menu visible]
    F -- Yes --> I[Archive quick button visible]

    subgraph SidebarFooter["Sidebar Footer Nav"]
        J[Read localStorage on mount] --> K{navExpanded?}
        K -- true --> L[Render nav sections and resources]
        K -- false --> M[Hide nav sections and resources]
        N[Toggle button click] --> O[Toggle state + write localStorage]
        O --> K
    end
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: web/src/components/SessionItem.tsx
Line: 266

Comment:
**Archive button invisible on all touch devices**

The archive quick-action button now has `opacity-0 pointer-events-none` as its base state with `can-hover:group-hover:*` to reveal it only on hover-capable devices. This means on **any** touch-only device (phone or tablet) the button is permanently invisible with no interaction affordance — there is no equivalent "always visible" baseline the way the three-dot menu has (`opacity-100 pointer-events-auto` at the top of its class list).

This is intentional only if the expectation is that touch users always reach Archive through the three-dot context menu. If so, it's worth adding a code comment confirming that — the asymmetry between the two buttons (three-dot is always visible on touch, archive is always hidden) is non-obvious and the current comment `/* Archive button — hover reveal (desktop), always visible (mobile) */` is now stale/misleading.

```suggestion
          className="absolute right-7 top-1/2 -translate-y-1/2 p-1 rounded-md opacity-0 pointer-events-none can-hover:group-hover:opacity-100 can-hover:group-hover:pointer-events-auto hover:bg-cc-border text-cc-muted hover:text-cc-fg transition-all cursor-pointer"
```

At a minimum, update the comment above the button (line 259) from `always visible (mobile)` to reflect the new reality that touch users access archive via the three-dot menu.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: web/src/components/Sidebar.tsx
Line: 715-721

Comment:
**Chevron SVG uses fill on an open path**

The chevron path `M4 6l4 4 4-4` is an open path (no `Z`/close command), but the SVG uses `fill="currentColor"`. SVG fills an open path by implicitly closing it — drawing a straight line from end `(12, 6)` back to start `(4, 6)` — which renders a solid filled triangle rather than two hairline strokes. At `w-2.5 h-2.5` (10 × 10 CSS px) this may look slightly thick/blurry.

A stroke-based approach is more conventional for chevron indicators and produces a sharper appearance at small sizes:

```suggestion
          <svg
            viewBox="0 0 16 16"
            fill="none"
            stroke="currentColor"
            strokeWidth="2"
            strokeLinecap="round"
            strokeLinejoin="round"
            className={`w-2.5 h-2.5 transition-transform duration-150 ${navExpanded ? "rotate-180" : ""}`}
          >
            <path d="M4 6l4 4 4-4" />
          </svg>
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: web/src/components/Sidebar.tsx
Line: 708-712

Comment:
**Missing `aria-controls` on the toggle button**

The toggle button correctly uses `aria-expanded`, but without `aria-controls` pointing to the `<nav>` element, screen readers cannot automatically jump to the controlled region. Adding the relationship makes the component more conformant with ARIA authoring practices for disclosure widgets.

```suggestion
        <button
          onClick={toggleNav}
          aria-expanded={navExpanded}
          aria-controls="sidebar-nav"
          aria-label={navExpanded ? "Collapse navigation" : "Expand navigation"}
          className="w-full flex items-center justify-between px-2 py-1 text-[10px] font-semibold uppercase tracking-[0.14em] text-cc-muted/75 hover:text-cc-fg hover:bg-cc-hover rounded-md transition-colors cursor-pointer"
        >
```

And add a matching `id` to the `<nav>`:
```
<nav id="sidebar-nav" className="flex flex-col gap-1.5 mt-1" aria-label="Navigation">
```

How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: d0ca679

- Update stale comment on archive button to reflect can-hover behavior
- Switch nav toggle chevron from fill to stroke for correct open-path rendering
- Add aria-controls linking nav toggle button to navigation element

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant