From 2fca8b07277663563da52e6c6d95c7616e5474b6 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 8 Jun 2026 15:23:48 +0000 Subject: [PATCH 1/3] 0.72.2 - Fix DashboardLayout sidebar links swallowing next/link navigation DashboardSidebarItemRenderer passed the consumer an onClick that called e.preventDefault(). Spec-compliant routers (next/link, React Router, TanStack Router, ...) treat a click whose default was prevented as "the handler is navigating itself, stand down", so every sidebar item became a dead link on every viewport. The only real work the handler did was collapse the sidebar. Fix: drop the preventDefault() call so the Link navigates from its href. Modifier-clicks (Cmd/Ctrl+click to open a new tab) now work too, since the router handles those itself once we stop cancelling the default. The eager setSidebarOpen(false) is now gated behind 'if (mobile)', matching useCloseDashboardSidebarOnRouteChange (which also only auto-closes on mobile). Behavior change for desktop: the persistent sidebar no longer collapses on every item click. The mobile overlay Sheet still closes immediately on tap, and also via the route-change hook once navigation lands. Added a 'NextLinkStyleNavigation' Storybook story that wires up a next/link-style Link adapter (runs the consumer onClick, then navigates only if the default was not prevented) with an on-screen navigation log and a play() assertion that the forwarded click is not defaultPrevented and navigation proceeds. Downstream note: this fix ships in @schemavaults/ui 0.72.2. Consumers that added a defensive Proxy wrapper to neutralize the library's preventDefault() (e.g. botree-workflow-discovery) can revert to a plain pass-through Link adapter once they bump to >=0.72.2. https://claude.ai/code/session_01BXzLQntYxxQsiVbRcM6jRK --- package.json | 2 +- .../DashboardLayout.stories.tsx | 176 +++++++++++++++++- .../dashboard-sidebar-item-renderer.tsx | 20 +- 3 files changed, 192 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index 24d82ae..bbae0f9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@schemavaults/ui", - "version": "0.72.1", + "version": "0.72.2", "private": false, "license": "UNLICENSED", "description": "React.js UI components for SchemaVaults frontend applications", diff --git a/src/components/layout/dashboard-layout/DashboardLayout.stories.tsx b/src/components/layout/dashboard-layout/DashboardLayout.stories.tsx index 333594b..36d4ffe 100644 --- a/src/components/layout/dashboard-layout/DashboardLayout.stories.tsx +++ b/src/components/layout/dashboard-layout/DashboardLayout.stories.tsx @@ -1,7 +1,14 @@ import type { Meta, StoryObj } from "@storybook/react"; // import { fn } from "storybook/test"; -import { useState, type ReactElement, type ReactNode } from "react"; +import { + createContext, + useCallback, + useContext, + useState, + type ReactElement, + type ReactNode, +} from "react"; import DashboardLayout, { type DashboardLayoutProps } from "./dashboard-layout"; import LoremIpsumText from "@/stories/LoremImpsumText"; @@ -33,7 +40,7 @@ import type { ICustomizableDashboardLayoutComponentProps, } from "./customizable-dashboard-component-type"; import type { LinkComponentProps, LinkComponentType } from "@/types/Link"; -import { fn } from "storybook/test"; +import { expect, fn, userEvent, waitFor, within } from "storybook/test"; function ExampleChildrenForContainer(): ReactNode { const REPEAT_TEXT: number = 25; @@ -510,3 +517,168 @@ export const WithPrintHidden: Story = { ), }; + +// --- Regression: next/link-style navigation must not be swallowed ------ +// +// DashboardSidebarItemRenderer used to call e.preventDefault() in the onClick +// it handed to the consumer . next/link (and React Router, TanStack +// Router, ...) read a prevented default as "the handler is navigating itself — +// stand down," so every sidebar link became a no-op. This story wires up a +// Link adapter that mimics next/link's contract: it runs the consumer onClick +// first and then navigates only if the event's default was not prevented. +// Clicking a sidebar item should record a navigation below; if the bug +// regresses, the adapter bails and nothing is recorded. + +interface NavigationRecord { + href: string; + defaultPrevented: boolean; +} + +// Declared at module scope so the Link component type is stable across +// renders — a fresh component identity on every render would unmount/remount +// the sidebar and re-fire its enter animations (see the note in +// dashboard-sidebar-header.tsx). The on-screen navigation log is fed through +// context instead, which re-renders the adapter without remounting it. +const RecordNavigationContext = createContext< + (record: NavigationRecord) => void +>(() => {}); + +// Spy the play() function asserts against — records every forwarded click and +// whether the consumer onClick prevented default. +const navigateSpy = fn(); + +function NextLinkStyleLink({ + href, + className, + onClick, + children, +}: LinkComponentProps): ReactElement { + const record = useContext(RecordNavigationContext); + return ( + { + // Run the consumer handler first, exactly like next/link does. + if (typeof onClick === "function") { + onClick(e); + } + const defaultPrevented: boolean = e.defaultPrevented; + navigateSpy({ href, defaultPrevented } satisfies NavigationRecord); + // next/link bails out here if the consumer prevented the default. + if (defaultPrevented) { + return; + } + // We perform client-side navigation ourselves, so stop the browser + // from doing a real full-page nav to `href` (which would navigate the + // Storybook iframe away mid-test). + e.preventDefault(); + record({ href, defaultPrevented }); + }} + > + {children} + + ); +} + +NextLinkStyleLink satisfies LinkComponentType; + +const regressionSidebarItems = [ + { + type: "dashboard-sidebar-item-group", + title: "Navigation", + items: [ + { + type: "dashboard-sidebar-item-definition", + title: "Reports", + url: "/regression/reports", + icon: ({ className }) => , + }, + { + type: "dashboard-sidebar-item-definition", + title: "Schedule", + url: "/regression/schedule", + icon: ({ className }) => , + }, + ], + }, +] satisfies DashboardSidebarItemsAndGroupsDefinitions; + +function NextLinkStyleNavigationRender( + args: Partial, +): ReactElement { + const [navigations, setNavigations] = useState([]); + const record = useCallback((rec: NavigationRecord): void => { + setNavigations((prev): string[] => [...prev, rec.href]); + }, []); + return ( + + +
+

