mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 01:57:31 +01:00
fix(ui): place pinned sidebar pages above More (#18915)
## Thinking Path > - Paperclip helps people manage AI agents and their work. > - The Coordinator sidebar gives people access to workspace pages. > - People can pin pages from the More menu. > - The sidebar puts these links below More in a separate Pinned section. > - This change puts pinned links above More in the main navigation. > - People can reach their saved pages without an extra section heading. ## Linked Issues or Issue Description **What existing behavior does this improve?** The placement of pinned pages in the Coordinator sidebar. **Subsystem affected** ui/ — React board UI. **Current behavior** Pinned pages appear below More with a Pinned heading and extra space. **Proposed behavior** Pinned pages appear after Needs you and before More. They use the same row spacing as the main links. **Reason and benefit** Saved pages belong with the main navigation. This placement keeps them visible above the menu that contains the other pages. **Breaking changes** The Pinned section and its heading are removed. Saved pin data, link order, routes, and pin controls use the existing code. ## What Changed - Move pinned links into the primary navigation above More. - Remove the separate section, heading, and extra space. - Correct the feature guide to state that pinned pages appear above More. - Update sidebar tests to check placement, saved order, account changes, restore, and pin removal. ## Verification - `pnpm exec vitest run ui/src/components/CoordinatorSidebar.test.tsx`: 15 tests pass. - `pnpm check:token-gates`: pass. - `node scripts/check-maps.mjs`: pass after the guide correction. - `pnpm --filter @paperclipai/ui typecheck`: pass. - Browser checks pass in Chromium against the production Coordinator sidebar in Storybook with fixture data. Screenshots cover saved pins and pinning from More. No page errors occurred. This does not test a live API. - The full workspace typecheck and build stop because this host has no Rust `cargo` command. - The first UI build was killed and the browser crashed while checks ran together. The host recorded OOM kills at its 8 GB limit. The browser check then passed after the full-suite process was stopped. The UI production build passed after the Storybook server was stopped. - `NODE_OPTIONS=--max-old-space-size=4096 pnpm --filter @paperclipai/ui build`: pass. - `pnpm test:run`: stopped during the first chat integration shard after memory exhaustion affected other checks. It has no full-suite result. The CI run passed the test lanes on the latest head. - [CI run](https://github.com/paperclipai/paperclip/actions/runs/41768881396) passed on `a836fc1320c363e0fbb5591312c6726026b6f151`. The required `ci / verify` and `ci / e2e` checks pass. Build, typecheck, Rust checks, test lanes, and all eight browser shards pass. - Greptile: 5/5 on this head. The documentation finding is fixed and its thread is resolved. - The additional Paperclip review is paused and queued. It is not a required branch check. ## Risks Low risk. This change moves existing links. It does not change storage or API behavior. The tests check expanded and collapsed rendering, the empty state, stored order, unknown routes, user changes, and changes from another browser tab. ## Model Used OpenAI GPT-6 through the Codex agent. The session supports reasoning, tool use, code edits, and command execution. The runtime does not expose a more specific model ID or context window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge
This commit is contained in:
1 parent
d180ad8c77
commit
89703c3ff7
3 files changed
+24
-19
No files matched your search
@@ -123,7 +123,7 @@ uses the production component with simulated API responses; it does not establis
|
||||
real persistence or permission behavior.
|
||||
|
||||
Coordinator More exposes a pin/unpin button beside every destination.
|
||||
Pinned pages appear beneath More in pinning order and retain the
|
||||
Pinned pages appear above More in pinning order and retain the
|
||||
existing company-aware routes.
|
||||
Unpin from either the flyout or the pinned row. The list is personal to the current
|
||||
user in this browser, shared across companies; it does not sync across devices.
|
||||
|
||||
@@ -85,7 +85,8 @@ describe("Coordinator pinned pages", () => {
|
||||
await renderSidebar();
|
||||
expect(container.querySelector('a[href="/chats"]')?.getAttribute("aria-label")).toBe("Chat, 2 unread chats");
|
||||
});
|
||||
const pinnedPages = () => container.querySelector('[aria-label="Pinned pages"]');
|
||||
const navigation = () => container.querySelector('nav[aria-label="Workspace navigation"]')!;
|
||||
const pinnedPage = (to: string) => navigation().querySelector(`a[href="${to}"]`);
|
||||
async function click(label: string) {
|
||||
const button = document.querySelector<HTMLButtonElement>(`button[aria-label="${label}"]`);
|
||||
expect(button).not.toBeNull();
|
||||
@@ -93,32 +94,39 @@ describe("Coordinator pinned pages", () => {
|
||||
await flush();
|
||||
}
|
||||
|
||||
it("pins from More without navigating, restores after remount, and unpins from the sidebar", async () => {
|
||||
it.each([false, true])("keeps pins above More without a separate section, restores and unpins when collapsed is %s", async collapsed => {
|
||||
fixture.collapsed = collapsed;
|
||||
await renderSidebar();
|
||||
expect(pinnedPages()).toBeNull();
|
||||
expect(pinnedPage("/projects")).toBeNull();
|
||||
await click("More workspace pages");
|
||||
expect(document.activeElement).toBe(document.querySelector('[data-slot="popover-content"]'));
|
||||
await click("Pin Projects");
|
||||
expect(document.querySelector('[role="dialog"]')).not.toBeNull();
|
||||
expect(pinnedPages()?.querySelector('a[href="/projects"]')?.textContent).toBe("Projects");
|
||||
expect(pinnedPage("/projects")?.textContent).toBe("Projects");
|
||||
expect(navigation().querySelector('[aria-label="Pinned pages"]')).toBeNull();
|
||||
expect(navigation().querySelector('[data-slot="sidebar-group-label"]')).toBeNull();
|
||||
const more = navigation().querySelector('button[aria-label="More workspace pages"]')!;
|
||||
expect(pinnedPage("/projects")!.compareDocumentPosition(more) & Node.DOCUMENT_POSITION_FOLLOWING).not.toBe(0);
|
||||
await act(async () => root.unmount());
|
||||
root = createRoot(container);
|
||||
await renderSidebar();
|
||||
expect(pinnedPages()?.querySelector('a[href="/projects"]')).not.toBeNull();
|
||||
expect(pinnedPage("/projects")).not.toBeNull();
|
||||
if (collapsed) await click("More workspace pages");
|
||||
await click("Unpin Projects");
|
||||
expect(pinnedPages()).toBeNull();
|
||||
expect(pinnedPage("/projects")).toBeNull();
|
||||
});
|
||||
|
||||
it("keeps pins personal and only renders known destinations", async () => {
|
||||
localStorage.setItem("paperclip.coordinator.pinned-nav.v1:reviewer", JSON.stringify(["/projects", "/unknown", "https://example.com"]));
|
||||
localStorage.setItem("paperclip.coordinator.pinned-nav.v1:reviewer", JSON.stringify(["/routines", "/projects", "/unknown", "https://example.com"]));
|
||||
await renderSidebar();
|
||||
expect(pinnedPages()?.querySelectorAll("a")).toHaveLength(1);
|
||||
expect(Array.from(navigation().querySelectorAll("a")).map(link => link.getAttribute("href")))
|
||||
.toEqual(["/chats", "/issues", "/decisions", "/routines", "/projects"]);
|
||||
fixture.userId = "other-user";
|
||||
await renderSidebar();
|
||||
expect(pinnedPages()).toBeNull();
|
||||
expect(pinnedPage("/projects")).toBeNull();
|
||||
fixture.userId = "reviewer";
|
||||
await renderSidebar();
|
||||
expect(pinnedPages()?.textContent).toContain("Projects");
|
||||
expect(pinnedPage("/projects")?.textContent).toBe("Projects");
|
||||
});
|
||||
|
||||
it("reflects pins changed in another browser tab", async () => {
|
||||
@@ -128,7 +136,7 @@ describe("Coordinator pinned pages", () => {
|
||||
localStorage.setItem(key, JSON.stringify(["/agents/all"]));
|
||||
window.dispatchEvent(new StorageEvent("storage", { key }));
|
||||
});
|
||||
expect(pinnedPages()?.querySelector('a[href="/agents/all"]')?.textContent).toBe("Agents");
|
||||
expect(pinnedPage("/agents/all")?.textContent).toBe("Agents");
|
||||
});
|
||||
});
|
||||
afterEach(async () => { await act(async () => root.unmount()); client.clear(); container.remove(); });
|
||||
|
||||
@@ -118,6 +118,10 @@ export function CoordinatorSidebar({ children }: { children?: ReactNode }) {
|
||||
className="absolute right-3 top-1/2 -translate-y-1/2 rounded-md p-1 text-muted-foreground opacity-0 transition-opacity group-hover/coordinator-new:opacity-100 group-focus-within/coordinator-new:opacity-100 pointer-coarse:opacity-100 hover:bg-sidebar-accent focus-visible:ring-2 focus-visible:ring-ring"><Plus className="size-4" /></button>}
|
||||
</div>
|
||||
<SidebarNavItem className="design-sidebar-item" to="/decisions" label="Needs you" icon={Bell} badge={attentionCount} badgeLabel="decisions" />
|
||||
{pinnedDestinations.map(({ label, to, icon }) => <div key={to} className="group/coordinator-pin relative">
|
||||
<SidebarNavItem to={to} label={label} icon={icon} className={cn("design-sidebar-item", !rail && "pr-9")} />
|
||||
{!rail && <button type="button" aria-label={`Unpin ${label}`} title={`Unpin ${label}`} onClick={() => pinnedNavigation.toggle(to)} className="absolute right-3 top-1/2 flex size-7 -translate-y-1/2 items-center justify-center rounded-md text-muted-foreground opacity-0 transition-opacity hover:bg-sidebar-accent hover:text-foreground focus-visible:ring-2 focus-visible:ring-ring group-hover/coordinator-pin:opacity-100 group-focus-within/coordinator-pin:opacity-100 pointer-coarse:opacity-100"><PinOff className="size-3.5" /></button>}
|
||||
</div>)}
|
||||
</div>
|
||||
<Flyout open={moreOpen} onOpenChange={setMoreOpen}>
|
||||
<FlyoutTrigger asChild><button type="button" aria-label="More workspace pages" className="design-sidebar-item flex w-auto items-center gap-2.5 rounded-lg py-1.5 text-left transition-colors focus-visible:ring-2 focus-visible:ring-ring"><MoreHorizontal className="size-4 shrink-0" /><span className={rail ? SIDEBAR_RAIL_HIDDEN_LABEL : undefined}>More</span></button></FlyoutTrigger>
|
||||
@@ -140,13 +144,6 @@ export function CoordinatorSidebar({ children }: { children?: ReactNode }) {
|
||||
</FlyoutContent>
|
||||
</Flyout>
|
||||
</div>
|
||||
{pinnedDestinations.length > 0 && <section className="space-y-1 pt-4" aria-label="Pinned pages">
|
||||
{!rail && <div data-slot="sidebar-group-label" className="px-4 pb-1 text-xs text-muted-foreground">Pinned</div>}
|
||||
{pinnedDestinations.map(({ label, to, icon }) => <div key={to} className="group/coordinator-pin relative">
|
||||
<SidebarNavItem to={to} label={label} icon={icon} className={cn("design-sidebar-item", !rail && "pr-9")} />
|
||||
{!rail && <button type="button" aria-label={`Unpin ${label}`} title={`Unpin ${label}`} onClick={() => pinnedNavigation.toggle(to)} className="absolute right-3 top-1/2 flex size-7 -translate-y-1/2 items-center justify-center rounded-md text-muted-foreground opacity-0 transition-opacity hover:bg-sidebar-accent hover:text-foreground focus-visible:ring-2 focus-visible:ring-ring group-hover/coordinator-pin:opacity-100 group-focus-within/coordinator-pin:opacity-100 pointer-coarse:opacity-100"><PinOff className="size-3.5" /></button>}
|
||||
</div>)}
|
||||
</section>}
|
||||
{pinnedNavigation.error && <p role="alert" className="px-4 py-2 text-xs text-destructive">{pinnedNavigation.error}</p>}
|
||||
{!rail && session.isSuccess && recents.length > 0 && <div className="design-sidebar-recents"><SidebarSection label="Recents">
|
||||
<div className="design-sidebar-recent-list scrollbar-while-scrolling" onScroll={handleRecentScroll} role="region" aria-label="Recents" tabIndex={0}>
|
||||
|
||||
Reference in new issue
Block a user