From 0ef5a209b876f30560e972084d23d5f35a8581c3 Mon Sep 17 00:00:00 2001 From: Zohar Manor-Abel Date: Thu, 6 Aug 2026 12:34:02 +0100 Subject: [PATCH 1/2] Add `SecondaryNav` and `NavigationLayout` for contextual secondary navigation - Introduce `SecondaryNav` and `NavigationLayout` components to support a secondary, contextual navigation panel alongside the primary SidebarNav. - Extract shared `LinkProps` into types.ts so both nav components can reuse it. --- .storybook/preview.tsx | 10 +- .../navigation/NavigationLayout.stories.tsx | 230 ++++++++++ .../navigation/NavigationLayout.test.tsx | 212 +++++++++ .../navigation/NavigationLayout.tsx | 104 +++++ .../navigation/SecondaryNav.stories.tsx | 194 ++++++++ .../navigation/SecondaryNav.test.tsx | 283 ++++++++++++ src/components/navigation/SecondaryNav.tsx | 421 ++++++++++++++++++ .../navigation/SidebarNav.stories.tsx | 6 + src/components/navigation/SidebarNav.tsx | 23 +- src/components/navigation/types.ts | 18 + src/index.ts | 4 + 11 files changed, 1486 insertions(+), 19 deletions(-) create mode 100644 src/components/navigation/NavigationLayout.stories.tsx create mode 100644 src/components/navigation/NavigationLayout.test.tsx create mode 100644 src/components/navigation/NavigationLayout.tsx create mode 100644 src/components/navigation/SecondaryNav.stories.tsx create mode 100644 src/components/navigation/SecondaryNav.test.tsx create mode 100644 src/components/navigation/SecondaryNav.tsx create mode 100644 src/components/navigation/types.ts diff --git a/.storybook/preview.tsx b/.storybook/preview.tsx index 0dd0ed0d..7d1321cb 100644 --- a/.storybook/preview.tsx +++ b/.storybook/preview.tsx @@ -9,7 +9,15 @@ import { ThemeSwapper, TextLight, TextDark } from "./ThemeSwapper"; const TextThemeDiamondDS = "Theme: DiamondDS"; export const decorators = [ - (StoriesWithPadding: React.FC) => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (StoriesWithPadding: React.FC, context: any) => { + /* Fixed-position content (for example permanent Drawers) ignores this + wrapper's padding and stays aligned to the viewport, leaving a visible + gap beside padded content. + Full-page layout stories opt out with this flag. */ + if (context.parameters.fullBleed === true) { + return ; + } return (
diff --git a/src/components/navigation/NavigationLayout.stories.tsx b/src/components/navigation/NavigationLayout.stories.tsx new file mode 100644 index 00000000..16b4b6f0 --- /dev/null +++ b/src/components/navigation/NavigationLayout.stories.tsx @@ -0,0 +1,230 @@ +import { Abc, ArrowForward, GraphicEq, Menu } from "@mui/icons-material"; +import { NavigationLayout } from "./NavigationLayout"; +import { Meta, StoryObj } from "@storybook/react"; +import React from "react"; +import { + AppBar, + Box, + Divider, + IconButton, + Toolbar, + Typography, +} from "../MUI/MuiWrapped"; +import { Theme } from "@mui/material/styles"; +import { Logo } from "../controls/Logo"; +import { ColourSchemeButton } from "../controls/ColourSchemeButton"; +import { NavLink, MemoryRouter, type NavLinkProps } from "react-router-dom"; + +const meta: Meta = { + title: "Components/Navigation/NavigationLayout", + component: NavigationLayout, + decorators: [ + (Story) => ( + + + + ), + ], + tags: ["autodocs"], + parameters: { + // NavigationLayout always renders SidebarNav, which is position:fixed on + // desktop - the story canvas's default padding wrapper would otherwise + // misalign it against the normal-flow SecondaryNav/main content beside it. + fullBleed: true, + docs: { + description: { + component: `Composes SidebarNav and SecondaryNav, owning the responsive coordination between them. On mobile only one drawer is visible at a time - opening the secondary panel drills in and hides the primary sidebar, and a back affordance drills back out. On desktop both panels are shown side by side. Which primary item a secondary panel belongs to (e.g. "Setup" having its own sub-navigation) is entirely up to the consumer - NavigationLayout only owns the responsive mechanics, not when the panel opens.`, + }, + }, + }, +}; + +export default meta; +type Story = StoryObj; + +const setupGroups = [ + { + items: [ + { + id: "general", + label: "General", + linkProps: { to: "/setup/general", component: NavLink }, + }, + { + id: "devices", + label: "Devices", + linkProps: { to: "/setup/devices", component: NavLink }, + }, + { + id: "permissions", + label: "Permissions", + linkProps: { to: "/setup/permissions", component: NavLink }, + }, + ], + }, +]; + +export const WithAppBar: Story = { + render: () => { + const [sidebarOpen, setSidebarOpen] = React.useState(true); + const [secondaryNavOpen, setSecondaryNavOpen] = React.useState(false); + + // Only "Setup" has an associated secondary panel, so its link opens it and + // every other top-level link closes it - in a real app this would instead + // be derived from the current route, not from click handlers on each link. + const SetupLink = React.useMemo(() => { + const Component = React.forwardRef( + (props, ref) => ( + { + props.onClick?.(e); + setSecondaryNavOpen(true); + }} + /> + ), + ); + Component.displayName = "SetupLink"; + return Component; + }, []); + const OtherLink = React.useMemo(() => { + const Component = React.forwardRef( + (props, ref) => ( + { + props.onClick?.(e); + setSecondaryNavOpen(false); + }} + /> + ), + ); + Component.displayName = "OtherLink"; + return Component; + }, []); + + const navigation = [ + { + navItems: [ + { + label: "Setup", + icon: , + linkProps: { to: "/1", component: SetupLink }, + }, + { + label: "Acquisition", + icon: , + linkProps: { to: "/2", component: OtherLink }, + selected: true, + }, + { + label: "Analysis", + icon: , + linkProps: { to: "/3", component: OtherLink }, + }, + ], + }, + ]; + + return ( + + theme.zIndex.drawer + 1, + borderBottom: "1px solid", + borderColor: "divider", + }} + elevation={0} + > + + setSidebarOpen(!sidebarOpen)} + > + + + + + + + + + + + My app + + + + + + + + + + Main content here + + + ); + }, + parameters: { + docs: { + description: { + story: + 'Clicking "Setup" opens its secondary panel; clicking any other top-level item closes it. On a mobile viewport this drills in and replaces the sidebar, with a back arrow in the panel\'s header to drill back out. On a desktop viewport the panel appears side by side with the sidebar.', + }, + }, + }, +}; + +export const DesktopSideBySide: Story = { + args: { + navigation: [ + { + navItems: [ + { + label: "Setup", + icon: , + linkProps: { to: "/1", component: NavLink }, + selected: true, + }, + { + label: "Acquisition", + icon: , + linkProps: { to: "/2", component: NavLink }, + }, + { + label: "Analysis", + icon: , + linkProps: { to: "/3", component: NavLink }, + }, + ], + }, + ], + sidebarOpen: true, + setSidebarOpen: () => {}, + secondaryNav: { title: "Setup", groups: setupGroups }, + secondaryNavOpen: true, + setSecondaryNavOpen: () => {}, + children: Main content here, + }, +}; diff --git a/src/components/navigation/NavigationLayout.test.tsx b/src/components/navigation/NavigationLayout.test.tsx new file mode 100644 index 00000000..a9c71237 --- /dev/null +++ b/src/components/navigation/NavigationLayout.test.tsx @@ -0,0 +1,212 @@ +import { render, screen, waitFor } from "@testing-library/react"; +import { useState } from "react"; +import { NavigationLayout } from "./NavigationLayout"; +import type { Navigation } from "./SidebarNav"; +import type { SecondaryNavProps } from "./SecondaryNav"; +import { createMemoryRouter, NavLink, RouterProvider } from "react-router-dom"; +import userEvent from "@testing-library/user-event"; +import useMediaQuery from "@mui/material/useMediaQuery"; +import { addProviders } from "../../__test-utils__/helpers"; + +vi.mock("@mui/material/useMediaQuery"); + +const mockedUseMediaQuery = vi.mocked(useMediaQuery); + +const navigation: Navigation = [ + { + navItems: [ + { + label: "Setup", + icon:
, + linkProps: { component: NavLink, to: "/setup" }, + }, + ], + }, +]; + +const secondaryNav: Omit = { + title: "Secondary", + groups: [ + { + items: [ + { + id: "detail", + label: "Detail", + linkProps: { component: NavLink, to: "/detail" }, + }, + ], + }, + ], +}; + +function Harness({ + initialSidebarOpen = true, + initialSecondaryNavOpen = false, + withSecondaryNav = true, +}: { + initialSidebarOpen?: boolean; + initialSecondaryNavOpen?: boolean; + withSecondaryNav?: boolean; +}) { + const [sidebarOpen, setSidebarOpen] = useState(initialSidebarOpen); + const [secondaryNavOpen, setSecondaryNavOpen] = useState( + initialSecondaryNavOpen, + ); + + return ( + +
Main content
+ + ); +} + +function renderHarness(props: React.ComponentProps = {}) { + const router = createMemoryRouter([ + { path: "/", element: }, + ]); + render(addProviders()); +} + +describe("NavigationLayout", () => { + describe("Desktop layout", () => { + beforeEach(() => { + mockedUseMediaQuery.mockReturnValue(true); + }); + + it("renders both panels simultaneously when both are open", () => { + renderHarness({ + initialSidebarOpen: true, + initialSecondaryNavOpen: true, + }); + + expect(screen.getByRole("link", { name: "Setup" })).toBeVisible(); + expect(screen.getByRole("heading", { name: "Secondary" })).toBeVisible(); + expect(screen.getByRole("link", { name: "Detail" })).toBeVisible(); + }); + + it("renders only SidebarNav as a Drawer - the secondary panel is a plain flex sibling, not a second fixed-position Drawer", () => { + renderHarness({ + initialSidebarOpen: true, + initialSecondaryNavOpen: true, + }); + + expect(document.querySelectorAll(".MuiDrawer-root")).toHaveLength(1); + }); + + it("hides only the secondary panel when secondaryNavOpen is false", () => { + renderHarness({ + initialSidebarOpen: true, + initialSecondaryNavOpen: false, + }); + + expect(screen.getByRole("link", { name: "Setup" })).toBeVisible(); + expect(screen.queryByText("Secondary")).not.toBeVisible(); + }); + + it("renders no secondary panel when secondaryNav is omitted", () => { + renderHarness({ withSecondaryNav: false }); + + expect(screen.getByRole("link", { name: "Setup" })).toBeVisible(); + expect( + screen.queryByRole("heading", { name: "Secondary" }), + ).not.toBeInTheDocument(); + }); + }); + + describe("Mobile layout", () => { + beforeEach(() => { + mockedUseMediaQuery.mockReturnValue(false); + }); + + it("shows only the sidebar when secondary nav is not open", () => { + renderHarness({ + initialSidebarOpen: true, + initialSecondaryNavOpen: false, + }); + + expect(screen.getByText("Setup")).toBeVisible(); + expect(screen.queryByText("Secondary")).not.toBeInTheDocument(); + }); + + it("drilling into secondary nav hides the sidebar drawer", () => { + renderHarness({ + initialSidebarOpen: true, + initialSecondaryNavOpen: true, + }); + + expect(screen.queryByText("Setup")).not.toBeInTheDocument(); + expect(screen.getByText("Secondary")).toBeVisible(); + expect(screen.getByRole("link", { name: "Detail" })).toBeVisible(); + }); + + it("the back button drills back to the sidebar", async () => { + const user = userEvent.setup(); + renderHarness({ + initialSidebarOpen: true, + initialSecondaryNavOpen: true, + }); + + expect(screen.queryByText("Setup")).not.toBeInTheDocument(); + + await user.click(screen.getByRole("button", { name: "Back" })); + + await waitFor(() => { + expect(screen.queryByText("Secondary")).not.toBeInTheDocument(); + }); + expect(screen.getByText("Setup")).toBeVisible(); + }); + + it("renders no secondary panel when secondaryNav is omitted", () => { + renderHarness({ withSecondaryNav: false }); + + expect(screen.getByText("Setup")).toBeVisible(); + expect(screen.queryByText("Secondary")).not.toBeInTheDocument(); + }); + + it("the back button still reaches the sidebar even if secondary nav opened while sidebarOpen was false", async () => { + // Reproduces opening the secondary panel without the sidebar ever + // having been marked open first (e.g. deep-linking straight into it, + // or a tap landing during the sidebar's own exit transition) - + // NavigationLayout should self-heal `sidebarOpen` rather than leaving + // the back button with nothing to fall back to. + const user = userEvent.setup(); + renderHarness({ + initialSidebarOpen: false, + initialSecondaryNavOpen: true, + }); + + await waitFor(() => { + expect(screen.getByRole("button", { name: "Back" })).toBeVisible(); + }); + await user.click(screen.getByRole("button", { name: "Back" })); + + await waitFor(() => { + expect(screen.queryByText("Secondary")).not.toBeInTheDocument(); + }); + expect(screen.getByText("Setup")).toBeVisible(); + }); + + it("the device back action drills back to the sidebar instead of leaving the page", async () => { + renderHarness({ + initialSidebarOpen: true, + initialSecondaryNavOpen: true, + }); + + expect(screen.queryByText("Setup")).not.toBeInTheDocument(); + + window.dispatchEvent(new PopStateEvent("popstate")); + + await waitFor(() => { + expect(screen.queryByText("Secondary")).not.toBeInTheDocument(); + }); + expect(screen.getByText("Setup")).toBeVisible(); + }); + }); +}); diff --git a/src/components/navigation/NavigationLayout.tsx b/src/components/navigation/NavigationLayout.tsx new file mode 100644 index 00000000..536bee3a --- /dev/null +++ b/src/components/navigation/NavigationLayout.tsx @@ -0,0 +1,104 @@ +import { Box, Toolbar } from "@mui/material"; +import { useTheme } from "@mui/material/styles"; +import useMediaQuery from "@mui/material/useMediaQuery"; +import { useEffect, useRef, type ReactNode } from "react"; +import { SidebarNav, type Navigation } from "./SidebarNav"; +import { SecondaryNav, type SecondaryNavProps } from "./SecondaryNav"; + +type NavigationLayoutProps = { + navigation: Navigation; + + sidebarOpen: boolean; + setSidebarOpen: (open: boolean) => void; + + /** Omit to render primary nav only (no secondary panel at all). */ + secondaryNav?: Omit; + + /** + * Desktop: whether the secondary panel is shown side-by-side. + * Mobile: whether the view has drilled into the secondary panel. + * One flag serves both responsive roles by design - see NavigationLayout's + * derivation of `effectiveSidebarOpen` below. + */ + secondaryNavOpen: boolean; + setSecondaryNavOpen: (open: boolean) => void; + + children: ReactNode; +}; + +/** + * Composes SidebarNav and SecondaryNav, owning the responsive coordination + * between them: on mobile only one temporary drawer can be visible at a + * time, so drilling into the secondary panel implicitly hides the primary + * one, and its back affordance is a pure consequence of flipping + * `secondaryNavOpen` back to false. On desktop both panels are independent. + */ +function NavigationLayout(props: NavigationLayoutProps) { + const theme = useTheme(); + const desktopLayout = useMediaQuery(theme.breakpoints.up("sm")); + + const effectiveSidebarOpen = desktopLayout + ? props.sidebarOpen + : props.sidebarOpen && !props.secondaryNavOpen; + + // Mobile: the back affordance (device/browser back, or the panel's own + // back arrow) only has something to fall back to if `sidebarOpen` is true + // once `secondaryNavOpen` flips false again. A consumer can open the + // secondary panel without `sidebarOpen` being true yet - e.g. deep-linking + // straight into it, or a tap landing mid-exit-transition of the primary + // drawer - so drilling in self-heals that invariant rather than trusting + // the caller to have set it. + const setSidebarOpenRef = useRef(props.setSidebarOpen); + setSidebarOpenRef.current = props.setSidebarOpen; + + useEffect(() => { + if (!desktopLayout && props.secondaryNavOpen) { + setSidebarOpenRef.current(true); + } + }, [desktopLayout, props.secondaryNavOpen]); + + // Mobile: drilling into the secondary panel pushes a history entry, so the + // device/browser back action steps back to the sidebar (a popstate we + // handle ourselves) instead of leaving the page entirely. + const setSecondaryNavOpenRef = useRef(props.setSecondaryNavOpen); + setSecondaryNavOpenRef.current = props.setSecondaryNavOpen; + + useEffect(() => { + if (desktopLayout || !props.secondaryNavOpen) { + return; + } + + window.history.pushState({ secondaryNavOpen: true }, ""); + const onPopState = () => setSecondaryNavOpenRef.current(false); + window.addEventListener("popstate", onPopState); + + return () => window.removeEventListener("popstate", onPopState); + }, [desktopLayout, props.secondaryNavOpen]); + + return ( + + + {props.secondaryNav && ( + props.setSecondaryNavOpen(false) + } + /> + )} + + {/* spacer equal to the AppBar's height */} + {props.children} + + + ); +} + +export { NavigationLayout }; +export type { NavigationLayoutProps }; diff --git a/src/components/navigation/SecondaryNav.stories.tsx b/src/components/navigation/SecondaryNav.stories.tsx new file mode 100644 index 00000000..a85e60f8 --- /dev/null +++ b/src/components/navigation/SecondaryNav.stories.tsx @@ -0,0 +1,194 @@ +import { Abc, ArrowForward, GraphicEq } from "@mui/icons-material"; +import { SecondaryNav } from "./SecondaryNav"; +import { Meta, StoryObj } from "@storybook/react"; +import React from "react"; +import { NavLink, MemoryRouter } from "react-router-dom"; + +const meta: Meta = { + title: "Components/Navigation/SecondaryNav", + component: SecondaryNav, + decorators: [ + (Story) => ( + + + + ), + ], + tags: ["autodocs"], + parameters: { + docs: { + description: { + component: `An optional contextual navigation panel that sits next to SidebarNav. Mostly ListItems, optionally with a title, search, grouped sections, and one-level expandable rows. Use NavigationLayout to compose it with SidebarNav and get the responsive mobile drill-down / desktop side-by-side behaviour for free.`, + }, + }, + }, +}; + +export default meta; +type Story = StoryObj; + +const basicGroups = [ + { + items: [ + { + id: "setup", + label: "Setup", + linkProps: { to: "/1", component: NavLink }, + }, + { + id: "acquisition", + label: "Acquisition", + linkProps: { to: "/2", component: NavLink }, + selected: true, + }, + { + id: "analysis", + label: "Analysis", + linkProps: { to: "/3", component: NavLink }, + }, + ], + }, +]; + +export const Basic: Story = { + args: { + groups: basicGroups, + open: true, + setOpen: () => {}, + }, + parameters: { + docs: { + description: { + story: "dense defaults to true - rows are compact by default.", + }, + }, + }, +}; + +export const Comfortable: Story = { + args: { + groups: basicGroups, + open: true, + setOpen: () => {}, + dense: false, + }, + parameters: { + docs: { + description: { + story: "Set dense={false} for taller, more touch-friendly rows.", + }, + }, + }, +}; + +export const WithTitleAndSearch: Story = { + render: () => { + const [value, setValue] = React.useState(""); + return ( + {}} + search={{ value, onChange: setValue, placeholder: "Search items" }} + /> + ); + }, +}; + +const groupedGroups = [ + { + subheader: "Recent", + items: [ + { + id: "setup", + label: "Setup", + icon: , + linkProps: { to: "/1", component: NavLink }, + }, + { + id: "acquisition", + label: "Acquisition", + icon: , + linkProps: { to: "/2", component: NavLink }, + }, + ], + }, + { + subheader: "All experiments", + items: [ + { + id: "analysis", + label: "Analysis", + icon: , + linkProps: { to: "/3", component: NavLink }, + }, + ], + }, +]; + +export const GroupedWithSubheaders: Story = { + args: { + groups: groupedGroups, + open: true, + setOpen: () => {}, + }, +}; + +const expandableGroups = [ + { + items: [ + { + id: "analysis", + label: "Analysis", + icon: , + defaultExpanded: true, + children: [ + { id: "analysis-a", label: "Run A" }, + { id: "analysis-b", label: "Run B" }, + ], + }, + { + id: "acquisition", + label: "Acquisition", + icon: , + linkProps: { to: "/2", component: NavLink }, + children: [{ id: "acquisition-a", label: "Session 1" }], + }, + ], + }, +]; + +export const WithExpandableItems: Story = { + args: { + groups: expandableGroups, + open: true, + setOpen: () => {}, + }, + parameters: { + docs: { + description: { + story: + "One level of expand/collapse only. A row with both a link and children navigates and expands together on label click, or can be expanded on its own via the chevron. A selected item (or one with a selected child) auto-expands.", + }, + }, + }, +}; + +export const WithBackButton: Story = { + args: { + title: "Experiments", + groups: basicGroups, + open: true, + setOpen: () => {}, + onBack: () => {}, + }, + parameters: { + docs: { + description: { + story: + "onBack is normally supplied by NavigationLayout on mobile to drill back to the primary sidebar, shown here in isolation.", + }, + }, + }, +}; diff --git a/src/components/navigation/SecondaryNav.test.tsx b/src/components/navigation/SecondaryNav.test.tsx new file mode 100644 index 00000000..dc6ce4e3 --- /dev/null +++ b/src/components/navigation/SecondaryNav.test.tsx @@ -0,0 +1,283 @@ +import { render, screen } from "@testing-library/react"; +import { SecondaryNav, SecondaryNavGroup } from "./SecondaryNav"; +import { createMemoryRouter, NavLink, RouterProvider } from "react-router-dom"; +import userEvent from "@testing-library/user-event"; +import useMediaQuery from "@mui/material/useMediaQuery"; +import type { ComponentProps } from "react"; +import { addProviders } from "../../__test-utils__/helpers"; + +vi.mock("@mui/material/useMediaQuery"); + +const mockedUseMediaQuery = vi.mocked(useMediaQuery); + +describe("SecondaryNav", () => { + const groups: SecondaryNavGroup[] = [ + { + subheader: "Group one", + items: [ + { + id: "setup", + label: "Setup", + linkProps: { component: NavLink, to: "/setup" }, + }, + { + id: "acquisition", + label: "Acquisition", + linkProps: { component: NavLink, to: "/acq" }, + }, + ], + }, + { + subheader: "Group two", + items: [ + { + id: "analysis", + label: "Analysis", + children: [ + { id: "analysis-a", label: "Analysis A" }, + { id: "analysis-b", label: "Analysis B" }, + ], + }, + { + id: "expandable-link", + label: "Expandable link", + linkProps: { href: "https://www.example.com" }, + children: [{ id: "expandable-link-child", label: "Child" }], + }, + ], + }, + ]; + + function renderSecondaryNav( + props: Partial> = {}, + ) { + const setOpen = props.setOpen ?? vi.fn(); + const router = createMemoryRouter([ + { + path: "/", + element: ( + + ), + }, + ]); + render(addProviders()); + return { setOpen }; + } + + describe("Desktop layout", () => { + beforeEach(() => { + mockedUseMediaQuery.mockReturnValue(true); + }); + + it("renders grouped items with subheaders and a divider between groups", () => { + renderSecondaryNav(); + + expect(screen.getByText("Group one")).toBeVisible(); + expect(screen.getByText("Group two")).toBeVisible(); + expect(screen.getByRole("link", { name: "Setup" })).toBeVisible(); + expect(screen.getByRole("link", { name: "Acquisition" })).toBeVisible(); + expect(screen.queryByRole("separator")).toBeInTheDocument(); + }); + + it("renders a title when provided", () => { + renderSecondaryNav({ title: "Secondary" }); + expect(screen.getByRole("heading", { name: "Secondary" })).toBeVisible(); + }); + + it("dense defaults to true, applying compact row styling", () => { + renderSecondaryNav(); + expect(screen.getByRole("link", { name: "Setup" })).toHaveClass( + "MuiListItemButton-dense", + ); + }); + + it("dense can be turned off for taller rows", () => { + renderSecondaryNav({ dense: false }); + expect(screen.getByRole("link", { name: "Setup" })).not.toHaveClass( + "MuiListItemButton-dense", + ); + }); + + it("does not use a fixed-position Drawer on desktop (would overlap a sibling panel)", () => { + renderSecondaryNav(); + expect(document.querySelector(".MuiDrawer-root")).not.toBeInTheDocument(); + }); + + it("does not render a header when no header props are provided", () => { + renderSecondaryNav(); + expect(screen.queryByRole("searchbox")).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Back" }), + ).not.toBeInTheDocument(); + }); + + it("search input calls onChange and does not filter the passed-in groups itself", async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + + renderSecondaryNav({ + search: { value: "", onChange, placeholder: "Search" }, + }); + + const input = screen.getByPlaceholderText("Search"); + await user.type(input, "a"); + + expect(onChange).toHaveBeenCalledWith("a"); + // groups are rendered unfiltered regardless of search value + expect(screen.getByRole("link", { name: "Setup" })).toBeVisible(); + }); + + it("renders a back button only when onBack is provided", async () => { + const user = userEvent.setup(); + const onBack = vi.fn(); + + renderSecondaryNav({ onBack }); + + const back = screen.getByRole("button", { name: "Back" }); + expect(back).toBeVisible(); + + await user.click(back); + expect(onBack).toHaveBeenCalled(); + }); + + it("expanding an item reveals its children and toggles aria-expanded", async () => { + const user = userEvent.setup(); + renderSecondaryNav(); + + expect(screen.queryByText("Analysis A")).not.toBeInTheDocument(); + + const expandButton = screen.getByRole("button", { + name: "Expand Analysis", + }); + expect(expandButton).toHaveAttribute("aria-expanded", "false"); + + await user.click(expandButton); + + expect(screen.getByText("Analysis A")).toBeVisible(); + expect( + screen.getByRole("button", { name: "Collapse Analysis" }), + ).toHaveAttribute("aria-expanded", "true"); + }); + + it("clicking the row itself (not just the chevron) toggles a toggle-only item", async () => { + const user = userEvent.setup(); + renderSecondaryNav(); + + expect(screen.queryByText("Analysis A")).not.toBeInTheDocument(); + + // Clicking the label text, not the chevron IconButton - regression test + // for the chevron previously being nested inside the row's own button. + await user.click(screen.getByText("Analysis")); + + expect(screen.getByText("Analysis A")).toBeVisible(); + }); + + it("a row with both linkProps and children navigates and toggles together on label click", async () => { + const user = userEvent.setup(); + renderSecondaryNav(); + + const link = screen.getByRole("link", { name: "Expandable link" }); + expect(link).toHaveAttribute("href", "https://www.example.com"); + + expect(screen.queryByText("Child")).not.toBeInTheDocument(); + + await user.click(link); + expect(screen.getByText("Child")).toBeVisible(); + }); + + it("a row with both linkProps and children can also be toggled via the chevron alone", async () => { + const user = userEvent.setup(); + renderSecondaryNav(); + + expect(screen.queryByText("Child")).not.toBeInTheDocument(); + + await user.click( + screen.getByRole("button", { name: "Expand Expandable link" }), + ); + expect(screen.getByText("Child")).toBeVisible(); + }); + + it("auto-expands an item that is selected or has a selected child", () => { + renderSecondaryNav({ + groups: [ + { + items: [ + { + id: "analysis", + label: "Analysis", + children: [ + { id: "analysis-a", label: "Analysis A", selected: true }, + ], + }, + ], + }, + ], + }); + + expect(screen.getByText("Analysis A")).toBeVisible(); + }); + }); + + describe("Mobile layout", () => { + beforeEach(() => { + mockedUseMediaQuery.mockReturnValue(false); + }); + + it("renders temporary drawer with visible content when open", () => { + renderSecondaryNav({ open: true }); + + expect(document.querySelector(".MuiDrawer-root")).toBeInTheDocument(); + expect(screen.getByText("Setup")).toBeVisible(); + }); + + it("closed drawer is not visible", () => { + renderSecondaryNav({ open: false }); + expect(screen.queryByText("Setup")).not.toBeInTheDocument(); + }); + + it("clicking a nav item closes the drawer", async () => { + const user = userEvent.setup(); + const { setOpen } = renderSecondaryNav({ setOpen: vi.fn() }); + + await user.click(screen.getByRole("link", { name: "Setup" })); + + expect(setOpen).toHaveBeenCalledWith(false); + }); + + it("clicking backdrop closes the drawer", async () => { + const user = userEvent.setup(); + const { setOpen } = renderSecondaryNav({ setOpen: vi.fn() }); + + const backdrop = document.querySelector(".MuiBackdrop-root"); + expect(backdrop).toBeInTheDocument(); + + await user.click(backdrop!); + + expect(setOpen).toHaveBeenCalledWith(false); + }); + + it("expanding a toggle-only item does not close the drawer", async () => { + const user = userEvent.setup(); + const { setOpen } = renderSecondaryNav({ setOpen: vi.fn() }); + + await user.click(screen.getByRole("button", { name: "Expand Analysis" })); + + expect(screen.getByText("Analysis A")).toBeVisible(); + expect(setOpen).not.toHaveBeenCalled(); + }); + + it("clicking a row that is both a link and expandable still closes the drawer", async () => { + const user = userEvent.setup(); + const { setOpen } = renderSecondaryNav({ setOpen: vi.fn() }); + + await user.click(screen.getByRole("link", { name: "Expandable link" })); + + expect(setOpen).toHaveBeenCalledWith(false); + }); + }); +}); diff --git a/src/components/navigation/SecondaryNav.tsx b/src/components/navigation/SecondaryNav.tsx new file mode 100644 index 00000000..ad50d4fe --- /dev/null +++ b/src/components/navigation/SecondaryNav.tsx @@ -0,0 +1,421 @@ +import { + Box, + Collapse, + Divider, + Drawer, + IconButton, + InputAdornment, + List, + ListItem, + ListItemButton, + ListItemIcon, + ListItemText, + ListSubheader, + TextField, + Toolbar, + Typography, +} from "@mui/material"; +import { useTheme, Theme } from "@mui/material/styles"; +import { + Fragment, + useEffect, + useState, + type MouseEvent, + type ReactNode, +} from "react"; +import useMediaQuery from "@mui/material/useMediaQuery"; +import ArrowBackIcon from "@mui/icons-material/ArrowBack"; +import ExpandMoreIcon from "@mui/icons-material/ExpandMore"; +import SearchIcon from "@mui/icons-material/Search"; +import { drawerTransition } from "./SidebarNav"; +import type { LinkProps } from "./types"; + +const SECONDARY_NAV_WIDTH = 256; // matches SidebarNav's open-state baseline width + +type SecondaryNavGroup = { + /** Rendered as an overline ListSubheader when present; omit for an ungrouped list. */ + subheader?: string; + items: SecondaryNavItemDefinition[]; +}; + +type SecondaryNavChildItemDefinition = { + id: string; + label: string; + icon?: ReactNode; + linkProps?: LinkProps; + selected?: boolean; +}; + +type SecondaryNavItemDefinition = SecondaryNavChildItemDefinition & { + /** One level only - children cannot themselves expand. */ + children?: SecondaryNavChildItemDefinition[]; + /** Initial Collapse state for this item; uncontrolled thereafter. */ + defaultExpanded?: boolean; +}; + +type SecondaryNavProps = { + open: boolean; + setOpen: (open: boolean) => void; + + title?: string; + + search?: { + value: string; + onChange: (value: string) => void; + placeholder?: string; + }; + + groups: SecondaryNavGroup[]; + + /** + * Renders a back affordance above the title/search when provided. + * NavigationLayout supplies this on mobile only; omit for standalone/desktop use. + */ + onBack?: () => void; + + /** Compact row height/spacing, suited to longer lists. Defaults to true. */ + dense?: boolean; +}; + +function SecondaryNav(props: SecondaryNavProps) { + const theme = useTheme(); + const desktopLayout = useMediaQuery(theme.breakpoints.up("sm")); + const resolvedProps = { ...props, dense: props.dense ?? true }; + + if (desktopLayout) { + return ; + } + return ; +} + +/** + * Desktop layout: a plain flex sibling of whatever sits to its left (e.g. + * SidebarNav) - not a Drawer. MUI's Drawer paper is position:fixed regardless + * of variant, so two permanent Drawers side by side render on top of each + * other rather than beside each other; a normal Box avoids that entirely. + * Transitions between full width and fully hidden, reusing SidebarNav's + * width-transition mechanism rather than a second show/hide pattern. + */ +function SecondaryPanel(props: SecondaryNavProps) { + const width = props.open ? SECONDARY_NAV_WIDTH + 1 : 0; // +1 pixel for the border + + return ( + ({ + width, + minHeight: "100vh", + flexShrink: 0, + overflowX: "hidden", + visibility: props.open ? "visible" : "hidden", + transition: drawerTransition(theme, props.open), + bgcolor: theme.palette.surface.elevated(1), + borderRight: props.open ? "1px solid" : "none", + borderColor: "divider", + })} + > + {/* spacer equal to the AppBar's height */} + + + + + ); +} + +/** + * Small-screen layout: a temporary drawer overlayed over main content, closed + * on backdrop click or on selecting a navigable item (not on expand/collapse). + */ +function TemporarySecondaryDrawer(props: SecondaryNavProps) { + return ( + props.setOpen(false)} + onClick={() => props.setOpen(false)} + sx={{ + width: SECONDARY_NAV_WIDTH, + flexShrink: 0, + [`& .MuiDrawer-paper`]: { + width: SECONDARY_NAV_WIDTH, + boxSizing: "border-box", + backgroundImage: "none", + bgcolor: (theme: Theme) => theme.palette.surface.elevated(1), + borderRight: "1px solid", + borderColor: "divider", + }, + }} + > + + + + ); +} + +function SecondaryNavContent(props: SecondaryNavProps) { + const dense = props.dense ?? true; + + return ( + + + + + {props.groups.map((group, groupIndex) => ( + + {groupIndex > 0 && } + {group.subheader && ( + + {group.subheader} + + )} + {group.items.map((item) => ( + + ))} + + ))} + + + + ); +} + +function SecondaryNavHeader(props: SecondaryNavProps) { + const hasHeader = props.onBack || props.title || props.search; + + if (!hasHeader) { + return null; + } + + return ( + + {(props.onBack || props.title) && ( + + {props.onBack && ( + + + + )} + {props.title && ( + + {props.title} + + )} + + )} + + {props.search && ( + props.search!.onChange(e.target.value)} + placeholder={props.search.placeholder ?? "Search"} + slotProps={{ + input: { + startAdornment: ( + + + + ), + }, + }} + /> + )} + + ); +} + +function SectionDivider() { + return ( + + + + ); +} + +function getItemButtonSx(dense: boolean) { + return { + p: dense ? 0.5 : 1, + borderRadius: 2, + gap: dense ? 1 : 1.5, + "&.active, &.Mui-selected": { + bgcolor: "action.selected", + color: "primary.onContainer", + }, + }; +} + +function SecondaryNavItem({ + item, + dense, +}: { + item: SecondaryNavItemDefinition; + dense: boolean; +}) { + const hasChildren = !!item.children?.length; + const isActive = + !!item.selected || !!item.children?.some((child) => child.selected); + const [expanded, setExpanded] = useState(item.defaultExpanded ?? isActive); + // A selected item (or one with a selected child) should reveal its + // children even if it wasn't expanded to begin with - e.g. the consumer + // marks an item selected once its route becomes active. + useEffect(() => { + if (isActive) { + setExpanded(true); + } + }, [isActive]); + const toggle = () => setExpanded((value) => !value); + const toggleFromEvent = (e: MouseEvent) => { + e.stopPropagation(); + toggle(); + }; + // Toggle-only rows (no linkProps) toggle on the whole row, stopping + // propagation so it doesn't also trigger the mobile drawer's + // close-on-select. Rows that are also links toggle on click too, but let + // the click keep bubbling so navigation and the drawer's close-on-select + // still happen alongside the toggle. + const onRowClick = hasChildren + ? item.linkProps + ? toggle + : toggleFromEvent + : undefined; + + const iconSize = dense ? 28 : 32; + const buttonSx = getItemButtonSx(dense); + + return ( + <> + , and nesting one inside + // another breaks click handling and is invalid HTML. + + theme.transitions.create("transform"), + }} + > + + + ) + } + > + + {item.icon && ( + + {item.icon} + + )} + + + + {hasChildren && ( + + + {item.children!.map((child) => ( + + ))} + + + )} + + ); +} + +function SecondaryNavChildItem({ + item, + dense, +}: { + item: SecondaryNavChildItemDefinition; + dense: boolean; +}) { + const iconSize = dense ? 24 : 28; + + return ( + + + {item.icon && ( + + {item.icon} + + )} + + + + ); +} + +export { SecondaryNav }; +export type { + SecondaryNavProps, + SecondaryNavGroup, + SecondaryNavItemDefinition, + SecondaryNavChildItemDefinition, +}; diff --git a/src/components/navigation/SidebarNav.stories.tsx b/src/components/navigation/SidebarNav.stories.tsx index e72c6b52..9aaa7355 100644 --- a/src/components/navigation/SidebarNav.stories.tsx +++ b/src/components/navigation/SidebarNav.stories.tsx @@ -275,4 +275,10 @@ export const WithAppBar: Story = { ); }, + parameters: { + // SidebarNav's permanent Drawer is position:fixed on desktop - the story + // canvas's default padding wrapper would otherwise misalign it against + // the normal-flow main content beside it. + fullBleed: true, + }, }; diff --git a/src/components/navigation/SidebarNav.tsx b/src/components/navigation/SidebarNav.tsx index 327f7106..f7d1c95f 100644 --- a/src/components/navigation/SidebarNav.tsx +++ b/src/components/navigation/SidebarNav.tsx @@ -11,8 +11,9 @@ import { Tooltip, } from "@mui/material"; import { useTheme, Theme } from "@mui/material/styles"; -import { Fragment, type ElementType, type ReactNode } from "react"; +import { Fragment, type ReactNode } from "react"; import useMediaQuery from "@mui/material/useMediaQuery"; +import type { LinkProps } from "./types"; export type Navigation = NavItemGroup[]; @@ -28,23 +29,9 @@ type NavItemDefinition = { selected?: boolean; }; -type LinkProps = ExternalLinkProps | InternalLinkProps; +const getSidebarNavWidth = (open: boolean) => (open ? 257 : 65); // 256/64 + 1 pixel for the border -/** For native anchor tags */ -type ExternalLinkProps = { - href: string; - component?: never; - to?: never; -}; - -/** For SPA navigation */ -type InternalLinkProps = { - component: ElementType; - to: string; - href?: never; -}; - -const drawerTransition = (theme: Theme, opening: boolean) => { +export const drawerTransition = (theme: Theme, opening: boolean) => { return theme.transitions.create("width", { easing: opening ? theme.transitions.easing.easeIn @@ -77,7 +64,7 @@ export function SidebarNav(props: NavProps) { * Pushes main content to the right. */ function PermanentDrawer(props: NavProps) { - const width = props.open ? 257 : 65; // 256/64 + 1 pixel for the border + const width = getSidebarNavWidth(props.open); return ( Date: Tue, 11 Aug 2026 14:49:28 +0100 Subject: [PATCH 2/2] Fix `NavigationLayout` selection/mobile bugs, decouple `SecondaryNav` presentation - fixed `WithAppBar` story: selection state was hardcoded to "Acquisition" and now fixed. - Split `SecondaryNav` content from its responsive layout, making it reusable (such as in modals). - Fixed mobile navigation so selecting a secondary item closes both sidebars and shows the main content. - Updated tests and stories. --- .../navigation/NavigationLayout.stories.tsx | 123 +++++-- .../navigation/NavigationLayout.test.tsx | 48 ++- .../navigation/NavigationLayout.tsx | 103 ++++-- .../navigation/SecondaryNav.stories.tsx | 22 +- .../navigation/SecondaryNav.test.tsx | 313 ++++++++---------- src/components/navigation/SecondaryNav.tsx | 109 +----- 6 files changed, 382 insertions(+), 336 deletions(-) diff --git a/src/components/navigation/NavigationLayout.stories.tsx b/src/components/navigation/NavigationLayout.stories.tsx index 16b4b6f0..ce453649 100644 --- a/src/components/navigation/NavigationLayout.stories.tsx +++ b/src/components/navigation/NavigationLayout.stories.tsx @@ -10,7 +10,8 @@ import { Toolbar, Typography, } from "../MUI/MuiWrapped"; -import { Theme } from "@mui/material/styles"; +import { Theme, useTheme } from "@mui/material/styles"; +import useMediaQuery from "@mui/material/useMediaQuery"; import { Logo } from "../controls/Logo"; import { ColourSchemeButton } from "../controls/ColourSchemeButton"; import { NavLink, MemoryRouter, type NavLinkProps } from "react-router-dom"; @@ -66,29 +67,67 @@ const setupGroups = [ export const WithAppBar: Story = { render: () => { + const theme = useTheme(); + const desktopLayout = useMediaQuery(theme.breakpoints.up("sm")); + const [sidebarOpen, setSidebarOpen] = React.useState(true); const [secondaryNavOpen, setSecondaryNavOpen] = React.useState(false); + const [selectedItem, setSelectedItem] = React.useState< + "setup" | "acquisition" | "analysis" + >("acquisition"); - // Only "Setup" has an associated secondary panel, so its link opens it and - // every other top-level link closes it - in a real app this would instead - // be derived from the current route, not from click handlers on each link. - const SetupLink = React.useMemo(() => { - const Component = React.forwardRef( - (props, ref) => ( - { - props.onClick?.(e); - setSecondaryNavOpen(true); - }} - /> - ), - ); - Component.displayName = "SetupLink"; - return Component; - }, []); - const OtherLink = React.useMemo(() => { + // Only "Setup" has an associated secondary panel, so its link opens it + // and every other top-level link closes it. + const makeNavLink = React.useCallback( + ( + id: "setup" | "acquisition" | "analysis", + opensSecondaryNav: boolean, + ) => { + const Component = React.forwardRef( + (props, ref) => ( + { + props.onClick?.(e); + setSecondaryNavOpen(opensSecondaryNav); + setSelectedItem(id); + }} + /> + ), + ); + Component.displayName = `${id}Link`; + return Component; + }, + [], + ); + const SetupLink = React.useMemo( + () => makeNavLink("setup", true), + [makeNavLink], + ); + const AcquisitionLink = React.useMemo( + () => makeNavLink("acquisition", false), + [makeNavLink], + ); + const AnalysisLink = React.useMemo( + () => makeNavLink("analysis", false), + [makeNavLink], + ); + + // On desktop the secondary panel is persistent chrome for the active + // section, so it should always match `selectedItem` - even if it was + // closed while drilling into it on mobile (selecting "General" closes + // the mobile overlay without changing `selectedItem`). + React.useEffect(() => { + if (desktopLayout) { + setSecondaryNavOpen(selectedItem === "setup"); + } + }, [desktopLayout, selectedItem]); + + // Selecting a destination inside the secondary panel closes both panels + // on mobile, dropping all the way to main content. No-op on desktop, + // where the panel stays open side by side. + const ChildLink = React.useMemo(() => { const Component = React.forwardRef( (props, ref) => ( { props.onClick?.(e); - setSecondaryNavOpen(false); + if (!desktopLayout) { + setSecondaryNavOpen(false); + setSidebarOpen(false); + } }} /> ), ); - Component.displayName = "OtherLink"; + Component.displayName = "ChildLink"; return Component; - }, []); + }, [desktopLayout]); + + const setupGroups = React.useMemo( + () => [ + { + items: [ + { + id: "general", + label: "General", + linkProps: { to: "/setup/general", component: ChildLink }, + }, + { + id: "devices", + label: "Devices", + linkProps: { to: "/setup/devices", component: ChildLink }, + }, + { + id: "permissions", + label: "Permissions", + linkProps: { to: "/setup/permissions", component: ChildLink }, + }, + ], + }, + ], + [ChildLink], + ); const navigation = [ { @@ -112,17 +179,19 @@ export const WithAppBar: Story = { label: "Setup", icon: , linkProps: { to: "/1", component: SetupLink }, + selected: selectedItem === "setup", }, { label: "Acquisition", icon: , - linkProps: { to: "/2", component: OtherLink }, - selected: true, + linkProps: { to: "/2", component: AcquisitionLink }, + selected: selectedItem === "acquisition", }, { label: "Analysis", icon: , - linkProps: { to: "/3", component: OtherLink }, + linkProps: { to: "/3", component: AnalysisLink }, + selected: selectedItem === "analysis", }, ], }, diff --git a/src/components/navigation/NavigationLayout.test.tsx b/src/components/navigation/NavigationLayout.test.tsx index a9c71237..083e9e94 100644 --- a/src/components/navigation/NavigationLayout.test.tsx +++ b/src/components/navigation/NavigationLayout.test.tsx @@ -2,7 +2,7 @@ import { render, screen, waitFor } from "@testing-library/react"; import { useState } from "react"; import { NavigationLayout } from "./NavigationLayout"; import type { Navigation } from "./SidebarNav"; -import type { SecondaryNavProps } from "./SecondaryNav"; +import type { SecondaryNavContentProps } from "./SecondaryNav"; import { createMemoryRouter, NavLink, RouterProvider } from "react-router-dom"; import userEvent from "@testing-library/user-event"; import useMediaQuery from "@mui/material/useMediaQuery"; @@ -24,7 +24,7 @@ const navigation: Navigation = [ }, ]; -const secondaryNav: Omit = { +const secondaryNav: Omit = { title: "Secondary", groups: [ { @@ -135,7 +135,7 @@ describe("NavigationLayout", () => { expect(screen.queryByText("Secondary")).not.toBeInTheDocument(); }); - it("drilling into secondary nav hides the sidebar drawer", () => { + it("drilling into secondary nav hides the sidebar drawer and shows the secondary content in its own temporary drawer", () => { renderHarness({ initialSidebarOpen: true, initialSecondaryNavOpen: true, @@ -144,6 +144,40 @@ describe("NavigationLayout", () => { expect(screen.queryByText("Setup")).not.toBeInTheDocument(); expect(screen.getByText("Secondary")).toBeVisible(); expect(screen.getByRole("link", { name: "Detail" })).toBeVisible(); + // Only one drawer mounted at a time on mobile - the sidebar's is + // closed (and unmounted), the secondary content's is open. + expect(document.querySelectorAll(".MuiDrawer-root")).toHaveLength(1); + }); + + it("clicking a nav item inside the secondary drawer closes it", async () => { + const user = userEvent.setup(); + renderHarness({ + initialSidebarOpen: true, + initialSecondaryNavOpen: true, + }); + + await user.click(screen.getByRole("link", { name: "Detail" })); + + await waitFor(() => { + expect(screen.queryByText("Secondary")).not.toBeInTheDocument(); + }); + }); + + it("clicking the backdrop closes the secondary drawer", async () => { + const user = userEvent.setup(); + renderHarness({ + initialSidebarOpen: true, + initialSecondaryNavOpen: true, + }); + + const backdrop = document.querySelector(".MuiBackdrop-root"); + expect(backdrop).toBeInTheDocument(); + + await user.click(backdrop!); + + await waitFor(() => { + expect(screen.queryByText("Secondary")).not.toBeInTheDocument(); + }); }); it("the back button drills back to the sidebar", async () => { @@ -171,11 +205,9 @@ describe("NavigationLayout", () => { }); it("the back button still reaches the sidebar even if secondary nav opened while sidebarOpen was false", async () => { - // Reproduces opening the secondary panel without the sidebar ever - // having been marked open first (e.g. deep-linking straight into it, - // or a tap landing during the sidebar's own exit transition) - - // NavigationLayout should self-heal `sidebarOpen` rather than leaving - // the back button with nothing to fall back to. + // Opens the secondary panel without the sidebar ever having been + // marked open first (e.g. deep-linking straight into it) - exercises + // NavigationLayout's self-heal of `sidebarOpen`. const user = userEvent.setup(); renderHarness({ initialSidebarOpen: false, diff --git a/src/components/navigation/NavigationLayout.tsx b/src/components/navigation/NavigationLayout.tsx index 536bee3a..22749859 100644 --- a/src/components/navigation/NavigationLayout.tsx +++ b/src/components/navigation/NavigationLayout.tsx @@ -1,9 +1,14 @@ -import { Box, Toolbar } from "@mui/material"; +import { Box, Drawer, Toolbar } from "@mui/material"; import { useTheme } from "@mui/material/styles"; import useMediaQuery from "@mui/material/useMediaQuery"; import { useEffect, useRef, type ReactNode } from "react"; -import { SidebarNav, type Navigation } from "./SidebarNav"; -import { SecondaryNav, type SecondaryNavProps } from "./SecondaryNav"; +import { SidebarNav, drawerTransition, type Navigation } from "./SidebarNav"; +import { + SecondaryNavContent, + type SecondaryNavContentProps, +} from "./SecondaryNav"; + +const SECONDARY_NAV_WIDTH = 256; // matches SidebarNav's open-state baseline width type NavigationLayoutProps = { navigation: Navigation; @@ -12,7 +17,7 @@ type NavigationLayoutProps = { setSidebarOpen: (open: boolean) => void; /** Omit to render primary nav only (no secondary panel at all). */ - secondaryNav?: Omit; + secondaryNav?: Omit; /** * Desktop: whether the secondary panel is shown side-by-side. @@ -27,11 +32,12 @@ type NavigationLayoutProps = { }; /** - * Composes SidebarNav and SecondaryNav, owning the responsive coordination - * between them: on mobile only one temporary drawer can be visible at a - * time, so drilling into the secondary panel implicitly hides the primary - * one, and its back affordance is a pure consequence of flipping - * `secondaryNavOpen` back to false. On desktop both panels are independent. + * Composes SidebarNav with SecondaryNavContent, owning all of the responsive + * behaviour between them: on mobile only one temporary drawer can be visible + * at a time, so drilling into the secondary content implicitly hides the + * primary sidebar, and its back affordance is a pure consequence of flipping + * `secondaryNavOpen` back to false. On desktop both are shown side by side, + * the secondary content in a plain panel rather than a drawer. */ function NavigationLayout(props: NavigationLayoutProps) { const theme = useTheme(); @@ -44,10 +50,8 @@ function NavigationLayout(props: NavigationLayoutProps) { // Mobile: the back affordance (device/browser back, or the panel's own // back arrow) only has something to fall back to if `sidebarOpen` is true // once `secondaryNavOpen` flips false again. A consumer can open the - // secondary panel without `sidebarOpen` being true yet - e.g. deep-linking - // straight into it, or a tap landing mid-exit-transition of the primary - // drawer - so drilling in self-heals that invariant rather than trusting - // the caller to have set it. + // secondary panel without `sidebarOpen` set yet - e.g. deep-linking + // straight into it - so this self-heals that invariant. const setSidebarOpenRef = useRef(props.setSidebarOpen); setSidebarOpenRef.current = props.setSidebarOpen; @@ -82,16 +86,37 @@ function NavigationLayout(props: NavigationLayoutProps) { open={effectiveSidebarOpen} setOpen={props.setSidebarOpen} /> - {props.secondaryNav && ( - props.setSecondaryNavOpen(false) - } - /> - )} + {props.secondaryNav && + (desktopLayout ? ( + + + + ) : ( + props.setSecondaryNavOpen(false)} + onClick={() => props.setSecondaryNavOpen(false)} // close after making a selection + sx={{ + width: SECONDARY_NAV_WIDTH, + flexShrink: 0, + [`& .MuiDrawer-paper`]: { + width: SECONDARY_NAV_WIDTH, + boxSizing: "border-box", + backgroundImage: "none", + bgcolor: theme.palette.surface.elevated(1), + borderRight: "1px solid", + borderColor: "divider", + }, + }} + > + + props.setSecondaryNavOpen(false)} + /> + + ))} {/* spacer equal to the AppBar's height */} {props.children} @@ -100,5 +125,37 @@ function NavigationLayout(props: NavigationLayoutProps) { ); } +/** + * Desktop layout: a plain flex sibling of SidebarNav, not a Drawer - MUI's + * Drawer paper is position:fixed, so two side-by-side Drawers would render + * on top of each other. Transitions width between 0 and full, reusing + * SidebarNav's width-transition mechanism. + */ +function SecondaryNavPanel(props: { open: boolean; children: ReactNode }) { + const theme = useTheme(); + const width = props.open ? SECONDARY_NAV_WIDTH + 1 : 0; // +1 pixel for the border + + return ( + + {/* spacer equal to the AppBar's height */} + + {props.children} + + + ); +} + export { NavigationLayout }; export type { NavigationLayoutProps }; diff --git a/src/components/navigation/SecondaryNav.stories.tsx b/src/components/navigation/SecondaryNav.stories.tsx index a85e60f8..253e30a3 100644 --- a/src/components/navigation/SecondaryNav.stories.tsx +++ b/src/components/navigation/SecondaryNav.stories.tsx @@ -1,12 +1,12 @@ import { Abc, ArrowForward, GraphicEq } from "@mui/icons-material"; -import { SecondaryNav } from "./SecondaryNav"; +import { SecondaryNavContent } from "./SecondaryNav"; import { Meta, StoryObj } from "@storybook/react"; import React from "react"; import { NavLink, MemoryRouter } from "react-router-dom"; -const meta: Meta = { +const meta: Meta = { title: "Components/Navigation/SecondaryNav", - component: SecondaryNav, + component: SecondaryNavContent, decorators: [ (Story) => ( @@ -18,7 +18,7 @@ const meta: Meta = { parameters: { docs: { description: { - component: `An optional contextual navigation panel that sits next to SidebarNav. Mostly ListItems, optionally with a title, search, grouped sections, and one-level expandable rows. Use NavigationLayout to compose it with SidebarNav and get the responsive mobile drill-down / desktop side-by-side behaviour for free.`, + component: `The content of an optional contextual navigation panel that sits next to SidebarNav: a header (title/search/back) plus a grouped, optionally-expandable list. Use NavigationLayout to get the responsive mobile drill-down / desktop side-by-side drawer-or-panel behaviour around it.`, }, }, }, @@ -53,8 +53,6 @@ const basicGroups = [ export const Basic: Story = { args: { groups: basicGroups, - open: true, - setOpen: () => {}, }, parameters: { docs: { @@ -68,8 +66,6 @@ export const Basic: Story = { export const Comfortable: Story = { args: { groups: basicGroups, - open: true, - setOpen: () => {}, dense: false, }, parameters: { @@ -85,11 +81,9 @@ export const WithTitleAndSearch: Story = { render: () => { const [value, setValue] = React.useState(""); return ( - {}} search={{ value, onChange: setValue, placeholder: "Search items" }} /> ); @@ -130,8 +124,6 @@ const groupedGroups = [ export const GroupedWithSubheaders: Story = { args: { groups: groupedGroups, - open: true, - setOpen: () => {}, }, }; @@ -162,8 +154,6 @@ const expandableGroups = [ export const WithExpandableItems: Story = { args: { groups: expandableGroups, - open: true, - setOpen: () => {}, }, parameters: { docs: { @@ -179,8 +169,6 @@ export const WithBackButton: Story = { args: { title: "Experiments", groups: basicGroups, - open: true, - setOpen: () => {}, onBack: () => {}, }, parameters: { diff --git a/src/components/navigation/SecondaryNav.test.tsx b/src/components/navigation/SecondaryNav.test.tsx index dc6ce4e3..7df84c01 100644 --- a/src/components/navigation/SecondaryNav.test.tsx +++ b/src/components/navigation/SecondaryNav.test.tsx @@ -1,16 +1,11 @@ import { render, screen } from "@testing-library/react"; -import { SecondaryNav, SecondaryNavGroup } from "./SecondaryNav"; +import { SecondaryNavContent, SecondaryNavGroup } from "./SecondaryNav"; import { createMemoryRouter, NavLink, RouterProvider } from "react-router-dom"; import userEvent from "@testing-library/user-event"; -import useMediaQuery from "@mui/material/useMediaQuery"; import type { ComponentProps } from "react"; import { addProviders } from "../../__test-utils__/helpers"; -vi.mock("@mui/material/useMediaQuery"); - -const mockedUseMediaQuery = vi.mocked(useMediaQuery); - -describe("SecondaryNav", () => { +describe("SecondaryNavContent", () => { const groups: SecondaryNavGroup[] = [ { subheader: "Group one", @@ -48,236 +43,218 @@ describe("SecondaryNav", () => { }, ]; - function renderSecondaryNav( - props: Partial> = {}, + function renderSecondaryNavContent( + props: Partial> = {}, + { onOuterClick }: { onOuterClick?: () => void } = {}, ) { - const setOpen = props.setOpen ?? vi.fn(); const router = createMemoryRouter([ { path: "/", element: ( - + // The outer click handler stands in for a consumer that closes + // itself on selection (e.g. NavigationLayout's mobile drawer) - + // it's how these tests observe stopPropagation without depending + // on any particular consumer's implementation. +
+ +
), }, ]); render(addProviders()); - return { setOpen }; } - describe("Desktop layout", () => { - beforeEach(() => { - mockedUseMediaQuery.mockReturnValue(true); - }); - - it("renders grouped items with subheaders and a divider between groups", () => { - renderSecondaryNav(); + it("renders grouped items with subheaders and a divider between groups", () => { + renderSecondaryNavContent(); - expect(screen.getByText("Group one")).toBeVisible(); - expect(screen.getByText("Group two")).toBeVisible(); - expect(screen.getByRole("link", { name: "Setup" })).toBeVisible(); - expect(screen.getByRole("link", { name: "Acquisition" })).toBeVisible(); - expect(screen.queryByRole("separator")).toBeInTheDocument(); - }); - - it("renders a title when provided", () => { - renderSecondaryNav({ title: "Secondary" }); - expect(screen.getByRole("heading", { name: "Secondary" })).toBeVisible(); - }); - - it("dense defaults to true, applying compact row styling", () => { - renderSecondaryNav(); - expect(screen.getByRole("link", { name: "Setup" })).toHaveClass( - "MuiListItemButton-dense", - ); - }); + expect(screen.getByText("Group one")).toBeVisible(); + expect(screen.getByText("Group two")).toBeVisible(); + expect(screen.getByRole("link", { name: "Setup" })).toBeVisible(); + expect(screen.getByRole("link", { name: "Acquisition" })).toBeVisible(); + expect(screen.queryByRole("separator")).toBeInTheDocument(); + }); - it("dense can be turned off for taller rows", () => { - renderSecondaryNav({ dense: false }); - expect(screen.getByRole("link", { name: "Setup" })).not.toHaveClass( - "MuiListItemButton-dense", - ); - }); + it("renders a title when provided", () => { + renderSecondaryNavContent({ title: "Secondary" }); + expect(screen.getByRole("heading", { name: "Secondary" })).toBeVisible(); + }); - it("does not use a fixed-position Drawer on desktop (would overlap a sibling panel)", () => { - renderSecondaryNav(); - expect(document.querySelector(".MuiDrawer-root")).not.toBeInTheDocument(); - }); + it("dense defaults to true, applying compact row styling", () => { + renderSecondaryNavContent(); + expect(screen.getByRole("link", { name: "Setup" })).toHaveClass( + "MuiListItemButton-dense", + ); + }); - it("does not render a header when no header props are provided", () => { - renderSecondaryNav(); - expect(screen.queryByRole("searchbox")).not.toBeInTheDocument(); - expect( - screen.queryByRole("button", { name: "Back" }), - ).not.toBeInTheDocument(); - }); + it("dense can be turned off for taller rows", () => { + renderSecondaryNavContent({ dense: false }); + expect(screen.getByRole("link", { name: "Setup" })).not.toHaveClass( + "MuiListItemButton-dense", + ); + }); - it("search input calls onChange and does not filter the passed-in groups itself", async () => { - const user = userEvent.setup(); - const onChange = vi.fn(); + it("never renders a Drawer itself - it has no responsive presentation of its own", () => { + renderSecondaryNavContent(); + expect(document.querySelector(".MuiDrawer-root")).not.toBeInTheDocument(); + }); - renderSecondaryNav({ - search: { value: "", onChange, placeholder: "Search" }, - }); + it("does not render a header when no header props are provided", () => { + renderSecondaryNavContent(); + expect(screen.queryByRole("searchbox")).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Back" }), + ).not.toBeInTheDocument(); + }); - const input = screen.getByPlaceholderText("Search"); - await user.type(input, "a"); + it("search input calls onChange and does not filter the passed-in groups itself", async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); - expect(onChange).toHaveBeenCalledWith("a"); - // groups are rendered unfiltered regardless of search value - expect(screen.getByRole("link", { name: "Setup" })).toBeVisible(); + renderSecondaryNavContent({ + search: { value: "", onChange, placeholder: "Search" }, }); - it("renders a back button only when onBack is provided", async () => { - const user = userEvent.setup(); - const onBack = vi.fn(); + const input = screen.getByPlaceholderText("Search"); + await user.type(input, "a"); - renderSecondaryNav({ onBack }); + expect(onChange).toHaveBeenCalledWith("a"); + // groups are rendered unfiltered regardless of search value + expect(screen.getByRole("link", { name: "Setup" })).toBeVisible(); + }); - const back = screen.getByRole("button", { name: "Back" }); - expect(back).toBeVisible(); + it("renders a back button only when onBack is provided", async () => { + const user = userEvent.setup(); + const onBack = vi.fn(); - await user.click(back); - expect(onBack).toHaveBeenCalled(); - }); + renderSecondaryNavContent({ onBack }); - it("expanding an item reveals its children and toggles aria-expanded", async () => { - const user = userEvent.setup(); - renderSecondaryNav(); + const back = screen.getByRole("button", { name: "Back" }); + expect(back).toBeVisible(); - expect(screen.queryByText("Analysis A")).not.toBeInTheDocument(); + await user.click(back); + expect(onBack).toHaveBeenCalled(); + }); - const expandButton = screen.getByRole("button", { - name: "Expand Analysis", - }); - expect(expandButton).toHaveAttribute("aria-expanded", "false"); + it("expanding an item reveals its children and toggles aria-expanded", async () => { + const user = userEvent.setup(); + renderSecondaryNavContent(); - await user.click(expandButton); + expect(screen.queryByText("Analysis A")).not.toBeInTheDocument(); - expect(screen.getByText("Analysis A")).toBeVisible(); - expect( - screen.getByRole("button", { name: "Collapse Analysis" }), - ).toHaveAttribute("aria-expanded", "true"); + const expandButton = screen.getByRole("button", { + name: "Expand Analysis", }); + expect(expandButton).toHaveAttribute("aria-expanded", "false"); - it("clicking the row itself (not just the chevron) toggles a toggle-only item", async () => { - const user = userEvent.setup(); - renderSecondaryNav(); + await user.click(expandButton); - expect(screen.queryByText("Analysis A")).not.toBeInTheDocument(); + expect(screen.getByText("Analysis A")).toBeVisible(); + expect( + screen.getByRole("button", { name: "Collapse Analysis" }), + ).toHaveAttribute("aria-expanded", "true"); + }); - // Clicking the label text, not the chevron IconButton - regression test - // for the chevron previously being nested inside the row's own button. - await user.click(screen.getByText("Analysis")); + it("clicking the row itself (not just the chevron) toggles a toggle-only item", async () => { + const user = userEvent.setup(); + renderSecondaryNavContent(); - expect(screen.getByText("Analysis A")).toBeVisible(); - }); + expect(screen.queryByText("Analysis A")).not.toBeInTheDocument(); - it("a row with both linkProps and children navigates and toggles together on label click", async () => { - const user = userEvent.setup(); - renderSecondaryNav(); + // Clicking the label text, not the chevron IconButton - regression test + // for the chevron previously being nested inside the row's own button. + await user.click(screen.getByText("Analysis")); - const link = screen.getByRole("link", { name: "Expandable link" }); - expect(link).toHaveAttribute("href", "https://www.example.com"); + expect(screen.getByText("Analysis A")).toBeVisible(); + }); - expect(screen.queryByText("Child")).not.toBeInTheDocument(); + it("a row with both linkProps and children navigates and toggles together on label click", async () => { + const user = userEvent.setup(); + renderSecondaryNavContent(); - await user.click(link); - expect(screen.getByText("Child")).toBeVisible(); - }); + const link = screen.getByRole("link", { name: "Expandable link" }); + expect(link).toHaveAttribute("href", "https://www.example.com"); - it("a row with both linkProps and children can also be toggled via the chevron alone", async () => { - const user = userEvent.setup(); - renderSecondaryNav(); + expect(screen.queryByText("Child")).not.toBeInTheDocument(); - expect(screen.queryByText("Child")).not.toBeInTheDocument(); + await user.click(link); + expect(screen.getByText("Child")).toBeVisible(); + }); - await user.click( - screen.getByRole("button", { name: "Expand Expandable link" }), - ); - expect(screen.getByText("Child")).toBeVisible(); - }); + it("a row with both linkProps and children can also be toggled via the chevron alone", async () => { + const user = userEvent.setup(); + renderSecondaryNavContent(); - it("auto-expands an item that is selected or has a selected child", () => { - renderSecondaryNav({ - groups: [ - { - items: [ - { - id: "analysis", - label: "Analysis", - children: [ - { id: "analysis-a", label: "Analysis A", selected: true }, - ], - }, - ], - }, - ], - }); + expect(screen.queryByText("Child")).not.toBeInTheDocument(); - expect(screen.getByText("Analysis A")).toBeVisible(); - }); + await user.click( + screen.getByRole("button", { name: "Expand Expandable link" }), + ); + expect(screen.getByText("Child")).toBeVisible(); }); - describe("Mobile layout", () => { - beforeEach(() => { - mockedUseMediaQuery.mockReturnValue(false); - }); - - it("renders temporary drawer with visible content when open", () => { - renderSecondaryNav({ open: true }); - - expect(document.querySelector(".MuiDrawer-root")).toBeInTheDocument(); - expect(screen.getByText("Setup")).toBeVisible(); + it("auto-expands an item that is selected or has a selected child", () => { + renderSecondaryNavContent({ + groups: [ + { + items: [ + { + id: "analysis", + label: "Analysis", + children: [ + { id: "analysis-a", label: "Analysis A", selected: true }, + ], + }, + ], + }, + ], }); - it("closed drawer is not visible", () => { - renderSecondaryNav({ open: false }); - expect(screen.queryByText("Setup")).not.toBeInTheDocument(); - }); + expect(screen.getByText("Analysis A")).toBeVisible(); + }); - it("clicking a nav item closes the drawer", async () => { + // A consumer (e.g. NavigationLayout's mobile drawer) may close itself on + // any click that bubbles out - these confirm which rows let that happen + // and which stop it, independent of any particular consumer. + describe("click propagation", () => { + it("a plain link row's click bubbles up to an ancestor", async () => { const user = userEvent.setup(); - const { setOpen } = renderSecondaryNav({ setOpen: vi.fn() }); + const onOuterClick = vi.fn(); + renderSecondaryNavContent({}, { onOuterClick }); await user.click(screen.getByRole("link", { name: "Setup" })); - expect(setOpen).toHaveBeenCalledWith(false); + expect(onOuterClick).toHaveBeenCalled(); }); - it("clicking backdrop closes the drawer", async () => { + it("a toggle-only row's click does not bubble up to an ancestor", async () => { const user = userEvent.setup(); - const { setOpen } = renderSecondaryNav({ setOpen: vi.fn() }); - - const backdrop = document.querySelector(".MuiBackdrop-root"); - expect(backdrop).toBeInTheDocument(); + const onOuterClick = vi.fn(); + renderSecondaryNavContent({}, { onOuterClick }); - await user.click(backdrop!); + await user.click(screen.getByText("Analysis")); - expect(setOpen).toHaveBeenCalledWith(false); + expect(screen.getByText("Analysis A")).toBeVisible(); + expect(onOuterClick).not.toHaveBeenCalled(); }); - it("expanding a toggle-only item does not close the drawer", async () => { + it("expanding via the chevron alone does not bubble up to an ancestor", async () => { const user = userEvent.setup(); - const { setOpen } = renderSecondaryNav({ setOpen: vi.fn() }); + const onOuterClick = vi.fn(); + renderSecondaryNavContent({}, { onOuterClick }); await user.click(screen.getByRole("button", { name: "Expand Analysis" })); - expect(screen.getByText("Analysis A")).toBeVisible(); - expect(setOpen).not.toHaveBeenCalled(); + expect(onOuterClick).not.toHaveBeenCalled(); }); - it("clicking a row that is both a link and expandable still closes the drawer", async () => { + it("a row that is both a link and expandable still bubbles up on click", async () => { const user = userEvent.setup(); - const { setOpen } = renderSecondaryNav({ setOpen: vi.fn() }); + const onOuterClick = vi.fn(); + renderSecondaryNavContent({}, { onOuterClick }); await user.click(screen.getByRole("link", { name: "Expandable link" })); - expect(setOpen).toHaveBeenCalledWith(false); + expect(onOuterClick).toHaveBeenCalled(); }); }); }); diff --git a/src/components/navigation/SecondaryNav.tsx b/src/components/navigation/SecondaryNav.tsx index ad50d4fe..0efca1f4 100644 --- a/src/components/navigation/SecondaryNav.tsx +++ b/src/components/navigation/SecondaryNav.tsx @@ -2,7 +2,6 @@ import { Box, Collapse, Divider, - Drawer, IconButton, InputAdornment, List, @@ -12,10 +11,9 @@ import { ListItemText, ListSubheader, TextField, - Toolbar, Typography, } from "@mui/material"; -import { useTheme, Theme } from "@mui/material/styles"; +import { Theme } from "@mui/material/styles"; import { Fragment, useEffect, @@ -23,15 +21,11 @@ import { type MouseEvent, type ReactNode, } from "react"; -import useMediaQuery from "@mui/material/useMediaQuery"; import ArrowBackIcon from "@mui/icons-material/ArrowBack"; import ExpandMoreIcon from "@mui/icons-material/ExpandMore"; import SearchIcon from "@mui/icons-material/Search"; -import { drawerTransition } from "./SidebarNav"; import type { LinkProps } from "./types"; -const SECONDARY_NAV_WIDTH = 256; // matches SidebarNav's open-state baseline width - type SecondaryNavGroup = { /** Rendered as an overline ListSubheader when present; omit for an ungrouped list. */ subheader?: string; @@ -53,10 +47,7 @@ type SecondaryNavItemDefinition = SecondaryNavChildItemDefinition & { defaultExpanded?: boolean; }; -type SecondaryNavProps = { - open: boolean; - setOpen: (open: boolean) => void; - +type SecondaryNavContentProps = { title?: string; search?: { @@ -69,7 +60,7 @@ type SecondaryNavProps = { /** * Renders a back affordance above the title/search when provided. - * NavigationLayout supplies this on mobile only; omit for standalone/desktop use. + * NavigationLayout supplies this on mobile only; omit for standalone use. */ onBack?: () => void; @@ -77,81 +68,13 @@ type SecondaryNavProps = { dense?: boolean; }; -function SecondaryNav(props: SecondaryNavProps) { - const theme = useTheme(); - const desktopLayout = useMediaQuery(theme.breakpoints.up("sm")); - const resolvedProps = { ...props, dense: props.dense ?? true }; - - if (desktopLayout) { - return ; - } - return ; -} - /** - * Desktop layout: a plain flex sibling of whatever sits to its left (e.g. - * SidebarNav) - not a Drawer. MUI's Drawer paper is position:fixed regardless - * of variant, so two permanent Drawers side by side render on top of each - * other rather than beside each other; a normal Box avoids that entirely. - * Transitions between full width and fully hidden, reusing SidebarNav's - * width-transition mechanism rather than a second show/hide pattern. + * Just the contextual nav's content - a header (title/search/back) plus a + * grouped, optionally-expandable list. Presentation (Drawer vs. side-by-side + * panel, responsive switching) is NavigationLayout's job, not this + * component's. */ -function SecondaryPanel(props: SecondaryNavProps) { - const width = props.open ? SECONDARY_NAV_WIDTH + 1 : 0; // +1 pixel for the border - - return ( - ({ - width, - minHeight: "100vh", - flexShrink: 0, - overflowX: "hidden", - visibility: props.open ? "visible" : "hidden", - transition: drawerTransition(theme, props.open), - bgcolor: theme.palette.surface.elevated(1), - borderRight: props.open ? "1px solid" : "none", - borderColor: "divider", - })} - > - {/* spacer equal to the AppBar's height */} - - - - - ); -} - -/** - * Small-screen layout: a temporary drawer overlayed over main content, closed - * on backdrop click or on selecting a navigable item (not on expand/collapse). - */ -function TemporarySecondaryDrawer(props: SecondaryNavProps) { - return ( - props.setOpen(false)} - onClick={() => props.setOpen(false)} - sx={{ - width: SECONDARY_NAV_WIDTH, - flexShrink: 0, - [`& .MuiDrawer-paper`]: { - width: SECONDARY_NAV_WIDTH, - boxSizing: "border-box", - backgroundImage: "none", - bgcolor: (theme: Theme) => theme.palette.surface.elevated(1), - borderRight: "1px solid", - borderColor: "divider", - }, - }} - > - - - - ); -} - -function SecondaryNavContent(props: SecondaryNavProps) { +function SecondaryNavContent(props: SecondaryNavContentProps) { const dense = props.dense ?? true; return ( @@ -186,7 +109,7 @@ function SecondaryNavContent(props: SecondaryNavProps) { ); } -function SecondaryNavHeader(props: SecondaryNavProps) { +function SecondaryNavHeader(props: SecondaryNavContentProps) { const hasHeader = props.onBack || props.title || props.search; if (!hasHeader) { @@ -288,11 +211,11 @@ function SecondaryNavItem({ e.stopPropagation(); toggle(); }; - // Toggle-only rows (no linkProps) toggle on the whole row, stopping - // propagation so it doesn't also trigger the mobile drawer's - // close-on-select. Rows that are also links toggle on click too, but let - // the click keep bubbling so navigation and the drawer's close-on-select - // still happen alongside the toggle. + // Toggle-only rows (no linkProps) toggle on the whole row and stop + // propagation, so a consumer wrapping this in a closable container (e.g. + // NavigationLayout's mobile drawer) doesn't treat expand/collapse as a + // selection. Rows that are also links toggle on click too, but let it + // keep bubbling so navigation and close-on-select still happen. const onRowClick = hasChildren ? item.linkProps ? toggle @@ -412,9 +335,9 @@ function SecondaryNavChildItem({ ); } -export { SecondaryNav }; +export { SecondaryNavContent }; export type { - SecondaryNavProps, + SecondaryNavContentProps, SecondaryNavGroup, SecondaryNavItemDefinition, SecondaryNavChildItemDefinition,