mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-28 04:02:18 +01:00
fix(ui): sidebar follow-ups — stable logo, symmetric rail highlight, sticky peek (PAP-10676)
Addresses board feedback on the collapsible sidebar: - Company logo no longer shifts on collapse. The workspace-switcher Button's default size includes `has-[>svg]:px-3`, so with the chevron svg present (expanded) its padding was 12px but in the rail (chevron hidden) it fell back to the `px-2` of 8px — a 4px horizontal jump. Pinning it to `px-3` keeps the logo's left edge in the icon column in both states. - Selected-item highlight is now symmetric in the rail. The icon sits at left=24 (nav px-3 + item px-3); a 56px rail left only 4px of trailing space so the active background looked clipped on the right / heavy on the left. Rail width is now 64 (24 + 16 icon + 24), centering the icon and making the full-width highlight symmetric around it. Icons still don't move (left edge unchanged). The cramped rail also clipped the search/toggle controls, so they are dropped in the rail (expansion stays available via hover-peek + Pin and Cmd/Ctrl+B); they return when expanded or peeking. - Peek no longer collapses on navigation. Removed the close-peek-on-pathname effect and guarded the panel blur handler with a pointer-inside ref: clicking a nav item moves focus to <main>, but while the pointer is still over the flyout the peek stays open until the user actually mouses off. Verified by rendering the real Sidebar at the 64px rail: company logo and every nav icon align to left=25 in both states (dX=0), the active highlight is centered/symmetric, and the icon-no-move guarantee still holds (max |delta| 1px, rounding). 60 sidebar tests + Layout tests pass; tsc clean. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
73f629de46
commit
8d13b25f51
5 files changed
+94
-58
No files matched your search
@@ -212,6 +212,11 @@ export function Layout() {
|
||||
// effective `peeking` to desktop + collapsed + hover-capable pointers, so
|
||||
// these handlers are inert otherwise.
|
||||
const peekTimer = useRef<number | null>(null);
|
||||
// Whether the pointer is currently over the peek panel. Used to keep the peek
|
||||
// open across focus changes (e.g. navigation steals focus to <main>) as long as
|
||||
// the user is still hovering — it should only close when they actually mouse off
|
||||
// (PAP-10676).
|
||||
const pointerInsidePanel = useRef(false);
|
||||
const clearPeekTimer = useCallback(() => {
|
||||
if (peekTimer.current !== null) {
|
||||
window.clearTimeout(peekTimer.current);
|
||||
@@ -230,15 +235,29 @@ export function Layout() {
|
||||
clearPeekTimer();
|
||||
peekTimer.current = window.setTimeout(() => setPeeking(false), 120);
|
||||
}, [clearPeekTimer, setPeeking]);
|
||||
const handlePanelPointerEnter = useCallback(() => {
|
||||
pointerInsidePanel.current = true;
|
||||
openPeek();
|
||||
}, [openPeek]);
|
||||
const handlePanelPointerLeave = useCallback(() => {
|
||||
pointerInsidePanel.current = false;
|
||||
closePeek();
|
||||
}, [closePeek]);
|
||||
// Close on focus leaving the panel only when the pointer isn't hovering it.
|
||||
// Clicking a rail/peek nav item moves focus to <main> on navigation; if the
|
||||
// mouse is still over the flyout we keep it open until the pointer leaves.
|
||||
const handlePanelBlur = useCallback(() => {
|
||||
if (pointerInsidePanel.current) return;
|
||||
closePeek();
|
||||
}, [closePeek]);
|
||||
|
||||
// Tidy up any pending peek timer on unmount.
|
||||
useEffect(() => clearPeekTimer, [clearPeekTimer]);
|
||||
|
||||
// Close peek on navigation so it never lingers over a freshly routed page.
|
||||
useEffect(() => {
|
||||
clearPeekTimer();
|
||||
setPeeking(false);
|
||||
}, [location.pathname, clearPeekTimer, setPeeking]);
|
||||
// Intentionally do NOT close the peek on navigation: clicking a nav item means
|
||||
// the pointer is still over the flyout, so it should stay open until the user
|
||||
// actually mouses off (handled by onPanelMouseLeave) or blurs out / hits Escape
|
||||
// (PAP-10676). Auto-closing here made the sidebar collapse on every page change.
|
||||
|
||||
// Escape closes an open peek without trapping the pointer.
|
||||
useEffect(() => {
|
||||
@@ -435,10 +454,10 @@ export function Layout() {
|
||||
collapsed={collapsed}
|
||||
peeking={peeking}
|
||||
resizable
|
||||
onPanelMouseEnter={collapsed ? openPeek : undefined}
|
||||
onPanelMouseLeave={collapsed ? closePeek : undefined}
|
||||
onPanelMouseEnter={collapsed ? handlePanelPointerEnter : undefined}
|
||||
onPanelMouseLeave={collapsed ? handlePanelPointerLeave : undefined}
|
||||
onPanelFocusCapture={collapsed ? openPeekImmediate : undefined}
|
||||
onPanelBlurCapture={collapsed ? closePeek : undefined}
|
||||
onPanelBlurCapture={collapsed ? handlePanelBlur : undefined}
|
||||
>
|
||||
<div className="flex flex-1 min-h-0">
|
||||
{isCompanySettingsRoute ? (
|
||||
|
||||
@@ -315,19 +315,20 @@ describe("Sidebar", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("header toggle expands a collapsed rail", async () => {
|
||||
it("keeps the collapsed rail top bar to just the company logo (no clipped search/toggle)", async () => {
|
||||
// In the narrow rail the search/toggle controls don't fit beside the logo and
|
||||
// would overflow/clip, shoving the logo out of the icon column (PAP-10676), so
|
||||
// they are dropped in the rail. Expansion stays reachable via hover-peek + Pin
|
||||
// and Cmd/Ctrl+B. The full controls return as soon as the panel is expanded or
|
||||
// peeking (covered by the other top-bar tests).
|
||||
mockSidebar.collapsed = true;
|
||||
mockInstanceSettingsApi.getExperimental.mockResolvedValue({ enableIsolatedWorkspaces: false });
|
||||
const root = await renderSidebar();
|
||||
|
||||
const toggle = container.querySelector<HTMLButtonElement>('button[aria-label="Expand sidebar"]');
|
||||
expect(toggle).not.toBeNull();
|
||||
expect(toggle?.getAttribute("aria-expanded")).toBe("false");
|
||||
|
||||
act(() => {
|
||||
toggle?.click();
|
||||
});
|
||||
expect(mockSidebar.toggleCollapsed).toHaveBeenCalledTimes(1);
|
||||
expect(container.querySelector('button[aria-label="Expand sidebar"]')).toBeNull();
|
||||
expect(container.querySelector('a[aria-label="Open search"]')).toBeNull();
|
||||
// The company menu (workspace switcher / logo) is still present in the rail.
|
||||
expect(container.textContent).toContain("Company menu");
|
||||
|
||||
flushSync(() => {
|
||||
root.unmount();
|
||||
|
||||
@@ -72,47 +72,56 @@ export function Sidebar() {
|
||||
{/* Top bar: Company name (bold) + Search — aligned with top sections (no visible border) */}
|
||||
<div className="flex items-center gap-1 px-3 h-12 shrink-0">
|
||||
<SidebarCompanyMenu />
|
||||
<Button
|
||||
asChild
|
||||
variant="ghost"
|
||||
size="icon-sm"
|
||||
className="text-muted-foreground shrink-0"
|
||||
aria-label="Open search"
|
||||
title="Open search"
|
||||
>
|
||||
<NavLink to="/search">
|
||||
<Search className="h-4 w-4" />
|
||||
</NavLink>
|
||||
</Button>
|
||||
{/* Desktop-only collapse/expand affordance. While peeking (hover flyout
|
||||
over the collapsed rail) it becomes a Pin that promotes the peek to a
|
||||
pinned-expanded sidebar; otherwise it toggles the pinned rail. Mobile
|
||||
uses the off-canvas drawer, so this control is hidden there. */}
|
||||
{!isMobile ? (
|
||||
peeking ? (
|
||||
{/* In the collapsed rail the search/toggle controls don't fit beside the
|
||||
logo — keeping them would overflow the 64px rail and squeeze the logo
|
||||
out of alignment with the icon column below it (PAP-10676). They return
|
||||
as soon as the panel is expanded (pinned) or peeking. Expansion in the
|
||||
rail is still reachable via hover-peek + Pin and Cmd/Ctrl+B. */}
|
||||
{!rail ? (
|
||||
<>
|
||||
<Button
|
||||
asChild
|
||||
variant="ghost"
|
||||
size="icon-sm"
|
||||
className="text-muted-foreground shrink-0"
|
||||
aria-label="Keep sidebar expanded"
|
||||
title="Keep sidebar expanded"
|
||||
onClick={() => setCollapsed(false)}
|
||||
aria-label="Open search"
|
||||
title="Open search"
|
||||
>
|
||||
<Pin className="h-4 w-4" />
|
||||
<NavLink to="/search">
|
||||
<Search className="h-4 w-4" />
|
||||
</NavLink>
|
||||
</Button>
|
||||
) : (
|
||||
<Button
|
||||
variant="ghost"
|
||||
size="icon-sm"
|
||||
className="text-muted-foreground shrink-0"
|
||||
aria-expanded={!collapsed}
|
||||
aria-label={collapsed ? "Expand sidebar" : "Collapse sidebar"}
|
||||
title={collapsed ? "Expand sidebar" : "Collapse sidebar"}
|
||||
onClick={() => toggleCollapsed()}
|
||||
>
|
||||
{collapsed ? <PanelLeftOpen className="h-4 w-4" /> : <PanelLeftClose className="h-4 w-4" />}
|
||||
</Button>
|
||||
)
|
||||
{/* Desktop-only collapse/expand affordance. While peeking (hover flyout
|
||||
over the collapsed rail) it becomes a Pin that promotes the peek to a
|
||||
pinned-expanded sidebar; otherwise it toggles the pinned rail. Mobile
|
||||
uses the off-canvas drawer, so this control is hidden there. */}
|
||||
{!isMobile ? (
|
||||
peeking ? (
|
||||
<Button
|
||||
variant="ghost"
|
||||
size="icon-sm"
|
||||
className="text-muted-foreground shrink-0"
|
||||
aria-label="Keep sidebar expanded"
|
||||
title="Keep sidebar expanded"
|
||||
onClick={() => setCollapsed(false)}
|
||||
>
|
||||
<Pin className="h-4 w-4" />
|
||||
</Button>
|
||||
) : (
|
||||
<Button
|
||||
variant="ghost"
|
||||
size="icon-sm"
|
||||
className="text-muted-foreground shrink-0"
|
||||
aria-expanded={!collapsed}
|
||||
aria-label={collapsed ? "Expand sidebar" : "Collapse sidebar"}
|
||||
title={collapsed ? "Expand sidebar" : "Collapse sidebar"}
|
||||
onClick={() => toggleCollapsed()}
|
||||
>
|
||||
{collapsed ? <PanelLeftOpen className="h-4 w-4" /> : <PanelLeftClose className="h-4 w-4" />}
|
||||
</Button>
|
||||
)
|
||||
) : null}
|
||||
</>
|
||||
) : null}
|
||||
</div>
|
||||
|
||||
|
||||
@@ -225,7 +225,12 @@ export function SidebarCompanyMenu({ open: controlledOpen, onOpenChange }: Sideb
|
||||
<DropdownMenuTrigger asChild>
|
||||
<Button
|
||||
variant="ghost"
|
||||
className="h-9 flex-1 justify-start gap-2 px-2 text-left"
|
||||
// `px-3` (not px-2) so the logo's left edge lines up with the nav icon
|
||||
// column (nav px-3 + item px-3) and, crucially, stays put between states:
|
||||
// the Button's default size adds `has-[>svg]:px-3`, so with the chevron
|
||||
// svg present (expanded) it was already 12px but without it (rail) it fell
|
||||
// back to 8px — a 4px horizontal jump on collapse (PAP-10676).
|
||||
className="h-9 flex-1 justify-start gap-2 px-3 text-left"
|
||||
aria-label={selectedCompany ? `Open ${selectedCompany.name} workspace switcher` : "Open workspace switcher"}
|
||||
>
|
||||
<span className="flex min-w-0 flex-1 items-center gap-2">
|
||||
|
||||
@@ -17,11 +17,13 @@ const MIN_SIDEBAR_WIDTH = 208;
|
||||
const MAX_SIDEBAR_WIDTH = 420;
|
||||
const SIDEBAR_WIDTH_STEP = 16;
|
||||
|
||||
// Collapsed icon rail. Tuned to the mock so the rail ends just past the icon:
|
||||
// nav px-3 (12) + item px-3 (12) + icon 16 = 40px to the icon's right edge,
|
||||
// leaving a small trailing margin. The icon's left edge is unchanged from the
|
||||
// expanded layout, which is what keeps icons pixel-identical across states.
|
||||
export const SIDEBAR_RAIL_WIDTH = 56;
|
||||
// Collapsed icon rail. Width is chosen so the icon is *centered* in the rail,
|
||||
// which keeps the active/hover highlight symmetric around it (PAP-10676):
|
||||
// the icon's left edge sits at nav px-3 (12) + item px-3 (12) = 24px, so a
|
||||
// matching 24px trailing margin (12 item px-3 + 12 nav px-3) yields
|
||||
// 24 + 16 (icon) + 24 = 64. The icon's left edge is unchanged from the expanded
|
||||
// layout, so icons stay pixel-identical across states.
|
||||
export const SIDEBAR_RAIL_WIDTH = 64;
|
||||
|
||||
function clampSidebarWidth(width: number) {
|
||||
return Math.min(MAX_SIDEBAR_WIDTH, Math.max(MIN_SIDEBAR_WIDTH, width));
|
||||
|
||||
Reference in new issue
Block a user