From 871f7d1124632449ac39d937005b9cf7515d5b23 Mon Sep 17 00:00:00 2001 From: scotttong Date: Thu, 3 Sep 2026 15:35:39 -0700 Subject: [PATCH] fix(ui): polish core navigation and task layout (#12793) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Operators use the main navigation, contextual navigation, and task chat throughout the product. > - The recent core UI refactor left uneven spacing and inconsistent navigation styles. > - The Apps label also did not match the Connectors product language. > - The account area did not provide a clear direct path for feedback. > - This pull request aligns these related core UI surfaces and preserves their existing behavior. > - The benefit is a more consistent interface with clearer navigation and balanced task-chat layout. ## Linked Issues or Issue Description **What existing behavior does this improve?** This improves the core sidebar, Settings navigation, Connectors catalog, task-chat layout, and account controls. **Subsystem affected** `ui/` — React and Vite board UI. **Current behavior** The task chat had uneven edge treatment. Settings used a separate contextual-navigation style. Apps used inconsistent product labels. The account footer did not expose a direct feedback control. **Proposed behavior** The task chat keeps balanced content padding while its scrollbar sits at the properties boundary. Settings replaces the primary sidebar with a matching navigation surface and a Back to app link. Apps uses Connectors and Browse labels. The account footer provides a dedicated feedback icon with a tooltip. **Reason and benefit** These changes make related navigation and layout patterns predictable. They reduce duplicate labels and improve access to feedback. **Breaking changes** None. Routes, APIs, and stored data do not change. ## What Changed - Balanced the task-chat content gutter and moved its scrollbar to the properties-panel boundary. - Reworked Settings navigation to replace the main sidebar and use the shared primary-sidebar style. - Added a Back to app navigation item to Settings. - Renamed Apps to Connectors in the main navigation and added the `Unplug` icon. - Renamed the Connectors contextual item to Browse. - Added the Connectors top-level header and aligned the search field with the connector cards. - Added account-footer hover states and a direct feedback flag with a Share feedback tooltip. - Removed the duplicate Feedback item from the account popover. - Added regression coverage for each changed UI surface. ## Verification - `pnpm --filter @paperclipai/ui exec vitest run src/components/AppsSidebar.test.tsx src/components/CompanySettingsSidebar.test.tsx src/components/Layout.test.tsx src/components/Sidebar.test.tsx src/components/SidebarAccountMenu.test.tsx src/components/task-chat/TaskMessageScroller.test.tsx src/pages/apps/Browse.test.tsx` — 90 tests passed. - `pnpm --filter @paperclipai/ui typecheck` — passed. - `pnpm --filter @paperclipai/ui build` — passed. - `pnpm check:token-gates` — passed. - `git diff --check origin/master...HEAD` — passed. - `env PAPERCLIP_PLAYWRIGHT_CHANNEL=chrome PAPERCLIP_E2E_PORT=3201 pnpm exec playwright test --config tests/e2e/playwright.config.ts tests/e2e/apps-dark-mode-shots.spec.ts tests/e2e/sidebar-takeover.spec.ts` — 10 tests passed. - The full workspace typecheck and build reached the Rust runner and stopped because `cargo` is not installed on this machine. - The full test suite exposed unrelated server and workspace-runtime failures and was stopped after the affected suites completed. No changed UI test failed. - Manually verified the changed Settings, Connectors, task-chat, and account-menu surfaces in the running app. ## Risks - Low risk. The change affects layout and navigation presentation only. - The Settings sidebar now replaces the main sidebar by design. Users must use Back to app to return to the application navigation. - The task scrollbar offset depends on the existing responsive page gutters. Regression tests cover both narrow and desktop spacing. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex, `gpt-5.6-sol`, extended reasoning with tool use and code execution. The host does not expose the 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 - [ ] 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 --------- Co-authored-by: Scott Tong Co-authored-by: Paperclip --- tests/e2e/apps-dark-mode-shots.spec.ts | 8 +- tests/e2e/sidebar-takeover.spec.ts | 18 +- ui/src/components/AppsSidebar.production.tsx | 10 +- ui/src/components/AppsSidebar.test.tsx | 43 ++- ui/src/components/AppsSidebar.tsx | 4 +- .../CompanySettingsSidebar.test.tsx | 35 ++- ui/src/components/CompanySettingsSidebar.tsx | 25 +- ui/src/components/Layout.test.tsx | 24 +- ui/src/components/Layout.tsx | 5 +- ui/src/components/Sidebar.production.tsx | 4 +- ui/src/components/Sidebar.test.tsx | 13 +- ui/src/components/Sidebar.tsx | 13 +- .../SidebarAccountMenu.production.tsx | 38 ++- ui/src/components/SidebarAccountMenu.test.tsx | 143 +++++++--- ui/src/components/SidebarAccountMenu.tsx | 270 +++++++++--------- ui/src/components/primary-sidebar-styles.ts | 9 + .../task-chat/TaskMessageScroller.test.tsx | 10 +- .../task-chat/TaskMessageScroller.tsx | 15 +- ui/src/pages/apps/Browse.test.tsx | 6 +- ui/src/pages/apps/Browse.tsx | 8 +- 20 files changed, 452 insertions(+), 249 deletions(-) create mode 100644 ui/src/components/primary-sidebar-styles.ts diff --git a/tests/e2e/apps-dark-mode-shots.spec.ts b/tests/e2e/apps-dark-mode-shots.spec.ts index bbc1de02f7..4090a0a2a7 100644 --- a/tests/e2e/apps-dark-mode-shots.spec.ts +++ b/tests/e2e/apps-dark-mode-shots.spec.ts @@ -115,12 +115,12 @@ test.describe.serial("dark-mode Apps surfaces", () => { await healthy?.close(); }); - test("sidebar says Apps and links to /apps", async ({ page }) => { + test("sidebar says Connectors and links to /apps", async ({ page }) => { await forceDark(page); await page.goto(`/${seed.prefix}/dashboard`); - const appsLink = page.getByRole("link", { name: "Apps", exact: true }); - await expect(appsLink).toBeVisible({ timeout: 30_000 }); - await expect(appsLink).toHaveAttribute("href", new RegExp(`/${seed.prefix}/apps$`)); + const connectorsLink = page.getByRole("link", { name: "Connectors", exact: true }); + await expect(connectorsLink).toBeVisible({ timeout: 30_000 }); + await expect(connectorsLink).toHaveAttribute("href", new RegExp(`/${seed.prefix}/apps$`)); }); test("apps list dark mode with attention banner", async ({ page }) => { diff --git a/tests/e2e/sidebar-takeover.spec.ts b/tests/e2e/sidebar-takeover.spec.ts index af55e6fff8..e979aaa1a4 100644 --- a/tests/e2e/sidebar-takeover.spec.ts +++ b/tests/e2e/sidebar-takeover.spec.ts @@ -3,9 +3,9 @@ import { test, expect, request as pwRequest, type APIRequestContext } from "@pla /** * E2E: contextual sidebar companion model. * - * Contextual routes render their navigation beside the stable global sidebar. - * The global company navigation and account menu remain available, and leaving - * the surface restores the user's global sidebar preference. + * Most contextual routes render their navigation beside the stable global + * sidebar. Settings intentionally takes over that sidebar while preserving the + * account menu, and its Back to app link restores the global navigation. * * Plugin route sidebars share the same Layout path. A live plugin-route test * requires a plugin fixture, so that branch remains covered by Layout tests. @@ -60,7 +60,7 @@ test.describe("Contextual sidebar companion", () => { }, COLLAPSED_STORAGE_KEY); }); - test("shows Settings beside the global navigation", async ({ page }) => { + test("replaces global navigation with Settings navigation", async ({ page }) => { await page.goto(`/${prefix}/company/settings`); const contextual = page.locator('[data-contextual-sidebar="settings"]'); @@ -70,10 +70,10 @@ test.describe("Contextual sidebar companion", () => { await expect(contextual.getByRole("link", { name: "General" })).toBeVisible(); await expect(contextual.getByText("Environments", { exact: true })).toBeVisible(); - await expect(page.getByRole("button", { name: "Back from Settings" })).toBeVisible(); + await expect(contextual.getByRole("link", { name: "Back to app" })).toBeVisible(); await expect(page.getByRole("button", { name: "Open account menu" })).toBeVisible(); - await expect(page.getByRole("link", { name: "Dashboard" })).toBeVisible(); + await expect(page.getByRole("link", { name: "Dashboard" })).toHaveCount(0); await expect(page.getByLabel(APP_SIDEBAR_EXPANDED_MARKER)).toHaveCount(0); }); @@ -91,7 +91,7 @@ test.describe("Contextual sidebar companion", () => { test("keeps the retired collapse control absent across contextual navigation", async ({ page }) => { await page.goto(`/${prefix}/company/settings`); await expect(page.locator('[data-contextual-sidebar="settings"]')).toBeVisible(); - await expect(page.getByRole("link", { name: "Dashboard" })).toBeVisible(); + await expect(page.getByRole("link", { name: "Back to app" })).toBeVisible(); await expect(page.getByLabel(APP_SIDEBAR_EXPANDED_MARKER)).toHaveCount(0); await page.goto(`/${prefix}/dashboard`); @@ -101,9 +101,9 @@ test.describe("Contextual sidebar companion", () => { await expect(page.getByLabel(APP_SIDEBAR_EXPANDED_MARKER)).toHaveCount(0); }); - test("uses Dashboard as the safe fallback for a direct Settings link", async ({ page }) => { + test("uses Dashboard as the destination for a direct Settings link", async ({ page }) => { await page.goto(`/${prefix}/company/settings`); - await page.getByRole("button", { name: "Back from Settings" }).click(); + await page.getByRole("link", { name: "Back to app" }).click(); await expect(page).toHaveURL(new RegExp(`/${prefix}/dashboard$`)); await expect(page.locator("[data-contextual-sidebar]")).toHaveCount(0); diff --git a/ui/src/components/AppsSidebar.production.tsx b/ui/src/components/AppsSidebar.production.tsx index d3f09d355d..a04d31f3b6 100644 --- a/ui/src/components/AppsSidebar.production.tsx +++ b/ui/src/components/AppsSidebar.production.tsx @@ -8,10 +8,10 @@ import { useReviewCount } from "@/pages/apps/useReviewCount"; import { SidebarNavItem } from "./SidebarNavItem.production"; /** - * Secondary sidebar for the prosumer Apps area (PAP-10856; three-door IA + * Secondary sidebar for the prosumer Connectors area (PAP-10856; three-door IA * PAP-13254 / U3). * - * ← Back · APPS: Browse / Review (n) + * ← Back · CONNECTORS: Browse / Review (n) * DEVELOPER: Connections / Gateways / Profiles / Rules / Health / Activity * * "Browse" is the store and "Review" holds decisions waiting on the user's @@ -19,7 +19,7 @@ import { SidebarNavItem } from "./SidebarNavItem.production"; * "Needs attention" is no longer a door: health/error triage folds into * Connections as a status filter + banner, so approvals are never buried * behind an error label. The Developer section was folded in from the retired - * ToolsSidebar (PAP-10915) so the whole Apps area shares one sidebar; a + * ToolsSidebar (PAP-10915) so the whole Connectors area shares one sidebar; a * one-line caption frames who it's for (Finding A). "Run your own" and "Paste a * config" moved out of the sidebar into rows on the Connect-an-app page * (PAP-10922). @@ -49,13 +49,13 @@ export function AppsSidebar() {
- Apps + Connectors