+ Click a sidebar item. The Link adapter mimics{" "} + next/link: it runs the item onClick handler and then + navigates only when the click was not prevented. Successful + navigations are listed here; if a sidebar item ever calls{" "} + preventDefault() again, this list stays empty. +

+
    + {navigations.map((href, index) => ( +
  • + Navigated to {href} +
    • + ))} +
+
+ + + ); +} + +export const NextLinkStyleNavigation: Story = { + args: { + sidebarItems: regressionSidebarItems, + topBarTitle: "Navigation regression", + } satisfies Partial, + render: (args): ReactElement => , + play: async ({ canvasElement }): Promise => { + navigateSpy.mockClear(); + const canvas = within(canvasElement); + + // Locate the sidebar item by its href — the title label is hidden while + // the desktop sidebar is collapsed, so the accessible name is unreliable. + const reportsLink: HTMLElement = await waitFor((): HTMLElement => { + const link = canvas + .getAllByRole("link") + .find( + (el): boolean => el.getAttribute("href") === "/regression/reports", + ); + if (!link) { + throw new Error("Sidebar 'Reports' link has not rendered yet"); + } + return link; + }); + + await userEvent.click(reportsLink); + + // The consumer onClick must NOT have prevented the default, so the + // next/link-style adapter is free to navigate. + await waitFor((): void => { + expect(navigateSpy).toHaveBeenCalledWith({ + href: "/regression/reports", + defaultPrevented: false, + }); + }); + + // ...and that navigation is reflected in the on-screen log. + await waitFor((): void => { + expect(canvas.getByTestId("navigation-log")).toHaveTextContent( + "/regression/reports", + ); + }); + }, +}; diff --git a/src/components/layout/dashboard-layout/dashboard-sidebar/dashboard-sidebar-item-renderer.tsx b/src/components/layout/dashboard-layout/dashboard-sidebar/dashboard-sidebar-item-renderer.tsx index 20dac4d..3bdad49 100644 --- a/src/components/layout/dashboard-layout/dashboard-sidebar/dashboard-sidebar-item-renderer.tsx +++ b/src/components/layout/dashboard-layout/dashboard-sidebar/dashboard-sidebar-item-renderer.tsx @@ -95,15 +95,29 @@ export function DashboardSidebarItemRenderer({ "w-full h-full", "hover:bg-gray-200", )} - onClick={(e): void => { - e.preventDefault(); + onClick={(): void => { if (debug) { console.log( "[DashboardSidebarItemRenderer] onClick(item), where item = ", item, ); } - setSidebarOpen(false); + // Do NOT call e.preventDefault() here. The consumer-supplied + // navigates from its `href`, and spec-compliant routers + // (next/link, React Router, TanStack Router, ...) treat a click + // whose default was prevented as "the handler is taking over — + // don't navigate." Calling preventDefault therefore swallowed + // every sidebar navigation. Collapsing the sidebar is a side + // effect that must not cancel the click. + // + // Only eagerly collapse on mobile, where the sidebar is an + // overlay Sheet that must get out of the way after a tap. On + // desktop the sidebar is persistent chrome, so leave it open — + // matching useCloseDashboardSidebarOnRouteChange, which likewise + // only auto-closes on mobile. + if (mobile) { + setSidebarOpen(false); + } }} > Date: Mon, 8 Jun 2026 15:53:56 +0000 Subject: [PATCH 2/3] 0.72.2 - Fix navigation regression story play() link lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The NextLinkStyleNavigation play() used getByRole('link'), which failed with "Unable to find role='link'" for two reasons: 1. Each sidebar item link is rendered inside a Radix Tooltip trigger