From eb72f53c1bd32b6ef3ae5ddc92e01d089fb7e0a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Fri, 31 Jul 2026 14:57:40 +0100 Subject: [PATCH] fix(web): phone navigation for plugin pages (#2093) Co-authored-by: Kandev Agent Co-authored-by: Claude Opus 5 (1M context) --- .../components/kanban/mobile-menu-sheet.tsx | 2 + apps/web/components/page-topbar.test.tsx | 42 +++++++ apps/web/components/page-topbar.tsx | 28 ++++- .../mobile-plugin-nav-section.test.tsx | 106 ++++++++++++++++ .../plugins/mobile-plugin-nav-section.tsx | 52 ++++++++ .../tests/plugins/mobile-plugin-nav.spec.ts | 115 ++++++++++++++++++ docs/plans/plugins/PLUGIN-API.md | 4 +- docs/public/plugins-authoring.md | 2 + 8 files changed, 347 insertions(+), 4 deletions(-) create mode 100644 apps/web/components/page-topbar.test.tsx create mode 100644 apps/web/components/plugins/mobile-plugin-nav-section.test.tsx create mode 100644 apps/web/components/plugins/mobile-plugin-nav-section.tsx create mode 100644 apps/web/e2e/tests/plugins/mobile-plugin-nav.spec.ts diff --git a/apps/web/components/kanban/mobile-menu-sheet.tsx b/apps/web/components/kanban/mobile-menu-sheet.tsx index 80ed61559b..02fc438144 100644 --- a/apps/web/components/kanban/mobile-menu-sheet.tsx +++ b/apps/web/components/kanban/mobile-menu-sheet.tsx @@ -19,6 +19,7 @@ import { } from "@tabler/icons-react"; import { AppSidebarWorkspacePicker } from "@/components/app-sidebar/app-sidebar-workspace-picker"; import { MobileIntegrationsSection } from "@/components/integrations/integrations-menu"; +import { MobilePluginNavSection } from "@/components/plugins/mobile-plugin-nav-section"; import { TaskSearchInput } from "./task-search-input"; import { MobileTasksListOptions, @@ -463,6 +464,7 @@ function MobileMenuContent({ showPipeline={showPipeline} /> + onOpenChange(false)} /> onOpenChange(false)} /> ({ + AppStatusDrawerTrigger: () => null, +})); + +const PHONE_HOME = "topbar-phone-home"; + +describe("PageTopbar phone home crumb", () => { + afterEach(cleanup); + + it("renders a phone-only home crumb for a root-level page with no nav of its own", () => { + render(); + + const home = screen.getByTestId(PHONE_HOME); + expect(home).not.toBeNull(); + // Hidden from md up, where the always-visible AppSidebar owns Home. + expect(home.className).toContain("md:hidden"); + expect(home.querySelector("a")?.getAttribute("href")).toBe("/"); + }); + + it("omits it when the page supplies its own leading nav", () => { + render(Open menu} />); + + expect(screen.queryByTestId(PHONE_HOME)).toBeNull(); + }); + + it("omits it when the page already shows a real back link", () => { + render(); + + expect(screen.queryByTestId(PHONE_HOME)).toBeNull(); + expect(screen.getByText("Agents")).not.toBeNull(); + }); + + it("omits it for the root variant, which renders a plain label instead of a breadcrumb", () => { + render(); + + expect(screen.queryByTestId(PHONE_HOME)).toBeNull(); + }); +}); diff --git a/apps/web/components/page-topbar.tsx b/apps/web/components/page-topbar.tsx index fccbfccbbb..e88e85613b 100644 --- a/apps/web/components/page-topbar.tsx +++ b/apps/web/components/page-topbar.tsx @@ -73,6 +73,7 @@ function TopbarBreadcrumb({ title, subtitle, icon, + showPhoneHome, }: { backHref: string; backLabel: string; @@ -80,11 +81,16 @@ function TopbarBreadcrumb({ title: string; subtitle?: string; icon?: ReactNode; + showPhoneHome: boolean; }) { - // The Home prefix is redundant now that the AppSidebar always shows a Home - // nav item. Only render the back link when a page sets a non-root backHref - // (e.g. a true ancestor route within a section). + // The Home prefix is redundant on desktop, where the AppSidebar always shows + // a Home nav item. Only render the back link when a page sets a non-root + // backHref (e.g. a true ancestor route within a section). const showBackLink = backHref !== "/" && !!backLabel; + // Below md the sidebar is hidden, so a root-level page that brings no nav of + // its own (a plugin route with default chrome) would otherwise strand phone + // users with no way back. Keep the home crumb phone-only. + const showHomeCrumb = !showBackLink && showPhoneHome; return ( @@ -98,6 +104,16 @@ function TopbarBreadcrumb({ )} + {showHomeCrumb && ( + <> + + + + + + + + )} {parents?.flatMap((p) => [ @@ -138,6 +154,7 @@ type TopbarLeadingProps = { title: string; subtitle?: string; icon?: ReactNode; + showPhoneHome: boolean; }; function TopbarLeading({ @@ -148,6 +165,7 @@ function TopbarLeading({ title, subtitle, icon, + showPhoneHome, }: TopbarLeadingProps) { if (variant === "root") { if (!backLabel) return null; @@ -165,6 +183,7 @@ function TopbarLeading({ title={title} subtitle={subtitle} icon={icon} + showPhoneHome={showPhoneHome} /> ); } @@ -206,6 +225,9 @@ export const PageTopbar = forwardRef(function Page title={title} subtitle={subtitle} icon={icon} + // A page that supplies its own `leading` nav (Settings' menu sheet) + // already gets phone users home; don't add a second affordance. + showPhoneHome={!leading} /> {leftActions && (
diff --git a/apps/web/components/plugins/mobile-plugin-nav-section.test.tsx b/apps/web/components/plugins/mobile-plugin-nav-section.test.tsx new file mode 100644 index 0000000000..fc4e15b7a7 --- /dev/null +++ b/apps/web/components/plugins/mobile-plugin-nav-section.test.tsx @@ -0,0 +1,106 @@ +import { cleanup, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { pluginRegistry } from "@/lib/plugins/registry"; +import { MobilePluginNavSection } from "./mobile-plugin-nav-section"; + +const HELLO_PATH = "/plugins/hello"; + +vi.mock("@/lib/routing/client-router", () => ({ + usePathname: () => "/", +})); + +function renderSection(onNavigate = () => {}) { + return render(); +} + +describe("MobilePluginNavSection", () => { + afterEach(() => { + cleanup(); + ["plugin-a", "plugin-b"].forEach((id) => pluginRegistry.unregisterPlugin(id)); + window.history.pushState({}, "", "/"); + }); + + it("renders nothing when no plugin has registered a nav item", () => { + const { container } = renderSection(); + expect(container.innerHTML).toBe(""); + }); + + it("renders main-section nav items so plugin pages are reachable on a phone", () => { + pluginRegistry + .forPlugin("plugin-a") + .registerNavItem({ id: "hello", label: "Hello", path: HELLO_PATH }); + + renderSection(); + + expect(screen.getByTestId("mobile-plugin-nav-section")).not.toBeNull(); + expect(screen.getByTestId("mobile-plugin-nav-item-hello")).not.toBeNull(); + expect(screen.getByText("Hello")).not.toBeNull(); + }); + + it("treats an omitted section as main, matching the desktop sidebar", () => { + pluginRegistry + .forPlugin("plugin-a") + .registerNavItem({ id: "implicit", label: "Implicit", path: "/plugins/implicit" }); + pluginRegistry.forPlugin("plugin-b").registerNavItem({ + id: "explicit", + label: "Explicit", + path: "/plugins/explicit", + section: "main", + }); + + renderSection(); + + expect(screen.getByTestId("mobile-plugin-nav-item-implicit")).not.toBeNull(); + expect(screen.getByTestId("mobile-plugin-nav-item-explicit")).not.toBeNull(); + }); + + it("omits non-main sections: integrations belongs to MobileIntegrationsSection, and settings is rendered nowhere", () => { + pluginRegistry.forPlugin("plugin-a").registerNavItem({ + id: "tracker", + label: "Tracker", + path: "/plugins/tracker", + section: "integrations", + }); + pluginRegistry.forPlugin("plugin-b").registerNavItem({ + id: "prefs", + label: "Prefs", + path: "/settings/plugins/plugin-b", + section: "settings", + }); + + const { container } = renderSection(); + + expect(container.innerHTML).toBe(""); + }); + + it("renders the named curated icon, falling back to the puzzle glyph", () => { + pluginRegistry + .forPlugin("plugin-a") + .registerNavItem({ id: "hello", label: "Hello", path: HELLO_PATH, icon: "ticket" }); + pluginRegistry + .forPlugin("plugin-b") + .registerNavItem({ id: "other", label: "Other", path: "/plugins/other", icon: "nope" }); + + renderSection(); + + expect( + screen.getByTestId("mobile-plugin-nav-item-hello").querySelector("svg.tabler-icon-ticket"), + ).not.toBeNull(); + expect( + screen.getByTestId("mobile-plugin-nav-item-other").querySelector("svg.tabler-icon-puzzle"), + ).not.toBeNull(); + }); + + it("navigates to item.path and closes the menu when tapped", () => { + const onNavigate = vi.fn(); + pluginRegistry + .forPlugin("plugin-a") + .registerNavItem({ id: "hello", label: "Hello", path: HELLO_PATH }); + + renderSection(onNavigate); + screen.getByTestId("mobile-plugin-nav-item-hello").click(); + + expect(window.location.pathname).toBe(HELLO_PATH); + expect(onNavigate).toHaveBeenCalledTimes(1); + }); +}); diff --git a/apps/web/components/plugins/mobile-plugin-nav-section.tsx b/apps/web/components/plugins/mobile-plugin-nav-section.tsx new file mode 100644 index 0000000000..07cc4a73c8 --- /dev/null +++ b/apps/web/components/plugins/mobile-plugin-nav-section.tsx @@ -0,0 +1,52 @@ +"use client"; + +import { Button } from "@kandev/ui/button"; +import Link from "@/components/routing/app-link"; +import { resolvePluginIcon } from "@/lib/plugins/icons"; +import { usePluginRegistry } from "@/lib/plugins/registry"; + +type MobilePluginNavSectionProps = { + /** Closes the surrounding menu sheet once a plugin page is opened. */ + onNavigate: () => void; +}; + +/** + * Mobile counterpart to the desktop sidebar's ``: the + * hamburger-sheet surface for plugin-registered "main"-section nav items + * (`registerNavItem({ section: "main" })`, the default). The desktop rail is + * `hidden md:block`, so without this section a plugin's own page has no phone + * entry point at all. "integrations"-section items keep rendering inside + * `MobileIntegrationsSection`. + */ +export function MobilePluginNavSection({ onNavigate }: MobilePluginNavSectionProps) { + const registry = usePluginRegistry(); + const items = registry.getNavItems().filter((item) => (item.section ?? "main") === "main"); + + if (items.length === 0) return null; + + return ( +
+
Plugins
+ {items.map((item) => { + const Icon = resolvePluginIcon(item.icon); + return ( + + ); + })} +
+ ); +} diff --git a/apps/web/e2e/tests/plugins/mobile-plugin-nav.spec.ts b/apps/web/e2e/tests/plugins/mobile-plugin-nav.spec.ts new file mode 100644 index 0000000000..6b36722862 --- /dev/null +++ b/apps/web/e2e/tests/plugins/mobile-plugin-nav.spec.ts @@ -0,0 +1,115 @@ +/** + * E2E: a plugin's own page is reachable from a phone. + * + * The desktop app sidebar (which renders ``) is + * `hidden md:block`, so before `MobilePluginNavSection` existed a + * `registerNavItem({ section: "main" })` entry had no phone entry point at + * all — the route worked, but only if you typed the URL. Settings > Plugins + * was always reachable through the settings menu sheet; this covers the + * plugin's *own* nav item. + */ +import path from "node:path"; +import type { Browser, BrowserContext, Page } from "@playwright/test"; +import { expect, test } from "../../fixtures/test-base"; +import { PrAssetCapture } from "../../helpers/pr-asset-capture"; + +const PLUGIN_ID = "kandev-plugin-e2e"; +const NAV_ITEM_ID = "e2e-hello"; +const DESKTOP_VIEWPORT = { width: 1440, height: 900 }; +const PACKAGE_PATH = path.resolve( + __dirname, + "../../../../../apps/backend/.build/kandev-plugin-e2e-1.0.0.tar.gz", +); + +/** A desktop-sized client against the same backend, for the parity screenshot. */ +async function openDesktopClient( + browser: Browser, + frontendUrl: string, + backendPort: number, +): Promise<{ page: Page; context: BrowserContext }> { + const context = await browser.newContext({ + baseURL: frontendUrl, + viewport: DESKTOP_VIEWPORT, + }); + const page = await context.newPage(); + await page.addInitScript( + ({ port }: { port: number }) => { + localStorage.setItem("kandev.onboarding.completed", "true"); + window.__KANDEV_API_PORT = String(port); + }, + { port: backendPort }, + ); + return { page, context }; +} + +test.describe("Mobile plugin navigation", () => { + test.afterEach(async ({ apiClient }) => { + await apiClient.rawRequest("DELETE", `/api/plugins/${PLUGIN_ID}`).catch(() => undefined); + }); + + test("opens a plugin page from the phone menu sheet", async ({ + testPage, + backend, + browser, + }, testInfo) => { + test.setTimeout(120_000); + const capture = new PrAssetCapture(testPage, testInfo.file); + + // Settings > Plugins is reachable on a phone through the settings menu + // sheet — install the fixture through that same real flow. + await testPage.goto("/settings/plugins"); + await testPage.getByTestId("install-plugin-trigger").click(); + await testPage.getByTestId("install-plugin-tab-upload").click(); + await testPage.getByTestId("install-plugin-file-input").setInputFiles(PACKAGE_PATH); + await testPage.getByTestId("install-plugin-upload-submit").click(); + await expect(testPage.getByTestId(`plugin-row-${PLUGIN_ID}`)).toBeVisible({ timeout: 30_000 }); + + await testPage.goto("/"); + await testPage.reload(); + + // The desktop rail carries the same item but is hidden on a phone. + await expect(testPage.getByTestId("app-sidebar")).toBeHidden(); + + await testPage.getByRole("button", { name: "Open menu" }).click(); + const navItem = testPage.getByTestId(`mobile-plugin-nav-item-${NAV_ITEM_ID}`); + await expect(navItem).toBeVisible(); + await expect(navItem).toHaveText(/Hello E2E/); + expect((await navItem.boundingBox())?.height).toBeGreaterThanOrEqual(44); + + // The sheet slides up on open and the section sits below the fold, so + // settle both before shooting or the asset captures an empty mid-animation + // drawer instead of the change under test. + await testPage.getByTestId("mobile-plugin-nav-section").scrollIntoViewIfNeeded(); + await expect(navItem).toBeInViewport(); + await capture.screenshot("mobile-plugin-nav-section", { + caption: "Phone menu sheet: the plugin's page now has an entry point", + }); + + await navItem.click(); + await expect(testPage).toHaveURL(/\/plugins\/e2e-hello$/); + await expect(testPage.locator("#hello-plugin-page")).toBeVisible(); + // Tapping the item also dismisses the sheet. + await expect(testPage.getByTestId("mobile-plugin-nav-section")).toBeHidden(); + + // A plugin route with default chrome brings no nav of its own, and the + // sidebar that normally owns Home is hidden below md — so the topbar must + // carry a phone escape route or the user is stranded here. + const home = testPage.getByTestId("topbar-phone-home"); + await expect(home).toBeVisible(); + await home.click(); + await expect(testPage).toHaveURL(/\/$/); + + // Desktop parity reference: the same item in the always-visible sidebar. + const desktop = await openDesktopClient(browser, backend.frontendUrl, backend.port); + await desktop.page.goto("/"); + await desktop.page.waitForLoadState("networkidle"); + await expect(desktop.page.getByTestId(`plugin-nav-item-${NAV_ITEM_ID}`)).toBeVisible(); + await capture.screenshot("desktop-plugin-nav-sidebar", { + page: desktop.page, + caption: "Desktop, unchanged: the same nav item in the app sidebar", + }); + await desktop.context.close(); + + capture.flush(); + }); +}); diff --git a/docs/plans/plugins/PLUGIN-API.md b/docs/plans/plugins/PLUGIN-API.md index 76645e8882..e4f3360db3 100644 --- a/docs/plans/plugins/PLUGIN-API.md +++ b/docs/plans/plugins/PLUGIN-API.md @@ -129,7 +129,9 @@ interface PluginRegistry { registerRoute(path: string, Component: React.ComponentType, options?: PluginRouteOptions): void; // Sidebar/main nav entry. Rendered by in the app sidebar, - // with item.icon resolved against the curated icon map (fallback: puzzle). + // and by in the phone menu sheet (the sidebar is + // hidden below md), with item.icon resolved against the curated icon map + // (fallback: puzzle). registerNavItem(item: NavItem): void; // Route under /settings/plugins/{id}/... rendered inside settings shell. diff --git a/docs/public/plugins-authoring.md b/docs/public/plugins-authoring.md index f8c766738b..627f687c6c 100644 --- a/docs/public/plugins-authoring.md +++ b/docs/public/plugins-authoring.md @@ -337,6 +337,8 @@ interface NavItem { icon?: string; // "main" (default): top-level sidebar entry. "integrations": renders inside // the sidebar's Integrations section alongside first-party integration links. + // Both also render in the phone menu sheet, so plugin pages stay reachable + // when the desktop sidebar is hidden. // "settings" is accepted by the type but not rendered by any sidebar section // today — for a settings page use registerSettingsRoute instead. section?: "main" | "settings" | "integrations";