From 5111b0bd7ee77133d7a81d2ddcc6660494b12dca Mon Sep 17 00:00:00 2001 From: Zohar Manor-Abel Date: Thu, 6 Aug 2026 12:34:02 +0100 Subject: [PATCH 1/5] 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 15649fae..cdf83009 100644 --- a/src/components/navigation/SidebarNav.stories.tsx +++ b/src/components/navigation/SidebarNav.stories.tsx @@ -395,4 +395,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 2e4ad2e8..dcd6e13e 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 @@ -81,7 +68,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/5] 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, From 8ee64829d4e4e31b91c2998db0e9f40b5c89aeac Mon Sep 17 00:00:00 2001 From: Zohar Manor-Abel Date: Tue, 11 Aug 2026 16:22:30 +0100 Subject: [PATCH 3/5] fixed how `NavigationLayout` looks and works on the docs page --- .../navigation/NavigationLayout.stories.tsx | 70 ++++++++++++------- 1 file changed, 43 insertions(+), 27 deletions(-) diff --git a/src/components/navigation/NavigationLayout.stories.tsx b/src/components/navigation/NavigationLayout.stories.tsx index ce453649..1fb38e1c 100644 --- a/src/components/navigation/NavigationLayout.stories.tsx +++ b/src/components/navigation/NavigationLayout.stories.tsx @@ -36,6 +36,10 @@ const meta: Meta = { 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.`, }, + // Isolates each story in its own iframe, since autodocs otherwise + // embeds them inline in one document and their position:fixed + // SidebarNavs stack on top of each other. + story: { inline: false, iframeHeight: 600 }, }, }, }; @@ -265,35 +269,47 @@ export const WithAppBar: Story = { }, }; -export const DesktopSideBySide: Story = { - args: { - navigation: [ +const desktopSideBySideNavigation = [ + { + navItems: [ { - 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 }, - }, - ], + 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, + }, +]; + +export const DesktopSideBySide: Story = { + // Real state so the drawers stay closeable if autodocs' real browser + // viewport narrows this below desktop width. + render: () => { + const [sidebarOpen, setSidebarOpen] = React.useState(true); + const [secondaryNavOpen, setSecondaryNavOpen] = React.useState(true); + + return ( + + Main content here + + ); }, }; From 722267b8add39a20e7c740d86d1853ee91c842f9 Mon Sep 17 00:00:00 2001 From: Zohar Manor-Abel Date: Mon, 24 Aug 2026 13:54:28 +0100 Subject: [PATCH 4/5] Address PR #262 SidebarNav review feedback - Move responsive primary/secondary panel coordination into `SidebarNav` - Rename the secondary panel component to `SubNav` - Consolidate shared nav item types and use generated keys instead of `id` - Replace the `search` prop with `searchSlot` - Improve `SidebarNav` story coverage and clean up story canvases - Update test assertions - Fix drawer/content height mismatch --- .storybook/storybook.css | 10 + .../navigation/NavigationLayout.stories.tsx | 315 ---------- .../navigation/NavigationLayout.test.tsx | 244 -------- .../navigation/NavigationLayout.tsx | 161 ----- .../navigation/SidebarNav.stories.tsx | 383 ++++++++++-- src/components/navigation/SidebarNav.test.tsx | 576 ++++++++++++------ src/components/navigation/SidebarNav.tsx | 215 ++++++- ...daryNav.stories.tsx => SubNav.stories.tsx} | 59 +- ...{SecondaryNav.test.tsx => SubNav.test.tsx} | 123 ++-- .../{SecondaryNav.tsx => SubNav.tsx} | 102 +--- src/components/navigation/types.ts | 21 +- src/index.ts | 3 +- 12 files changed, 1087 insertions(+), 1125 deletions(-) delete mode 100644 src/components/navigation/NavigationLayout.stories.tsx delete mode 100644 src/components/navigation/NavigationLayout.test.tsx delete mode 100644 src/components/navigation/NavigationLayout.tsx rename src/components/navigation/{SecondaryNav.stories.tsx => SubNav.stories.tsx} (65%) rename src/components/navigation/{SecondaryNav.test.tsx => SubNav.test.tsx} (62%) rename src/components/navigation/{SecondaryNav.tsx => SubNav.tsx} (73%) diff --git a/.storybook/storybook.css b/.storybook/storybook.css index 03cbfd65..4a701853 100644 --- a/.storybook/storybook.css +++ b/.storybook/storybook.css @@ -1,3 +1,13 @@ +/* Gives the plain story canvas (not the Docs page, which sets its own + fixed height per docs.story.height) a definite height, so a story's + `minHeight: "100%"` resolves against the real viewport instead of + collapsing to its content's height. */ +html, +body, +#storybook-root { + height: 100%; +} + :root { --sb-ds-background: #f6f6f9; --sb-ds-surface: #ffffff; diff --git a/src/components/navigation/NavigationLayout.stories.tsx b/src/components/navigation/NavigationLayout.stories.tsx deleted file mode 100644 index 1fb38e1c..00000000 --- a/src/components/navigation/NavigationLayout.stories.tsx +++ /dev/null @@ -1,315 +0,0 @@ -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, 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"; - -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.`, - }, - // Isolates each story in its own iframe, since autodocs otherwise - // embeds them inline in one document and their position:fixed - // SidebarNavs stack on top of each other. - story: { inline: false, iframeHeight: 600 }, - }, - }, -}; - -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 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. - 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); - if (!desktopLayout) { - setSecondaryNavOpen(false); - setSidebarOpen(false); - } - }} - /> - ), - ); - 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 = [ - { - navItems: [ - { - label: "Setup", - icon: , - linkProps: { to: "/1", component: SetupLink }, - selected: selectedItem === "setup", - }, - { - label: "Acquisition", - icon: , - linkProps: { to: "/2", component: AcquisitionLink }, - selected: selectedItem === "acquisition", - }, - { - label: "Analysis", - icon: , - linkProps: { to: "/3", component: AnalysisLink }, - selected: selectedItem === "analysis", - }, - ], - }, - ]; - - 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.', - }, - }, - }, -}; - -const desktopSideBySideNavigation = [ - { - 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 }, - }, - ], - }, -]; - -export const DesktopSideBySide: Story = { - // Real state so the drawers stay closeable if autodocs' real browser - // viewport narrows this below desktop width. - render: () => { - const [sidebarOpen, setSidebarOpen] = React.useState(true); - const [secondaryNavOpen, setSecondaryNavOpen] = React.useState(true); - - return ( - - Main content here - - ); - }, -}; diff --git a/src/components/navigation/NavigationLayout.test.tsx b/src/components/navigation/NavigationLayout.test.tsx deleted file mode 100644 index 083e9e94..00000000 --- a/src/components/navigation/NavigationLayout.test.tsx +++ /dev/null @@ -1,244 +0,0 @@ -import { render, screen, waitFor } from "@testing-library/react"; -import { useState } from "react"; -import { NavigationLayout } from "./NavigationLayout"; -import type { Navigation } from "./SidebarNav"; -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"; -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 and shows the secondary content in its own temporary drawer", () => { - renderHarness({ - initialSidebarOpen: true, - initialSecondaryNavOpen: true, - }); - - 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 () => { - 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 () => { - // 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, - 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 deleted file mode 100644 index 22749859..00000000 --- a/src/components/navigation/NavigationLayout.tsx +++ /dev/null @@ -1,161 +0,0 @@ -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, 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; - - 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 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(); - 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` set yet - e.g. deep-linking - // straight into it - so this self-heals that invariant. - 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 && - (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} - - - ); -} - -/** - * 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/SidebarNav.stories.tsx b/src/components/navigation/SidebarNav.stories.tsx index cdf83009..096c4457 100644 --- a/src/components/navigation/SidebarNav.stories.tsx +++ b/src/components/navigation/SidebarNav.stories.tsx @@ -25,10 +25,11 @@ import { ListItemIcon, ListItemText, } from "@mui/material"; -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 } from "react-router-dom"; +import { NavLink, MemoryRouter, type NavLinkProps } from "react-router-dom"; const meta: Meta = { title: "Components/Navigation/SidebarNav", @@ -42,15 +43,13 @@ const meta: Meta = { ], tags: ["autodocs"], 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 content beside it. + fullBleed: true, docs: { - disable: true, - pages: {}, description: { - component: ` -A collapsing/expanding sidebar for your app's primary navigation. - -For normal screen sizes, the implementation uses MUI's permanent drawer toggling between two widths showing either icon and text or just icon. -For smaller screens, we use the temporary variant instead.`, + component: `Your app's primary navigation, with an optional contextual secondary panel alongside it. Without \`subNav\` this renders the collapsing/expanding primary drawer alone - a permanent drawer toggling between two widths (icon and text, or just icon) on normal screen sizes, and a temporary (overlaid) drawer on smaller screens. With \`subNav\`, SidebarNav also owns the responsive coordination between the two panels: 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 - SidebarNav only owns the responsive mechanics, not when the panel opens.`, }, story: { height: "600px", @@ -89,16 +88,17 @@ export const NormalLinks: Story = { render: (_args) => { const [open, setOpen] = React.useState(true); return ( - + setOpen(!open)}> - - When using standard links, the caller must handle the selected state - and set it to the correct item. - + + + + Main content here + ); @@ -106,6 +106,14 @@ export const NormalLinks: Story = { args: { navigation: standardLinks, }, + parameters: { + docs: { + description: { + story: + "When using standard links, the caller must handle the selected state and set it to the correct item.", + }, + }, + }, }; const reactRouterNavigation = [ @@ -143,7 +151,7 @@ export const RouterLinks: Story = { render: (_args) => { const [open, setOpen] = React.useState(false); return ( - + setOpen(!open)}> - - React Router NavLinks will handle selected state - internally. - + + + + Main content here + ); @@ -164,6 +173,13 @@ export const RouterLinks: Story = { args: { navigation: reactRouterNavigation, }, + parameters: { + docs: { + description: { + story: "React Router NavLinks will handle selected state internally.", + }, + }, + }, }; const groupedNavigation = [ @@ -210,7 +226,7 @@ export const GroupedNavigation: Story = { render: (_args) => { const [open, setOpen] = React.useState(true); return ( - + setOpen(!open)}> - Sections are grouped with dividers. + + + + Main content here + ); }, + parameters: { + docs: { + description: { + story: "Sections are grouped with dividers.", + }, + }, + }, }; /** A dashed, tinted wrapper so it's obvious in the story which content is coming from a slot vs. the `navigation` prop. */ @@ -265,7 +292,7 @@ export const WithSlots: Story = { render: (_args) => { const [open, setOpen] = React.useState(true); return ( - + setOpen(!open)}> - - Adds slots to the navbar, boxes are only there to highlight what - each slot renders, they aren't part of the component.{" "} - afterNavSlot renders inside the scrollable area, right - after the navigation items. footerSlot is pinned to the - bottom of the drawer, outside the scroll area. - + + + + Main content here + ); }, + parameters: { + docs: { + description: { + story: + "The dashed boxes are only there to highlight what each slot renders - they aren't part of the component. afterNavSlot renders inside the scrollable area, right after the navigation items. footerSlot is pinned to the bottom of the drawer, outside the scroll area.", + }, + }, + }, }; export const WithAppBar: Story = { render: (_args) => { const [open, setOpen] = React.useState(true); return ( - + - - - - MUI wants to draw a Drawer above everything, so in this example the - AppBar's zIndex is increased. - + + {/* spacer equal to the AppBar's height */} + + Main content here + ); }, 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, + docs: { + description: { + story: + "MUI wants to draw a Drawer above everything, so in this example the AppBar's zIndex is increased. Clicking the menu icon toggles the sidebar open and closed.", + }, + }, + }, +}; + +const setupGroups = [ + { + items: [ + { + label: "General", + linkProps: { to: "/setup/general", component: NavLink }, + }, + { + label: "Devices", + linkProps: { to: "/setup/devices", component: NavLink }, + }, + { + label: "Permissions", + linkProps: { to: "/setup/permissions", component: NavLink }, + }, + ], + }, +]; + +export const WithAppBarAndSubNav: Story = { + render: () => { + const theme = useTheme(); + const desktopLayout = useMediaQuery(theme.breakpoints.up("sm")); + + const [sidebarOpen, setSidebarOpen] = React.useState(true); + const [subNavOpen, setSubNavOpen] = 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. + const makeNavLink = React.useCallback( + (id: "setup" | "acquisition" | "analysis", opensSubNav: boolean) => { + const Component = React.forwardRef( + (props, ref) => ( + { + props.onClick?.(e); + setSubNavOpen(opensSubNav); + 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) { + setSubNavOpen(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); + if (!desktopLayout) { + setSubNavOpen(false); + setSidebarOpen(false); + } + }} + /> + ), + ); + Component.displayName = "ChildLink"; + return Component; + }, [desktopLayout]); + + const setupGroupsWithChildLinks = React.useMemo( + () => [ + { + items: [ + { + label: "General", + linkProps: { to: "/setup/general", component: ChildLink }, + }, + { + label: "Devices", + linkProps: { to: "/setup/devices", component: ChildLink }, + }, + { + label: "Permissions", + linkProps: { to: "/setup/permissions", component: ChildLink }, + }, + ], + }, + ], + [ChildLink], + ); + + const navigation = [ + { + navItems: [ + { + label: "Setup", + icon: , + linkProps: { to: "/1", component: SetupLink }, + selected: selectedItem === "setup", + }, + { + label: "Acquisition", + icon: , + linkProps: { to: "/2", component: AcquisitionLink }, + selected: selectedItem === "acquisition", + }, + { + label: "Analysis", + icon: , + linkProps: { to: "/3", component: AnalysisLink }, + selected: selectedItem === "analysis", + }, + ], + }, + ]; + + return ( + + theme.zIndex.drawer + 1, + borderBottom: "1px solid", + borderColor: "divider", + }} + elevation={0} + > + + setSidebarOpen(!sidebarOpen)} + > + + + + + + + + + + + My app + + + + + + + + + + + + {/* spacer equal to the AppBar's height */} + + 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.', + }, + }, + }, +}; + +const desktopSideBySideNavigation = [ + { + 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 }, + }, + ], + }, +]; + +export const DesktopSideBySide: Story = { + // Real state so the drawers stay closeable if autodocs' real browser + // viewport narrows this below desktop width. + render: () => { + const [sidebarOpen, setSidebarOpen] = React.useState(true); + const [subNavOpen, setSubNavOpen] = React.useState(true); + + return ( + + + + {/* spacer equal to the AppBar's height */} + + Main content here + + + + ); }, }; diff --git a/src/components/navigation/SidebarNav.test.tsx b/src/components/navigation/SidebarNav.test.tsx index a282b333..2c1cf55f 100644 --- a/src/components/navigation/SidebarNav.test.tsx +++ b/src/components/navigation/SidebarNav.test.tsx @@ -1,232 +1,464 @@ -import { render, screen } from "@testing-library/react"; -import { Navigation, SidebarNav } from "./SidebarNav"; +import { render, screen, waitFor } from "@testing-library/react"; +import { useState } from "react"; +import { SidebarNav, type Navigation } from "./SidebarNav"; +import type { SubNavContentProps } from "./SubNav"; 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); describe("SidebarNav", () => { - const navigation: Navigation = [ - { - navItems: [ - { - label: "Setup", - icon:
, - linkProps: { component: NavLink, to: "/setup" }, - }, - { - label: "Acquisition", - icon:
, - linkProps: { component: NavLink, to: "/acq" }, - }, - { - label: "Analysis", - icon:
, - linkProps: { component: NavLink, to: "/analysis" }, - }, - ], - }, - { - navItems: [ + describe("primary nav only (no subNav)", () => { + const navigation: Navigation = [ + { + navItems: [ + { + label: "Setup", + icon:
, + linkProps: { component: NavLink, to: "/setup" }, + }, + { + label: "Acquisition", + icon:
, + linkProps: { component: NavLink, to: "/acq" }, + }, + { + label: "Analysis", + icon:
, + linkProps: { component: NavLink, to: "/analysis" }, + }, + ], + }, + { + navItems: [ + { + label: "Organisation", + icon:
, + linkProps: { href: "https://www.example.com" }, + }, + ], + }, + ]; + + function renderSidenav(open: boolean, setOpen = vi.fn()) { + const router = createMemoryRouter([ { - label: "Organisation", - icon:
, - linkProps: { href: "https://www.example.com" }, + path: "/", + element: ( + + ), }, - ], - }, - ]; + ]); + render(); + } - function renderSidenav(open: boolean, setOpen = vi.fn()) { - const router = createMemoryRouter([ - { - path: "/", - element: ( - - ), - }, - ]); - render(); - } + describe("Desktop layout", () => { + beforeEach(() => { + mockedUseMediaQuery.mockReturnValue(true); + }); - describe("Desktop layout", () => { - beforeEach(() => { - mockedUseMediaQuery.mockReturnValue(true); - }); + it("shows icons and names when open", () => { + renderSidenav(true); - it("Shows icons and names when open", () => { - renderSidenav(true); + const items = navigation[0].navItems; - const items = navigation[0].navItems; + items.forEach((item) => { + const button = screen.getByRole("link", { name: item.label }); + expect(button).toBeVisible(); + const label = screen.getByText(item.label); + expect(label).toBeVisible(); + }); + ["navicon1", "navicon2", "navicon3", "navicon4"].forEach((id) => + expect(screen.getByTestId(id)).toBeVisible(), + ); + }); - items.forEach((item) => { - const button = screen.getByRole("link", { name: item.label }); - expect(button).toBeVisible(); - const label = screen.getByText(item.label); - expect(label).toBeVisible(); + it("shows icons only when closed", () => { + renderSidenav(false); + const items = navigation[0].navItems; + items.forEach((item) => { + const button = screen.getByRole("link", { name: item.label }); + expect(button).toBeVisible(); // a11y-wise still visible + const label = screen.getByText(item.label); + expect(label).toBeInTheDocument(); // label exists but + expect(label).not.toBeVisible(); // not visible + }); + ["navicon1", "navicon2", "navicon3", "navicon4"].forEach((id) => + expect(screen.getByTestId(id)).toBeVisible(), + ); }); - ["navicon1", "navicon2", "navicon3", "navicon4"].forEach((id) => - expect(screen.getByTestId(id)).toBeVisible(), - ); - }); - it("Shows icons only when closed", () => { - renderSidenav(false); - const items = navigation[0].navItems; - items.forEach((item) => { - const button = screen.getByRole("link", { name: item.label }); - expect(button).toBeVisible(); // a11y-wise still visible - const label = screen.getByText(item.label); - expect(label).toBeInTheDocument(); // label exists but - expect(label).not.toBeVisible(); // not visible - }); - ["navicon1", "navicon2", "navicon3", "navicon4"].forEach((id) => - expect(screen.getByTestId(id)).toBeVisible(), - ); - }); + it("shows tooltip on buttons when closed", async () => { + renderSidenav(false); + + const icon = screen.getByTestId("navicon2"); + const user = userEvent.setup(); + await user.hover(icon); + + // notice we await because the tooltip appears after some time + const tooltip = await screen.findByRole("tooltip", { + name: "Acquisition", + }); + expect(tooltip).toBeVisible(); + }); - it("shows tooltip on buttons when closed", async () => { - renderSidenav(false); + it("shows no tooltip on buttons when open", async () => { + renderSidenav(true); - const icon = screen.getByTestId("navicon2"); - const user = userEvent.setup(); - await user.hover(icon); + const icon = screen.getByTestId("navicon2"); + const user = userEvent.setup(); + await user.hover(icon); - // notice we await because the tooltip appears after some time - const tooltip = await screen.findByRole("tooltip", { - name: "Acquisition", + const tooltip = screen.queryByRole("tooltip", { + name: "Acquisition", + }); + expect(tooltip).not.toBeInTheDocument(); }); - expect(tooltip).toBeVisible(); - }); - it("shows no tooltip on buttons when open", async () => { - renderSidenav(true); + it("creates divider between nav sections", () => { + renderSidenav(true); + const divider = screen.queryByRole("separator"); + expect(divider).toBeInTheDocument(); + }); - const icon = screen.getByTestId("navicon2"); - const user = userEvent.setup(); - await user.hover(icon); + it("renders afterNavSlot after the navigation items", () => { + const router = createMemoryRouter([ + { + path: "/", + element: ( + Extra links
} + /> + ), + }, + ]); + render(); + + expect(screen.getByTestId("after-nav")).toBeVisible(); + }); - const tooltip = screen.queryByRole("tooltip", { - name: "Acquisition", + it("renders footerSlot", () => { + const router = createMemoryRouter([ + { + path: "/", + element: ( + User menu
} + /> + ), + }, + ]); + render(); + + expect(screen.getByTestId("footer")).toBeVisible(); }); - expect(tooltip).not.toBeInTheDocument(); - }); - it("creates divider between nav sections", () => { - renderSidenav(true); - const divider = screen.queryByRole("separator"); - expect(divider).toBeInTheDocument(); + it("renders internal and external links with correct href", () => { + // even though specified differently, ultimately both types + // should have the correct href attribute + renderSidenav(true); + + const externalLink = screen.getByRole("link", { + name: "Organisation", + }); + expect(externalLink).toHaveAttribute("href", "https://www.example.com"); + + const internalLink = screen.getByRole("link", { name: "Setup" }); + expect(internalLink).toHaveAttribute("href", "/setup"); + }); }); - it("renders afterNavSlot after the navigation items", () => { - const router = createMemoryRouter([ - { - path: "/", - element: ( - Extra links
} - /> - ), - }, - ]); - render(); + describe("Mobile layout", () => { + beforeEach(() => { + mockedUseMediaQuery.mockReturnValue(false); + }); + + it("renders temporary drawer", () => { + renderSidenav(true); + + // Drawer paper is rendered + expect(document.querySelector(".MuiDrawer-root")).toBeInTheDocument(); + + // nav content is visible + expect(screen.getByText("Setup")).toBeVisible(); + }); + + it("is not visible when the drawer is closed", () => { + renderSidenav(false); + + expect(screen.queryByText("Setup")).not.toBeInTheDocument(); + expect( + screen.queryByRole("link", { name: "Setup" }), + ).not.toBeInTheDocument(); + }); - expect(screen.getByTestId("after-nav")).toBeVisible(); + it("is visible when the drawer is open", () => { + renderSidenav(true); + + expect(screen.getByText("Setup")).toBeVisible(); + expect(screen.getByTestId("navicon1")).toBeVisible(); + }); + + it("closes the drawer when a nav item is clicked", async () => { + const user = userEvent.setup(); + const setOpen = vi.fn(); + + renderSidenav(true, setOpen); + + await user.click(screen.getByRole("link", { name: "Setup" })); + + expect(setOpen).toHaveBeenCalledWith(false); + }); + + it("closes the drawer when the backdrop is clicked", async () => { + const user = userEvent.setup(); + const setOpen = vi.fn(); + + renderSidenav(true, setOpen); + + // backdrop is rendered by MUI in portal + const backdrop = document.querySelector(".MuiBackdrop-root"); + expect(backdrop).toBeInTheDocument(); + + await user.click(backdrop!); + + expect(setOpen).toHaveBeenCalledWith(false); + }); }); + }); - it("renders footerSlot", () => { - const router = createMemoryRouter([ + describe("with subNav", () => { + const navigation: Navigation = [ + { + navItems: [ + { + label: "Setup", + icon:
, + linkProps: { component: NavLink, to: "/setup" }, + }, + ], + }, + ]; + + const subNav: Omit = { + title: "Secondary", + groups: [ { - path: "/", - element: ( - User menu
} - /> - ), + items: [ + { + label: "Detail", + linkProps: { component: NavLink, to: "/detail" }, + }, + ], }, + ], + }; + + function Harness({ + initialOpen = true, + initialSubNavOpen = false, + withSubNav = true, + }: { + initialOpen?: boolean; + initialSubNavOpen?: boolean; + withSubNav?: boolean; + }) { + const [open, setOpen] = useState(initialOpen); + const [subNavOpen, setSubNavOpen] = useState(initialSubNavOpen); + + return withSubNav ? ( + + ) : ( + + ); + } + + function renderHarness(props: React.ComponentProps = {}) { + const router = createMemoryRouter([ + { path: "/", element: }, ]); - render(); + render(addProviders()); + } - expect(screen.getByTestId("footer")).toBeVisible(); - }); + describe("Desktop layout", () => { + beforeEach(() => { + mockedUseMediaQuery.mockReturnValue(true); + }); - it("renders internal and external links with correct href", () => { - // even though specified differently, ultimately both types - // should have the correct href attribute - renderSidenav(true); + it("renders both panels simultaneously when both are open", () => { + renderHarness({ + initialOpen: true, + initialSubNavOpen: true, + }); + + expect(screen.getByRole("link", { name: "Setup" })).toBeVisible(); + expect( + screen.getByRole("heading", { name: "Secondary" }), + ).toBeVisible(); + expect(screen.getByRole("link", { name: "Detail" })).toBeVisible(); + }); - const externalLink = screen.getByRole("link", { name: "Organisation" }); - expect(externalLink).toHaveAttribute("href", "https://www.example.com"); + it("hides only the secondary panel when subNavOpen is false", () => { + renderHarness({ + initialOpen: true, + initialSubNavOpen: false, + }); - const internalLink = screen.getByRole("link", { name: "Setup" }); - expect(internalLink).toHaveAttribute("href", "/setup"); - }); - }); + expect(screen.getByRole("link", { name: "Setup" })).toBeVisible(); + expect(screen.queryByText("Secondary")).not.toBeVisible(); + }); + + it("renders no secondary panel when subNav is omitted", () => { + renderHarness({ withSubNav: false }); - describe("Mobile layout", () => { - beforeEach(() => { - mockedUseMediaQuery.mockReturnValue(false); + expect(screen.getByRole("link", { name: "Setup" })).toBeVisible(); + expect( + screen.queryByRole("heading", { name: "Secondary" }), + ).not.toBeInTheDocument(); + }); }); - it("renders temporary drawer", () => { - renderSidenav(true); + describe("Mobile layout", () => { + beforeEach(() => { + mockedUseMediaQuery.mockReturnValue(false); + }); - // Drawer paper is rendered - expect(document.querySelector(".MuiDrawer-root")).toBeInTheDocument(); + it("shows only the sidebar when secondary nav is not open", () => { + renderHarness({ + initialOpen: true, + initialSubNavOpen: false, + }); - // nav content is visible - expect(screen.getByText("Setup")).toBeVisible(); - }); + expect(screen.getByText("Setup")).toBeVisible(); + expect(screen.queryByText("Secondary")).not.toBeInTheDocument(); + }); - it("closed drawer is not visible", () => { - renderSidenav(false); + it("hides the sidebar drawer and shows the secondary content in its own temporary drawer when the secondary panel opens", () => { + renderHarness({ + initialOpen: true, + initialSubNavOpen: true, + }); + + 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); + }); - expect(screen.queryByText("Setup")).not.toBeInTheDocument(); - expect( - screen.queryByRole("link", { name: "Setup" }), - ).not.toBeInTheDocument(); - }); + it("closes the secondary drawer when a nav item inside it is clicked", async () => { + const user = userEvent.setup(); + renderHarness({ + initialOpen: true, + initialSubNavOpen: true, + }); - it("open drawer is visible", () => { - renderSidenav(true); + await user.click(screen.getByRole("link", { name: "Detail" })); - expect(screen.getByText("Setup")).toBeVisible(); - expect(screen.getByTestId("navicon1")).toBeVisible(); - }); + await waitFor(() => { + expect(screen.queryByText("Secondary")).not.toBeInTheDocument(); + }); + }); - it("clicking a nav item closes the drawer", async () => { - const user = userEvent.setup(); - const setOpen = vi.fn(); + it("closes the secondary drawer when the backdrop is clicked", async () => { + const user = userEvent.setup(); + renderHarness({ + initialOpen: true, + initialSubNavOpen: true, + }); - renderSidenav(true, setOpen); + const backdrop = document.querySelector(".MuiBackdrop-root"); + expect(backdrop).toBeInTheDocument(); - await user.click(screen.getByRole("link", { name: "Setup" })); + await user.click(backdrop!); - expect(setOpen).toHaveBeenCalledWith(false); - }); + await waitFor(() => { + expect(screen.queryByText("Secondary")).not.toBeInTheDocument(); + }); + }); - it("clicking backdrop closes the drawer", async () => { - const user = userEvent.setup(); - const setOpen = vi.fn(); + it("drills back to the sidebar via the back button", async () => { + const user = userEvent.setup(); + renderHarness({ + initialOpen: true, + initialSubNavOpen: true, + }); - renderSidenav(true, setOpen); + expect(screen.queryByText("Setup")).not.toBeInTheDocument(); - // backdrop is rendered by MUI in portal - const backdrop = document.querySelector(".MuiBackdrop-root"); - expect(backdrop).toBeInTheDocument(); + await user.click(screen.getByRole("button", { name: "Back" })); - await user.click(backdrop!); + await waitFor(() => { + expect(screen.queryByText("Secondary")).not.toBeInTheDocument(); + }); + expect(screen.getByText("Setup")).toBeVisible(); + }); + + it("renders no secondary panel when subNav is omitted", () => { + renderHarness({ withSubNav: false }); + + expect(screen.getByText("Setup")).toBeVisible(); + expect(screen.queryByText("Secondary")).not.toBeInTheDocument(); + }); - expect(setOpen).toHaveBeenCalledWith(false); + it("still reaches the sidebar via the back button even if secondary nav opened while open was false", async () => { + // Opens the secondary panel without the sidebar ever having been + // marked open first (e.g. deep-linking straight into it) - exercises + // SidebarNav's self-heal of `open`. + const user = userEvent.setup(); + renderHarness({ + initialOpen: false, + initialSubNavOpen: 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("responds to a popstate notification (e.g. a real device/browser back navigation) by drilling back to the sidebar", async () => { + renderHarness({ + initialOpen: true, + initialSubNavOpen: true, + }); + + expect(screen.queryByText("Setup")).not.toBeInTheDocument(); + + // SidebarNav pushed a history entry when the secondary panel opened + // (see the effect in SidebarNav.tsx), so dispatching the same + // "popstate" event a real back navigation would fire is a faithful + // simulation of the user pressing back, not just an arbitrary event. + window.dispatchEvent(new PopStateEvent("popstate")); + + await waitFor(() => { + expect(screen.queryByText("Secondary")).not.toBeInTheDocument(); + }); + expect(screen.getByText("Setup")).toBeVisible(); + }); }); }); }); diff --git a/src/components/navigation/SidebarNav.tsx b/src/components/navigation/SidebarNav.tsx index dcd6e13e..89bcba66 100644 --- a/src/components/navigation/SidebarNav.tsx +++ b/src/components/navigation/SidebarNav.tsx @@ -11,9 +11,10 @@ import { Tooltip, } from "@mui/material"; import { useTheme, Theme } from "@mui/material/styles"; -import { Fragment, type ReactNode } from "react"; +import { Fragment, useEffect, useRef, type ReactNode } from "react"; import useMediaQuery from "@mui/material/useMediaQuery"; -import type { LinkProps } from "./types"; +import { SubNavContent, type SubNavContentProps } from "./SubNav"; +import { NAV_WIDTH, type NavItem } from "./types"; export type Navigation = NavItemGroup[]; @@ -22,14 +23,13 @@ type NavItemGroup = { navItems: NavItemDefinition[]; }; -type NavItemDefinition = { - label: string; - icon: ReactNode; - linkProps: LinkProps; - selected?: boolean; +type NavItemDefinition = NavItem & { + icon: NonNullable; + linkProps: NonNullable; }; -const getSidebarNavWidth = (open: boolean) => (open ? 257 : 65); // 256/64 + 1 pixel for the border +const getPrimaryNavWidth = (open: boolean) => + (open ? NAV_WIDTH : NAV_WIDTH / 4) + 1; // +1 pixel for the border export const drawerTransition = (theme: Theme, opening: boolean) => { return theme.transitions.create("width", { @@ -42,17 +42,177 @@ export const drawerTransition = (theme: Theme, opening: boolean) => { }); }; -type NavProps = { +type SidebarNavProps = { navigation: Navigation; + open: boolean; setOpen: (open: boolean) => void; + /** Rendered after the navigation items, inside the scrollable area. */ afterNavSlot?: ReactNode; /** Rendered pinned to the bottom of the drawer, outside the scrollable area. */ footerSlot?: ReactNode; + + /** Omit to render the primary nav only, with no secondary panel at all. */ + subNav?: 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 the derivation of + * `effectiveOpen` below. Required whenever `subNav` is provided. + */ + subNavOpen?: boolean; + setSubNavOpen?: (open: boolean) => void; }; -export function SidebarNav(props: NavProps) { +/** + * Primary navigation, with an optional contextual secondary panel alongside + * it. Without `subNav` this is just the collapsing/expanding primary + * drawer. With it, SidebarNav also owns the responsive behaviour between the + * two: 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 + * `subNavOpen` back to false. On desktop both are shown side by side, + * the secondary content in a plain panel rather than a drawer. + */ +function SidebarNav(props: SidebarNavProps) { + const theme = useTheme(); + const desktopLayout = useMediaQuery(theme.breakpoints.up("sm")); + + const subNavOpen = props.subNavOpen ?? false; + + const effectiveOpen = desktopLayout ? props.open : props.open && !subNavOpen; + + // Mobile: the back affordance (device/browser back, or the panel's own + // back arrow) only has something to fall back to if `open` is true once + // `subNavOpen` flips false again. A consumer can open the secondary + // panel without `open` set yet - e.g. deep-linking straight into it - so + // this self-heals that invariant. + const setOpenRef = useRef(props.setOpen); + setOpenRef.current = props.setOpen; + + useEffect(() => { + if (!desktopLayout && subNavOpen) { + setOpenRef.current(true); + } + }, [desktopLayout, subNavOpen]); + + // 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 setSubNavOpenRef = useRef(props.setSubNavOpen); + setSubNavOpenRef.current = props.setSubNavOpen; + + useEffect(() => { + if (desktopLayout || !subNavOpen) { + return; + } + + window.history.pushState({ subNavOpen: true }, ""); + const onPopState = () => setSubNavOpenRef.current?.(false); + window.addEventListener("popstate", onPopState); + + return () => window.removeEventListener("popstate", onPopState); + }, [desktopLayout, subNavOpen]); + + return ( + <> + + {props.subNav && + (desktopLayout ? ( + + + + ) : ( + props.setSubNavOpen?.(false)} + onClick={() => props.setSubNavOpen?.(false)} // close after making a selection + sx={{ + width: NAV_WIDTH, + flexShrink: 0, + [`& .MuiDrawer-paper`]: { + width: NAV_WIDTH, + boxSizing: "border-box", + backgroundImage: "none", + bgcolor: theme.palette.surface.elevated(1), + borderRight: "1px solid", + borderColor: "divider", + }, + }} + > + + props.setSubNavOpen?.(false)} + /> + + ))} + + ); +} + +/** + * Desktop layout: a plain flex sibling of the primary drawer, 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 + * the primary drawer's width-transition mechanism. + */ +function SubNavPanel(props: { open: boolean; children: ReactNode }) { + const theme = useTheme(); + const width = props.open ? NAV_WIDTH + 1 : 0; // +1 pixel for the border + + return ( + + {/* spacer equal to the AppBar's height */} + {/* flex:1 (not a percentage height) so this fills the panel without + depending on an ancestor having a "definite" height to resolve + against - percentage heights chained through a flex row were the + likely cause of the stray scrollbar this replaced. */} + + {props.children} + + + ); +} + +type PrimaryNavProps = { + navigation: Navigation; + open: boolean; + setOpen: (open: boolean) => void; + /** Rendered after the navigation items, inside the scrollable area. */ + afterNavSlot?: ReactNode; + /** Rendered pinned to the bottom of the drawer, outside the scrollable area. */ + footerSlot?: ReactNode; +}; + +/** + * The primary drawer alone: a permanent-variant drawer on desktop which + * toggles between full width and slim (icon-only) states, or a temporary + * (overlaid) drawer on smaller screens. + */ +function PrimaryNav(props: PrimaryNavProps) { const theme = useTheme(); const desktopLayout = useMediaQuery(theme.breakpoints.up("sm")); @@ -62,13 +222,8 @@ export function SidebarNav(props: NavProps) { return ; } -/** - * Main layout: a permanant-variant drawer - * which toggles between full width and slim states. - * Pushes main content to the right. - */ -function PermanentDrawer(props: NavProps) { - const width = getSidebarNavWidth(props.open); +function PermanentDrawer(props: PrimaryNavProps) { + const width = getPrimaryNavWidth(props.open); return ( 0 && } {group.navItems.map((item, itemIndex) => { return ( - + ); })} @@ -174,12 +328,12 @@ function SectionDivider() { ); } -interface NavItemProps { +interface PrimaryNavItemProps { definition: NavItemDefinition; sidebarOpen: boolean; } -function NavItem(props: NavItemProps) { +function PrimaryNavItem(props: PrimaryNavItemProps) { const item = props.definition; const open = props.sidebarOpen; const icon = ( @@ -236,3 +390,6 @@ function NavItem(props: NavItemProps) { ); } + +export { SidebarNav }; +export type { SidebarNavProps }; diff --git a/src/components/navigation/SecondaryNav.stories.tsx b/src/components/navigation/SubNav.stories.tsx similarity index 65% rename from src/components/navigation/SecondaryNav.stories.tsx rename to src/components/navigation/SubNav.stories.tsx index 253e30a3..b107dcbc 100644 --- a/src/components/navigation/SecondaryNav.stories.tsx +++ b/src/components/navigation/SubNav.stories.tsx @@ -1,12 +1,14 @@ import { Abc, ArrowForward, GraphicEq } from "@mui/icons-material"; -import { SecondaryNavContent } from "./SecondaryNav"; +import SearchIcon from "@mui/icons-material/Search"; +import { InputAdornment, TextField } from "@mui/material"; +import { SubNavContent } from "./SubNav"; 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: SecondaryNavContent, +const meta: Meta = { + title: "Components/Navigation/SubNav", + component: SubNavContent, decorators: [ (Story) => ( @@ -18,7 +20,7 @@ const meta: Meta = { parameters: { docs: { description: { - 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.`, + component: `The content of a contextual secondary navigation panel: a header (title/search slot/back) plus a grouped, optionally-expandable list. Presentation-agnostic - it renders no Drawer or panel of its own. Use SidebarNav's \`subNav\` prop to get the responsive mobile drill-down / desktop side-by-side behaviour around it.`, }, }, }, @@ -31,18 +33,14 @@ 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 }, }, @@ -81,13 +79,38 @@ export const WithTitleAndSearch: Story = { render: () => { const [value, setValue] = React.useState(""); return ( - setValue(e.target.value)} + placeholder="Search items" + slotProps={{ + input: { + startAdornment: ( + + + + ), + }, + }} + /> + } /> ); }, + parameters: { + docs: { + description: { + story: + "searchSlot takes any ReactNode - SubNavContent has no search logic of its own, so filtering `groups` in response to the value is entirely up to the consumer.", + }, + }, + }, }; const groupedGroups = [ @@ -95,13 +118,11 @@ 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 }, @@ -112,7 +133,6 @@ const groupedGroups = [ subheader: "All experiments", items: [ { - id: "analysis", label: "Analysis", icon: , linkProps: { to: "/3", component: NavLink }, @@ -131,21 +151,16 @@ const expandableGroups = [ { items: [ { - id: "analysis", label: "Analysis", icon: , defaultExpanded: true, - children: [ - { id: "analysis-a", label: "Run A" }, - { id: "analysis-b", label: "Run B" }, - ], + children: [{ label: "Run A" }, { label: "Run B" }], }, { - id: "acquisition", label: "Acquisition", icon: , linkProps: { to: "/2", component: NavLink }, - children: [{ id: "acquisition-a", label: "Session 1" }], + children: [{ label: "Session 1" }], }, ], }, @@ -175,7 +190,7 @@ export const WithBackButton: Story = { docs: { description: { story: - "onBack is normally supplied by NavigationLayout on mobile to drill back to the primary sidebar, shown here in isolation.", + "onBack is normally supplied by SidebarNav 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/SubNav.test.tsx similarity index 62% rename from src/components/navigation/SecondaryNav.test.tsx rename to src/components/navigation/SubNav.test.tsx index 7df84c01..732f1a43 100644 --- a/src/components/navigation/SecondaryNav.test.tsx +++ b/src/components/navigation/SubNav.test.tsx @@ -1,22 +1,21 @@ import { render, screen } from "@testing-library/react"; -import { SecondaryNavContent, SecondaryNavGroup } from "./SecondaryNav"; +import { TextField } from "@mui/material"; +import { SubNavContent, SubNavGroup } from "./SubNav"; import { createMemoryRouter, NavLink, RouterProvider } from "react-router-dom"; import userEvent from "@testing-library/user-event"; import type { ComponentProps } from "react"; import { addProviders } from "../../__test-utils__/helpers"; -describe("SecondaryNavContent", () => { - const groups: SecondaryNavGroup[] = [ +describe("SubNavContent", () => { + const groups: SubNavGroup[] = [ { subheader: "Group one", items: [ { - id: "setup", label: "Setup", linkProps: { component: NavLink, to: "/setup" }, }, { - id: "acquisition", label: "Acquisition", linkProps: { component: NavLink, to: "/acq" }, }, @@ -26,25 +25,20 @@ describe("SecondaryNavContent", () => { subheader: "Group two", items: [ { - id: "analysis", label: "Analysis", - children: [ - { id: "analysis-a", label: "Analysis A" }, - { id: "analysis-b", label: "Analysis B" }, - ], + children: [{ label: "Analysis A" }, { label: "Analysis B" }], }, { - id: "expandable-link", label: "Expandable link", linkProps: { href: "https://www.example.com" }, - children: [{ id: "expandable-link-child", label: "Child" }], + children: [{ label: "Child" }], }, ], }, ]; - function renderSecondaryNavContent( - props: Partial> = {}, + function renderSubNavContent( + props: Partial> = {}, { onOuterClick }: { onOuterClick?: () => void } = {}, ) { const router = createMemoryRouter([ @@ -52,11 +46,11 @@ describe("SecondaryNavContent", () => { 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. + // itself on selection (e.g. SidebarNav's mobile drawer) - it's how + // these tests observe stopPropagation without depending on any + // particular consumer's implementation.
- +
), }, @@ -65,7 +59,7 @@ describe("SecondaryNavContent", () => { } it("renders grouped items with subheaders and a divider between groups", () => { - renderSecondaryNavContent(); + renderSubNavContent(); expect(screen.getByText("Group one")).toBeVisible(); expect(screen.getByText("Group two")).toBeVisible(); @@ -75,58 +69,60 @@ describe("SecondaryNavContent", () => { }); it("renders a title when provided", () => { - renderSecondaryNavContent({ title: "Secondary" }); + renderSubNavContent({ title: "Secondary" }); expect(screen.getByRole("heading", { name: "Secondary" })).toBeVisible(); }); - it("dense defaults to true, applying compact row styling", () => { - renderSecondaryNavContent(); + it("applies compact row styling by default, since dense defaults to true", () => { + renderSubNavContent(); expect(screen.getByRole("link", { name: "Setup" })).toHaveClass( "MuiListItemButton-dense", ); }); - it("dense can be turned off for taller rows", () => { - renderSecondaryNavContent({ dense: false }); + it("renders taller rows when dense is turned off", () => { + renderSubNavContent({ dense: false }); expect(screen.getByRole("link", { name: "Setup" })).not.toHaveClass( "MuiListItemButton-dense", ); }); - it("never renders a Drawer itself - it has no responsive presentation of its own", () => { - renderSecondaryNavContent(); - expect(document.querySelector(".MuiDrawer-root")).not.toBeInTheDocument(); - }); - it("does not render a header when no header props are provided", () => { - renderSecondaryNavContent(); - expect(screen.queryByRole("searchbox")).not.toBeInTheDocument(); + renderSubNavContent(); expect( screen.queryByRole("button", { name: "Back" }), ).not.toBeInTheDocument(); + expect(screen.queryByRole("textbox")).not.toBeInTheDocument(); }); - it("search input calls onChange and does not filter the passed-in groups itself", async () => { + it("renders whatever is passed as searchSlot, with no search logic of its own", async () => { const user = userEvent.setup(); const onChange = vi.fn(); - renderSecondaryNavContent({ - search: { value: "", onChange, placeholder: "Search" }, + renderSubNavContent({ + searchSlot: ( + onChange(e.target.value)} + placeholder="Search" + /> + ), }); const input = screen.getByPlaceholderText("Search"); await user.type(input, "a"); expect(onChange).toHaveBeenCalledWith("a"); - // groups are rendered unfiltered regardless of search value + // groups are rendered unfiltered - filtering on the slot's value, if + // any, is entirely up to the consumer. expect(screen.getByRole("link", { name: "Setup" })).toBeVisible(); }); - it("renders a back button only when onBack is provided", async () => { + it("renders a back button when onBack is provided", async () => { const user = userEvent.setup(); const onBack = vi.fn(); - renderSecondaryNavContent({ onBack }); + renderSubNavContent({ onBack }); const back = screen.getByRole("button", { name: "Back" }); expect(back).toBeVisible(); @@ -135,9 +131,17 @@ describe("SecondaryNavContent", () => { expect(onBack).toHaveBeenCalled(); }); - it("expanding an item reveals its children and toggles aria-expanded", async () => { + it("does not render a back button when onBack is not provided", () => { + renderSubNavContent({ title: "Secondary" }); + + expect( + screen.queryByRole("button", { name: "Back" }), + ).not.toBeInTheDocument(); + }); + + it("reveals an item's children and toggles aria-expanded when expanded", async () => { const user = userEvent.setup(); - renderSecondaryNavContent(); + renderSubNavContent(); expect(screen.queryByText("Analysis A")).not.toBeInTheDocument(); @@ -154,9 +158,9 @@ describe("SecondaryNavContent", () => { ).toHaveAttribute("aria-expanded", "true"); }); - it("clicking the row itself (not just the chevron) toggles a toggle-only item", async () => { + it("toggles a toggle-only item when clicking the row itself, not just the chevron", async () => { const user = userEvent.setup(); - renderSecondaryNavContent(); + renderSubNavContent(); expect(screen.queryByText("Analysis A")).not.toBeInTheDocument(); @@ -167,9 +171,9 @@ describe("SecondaryNavContent", () => { expect(screen.getByText("Analysis A")).toBeVisible(); }); - it("a row with both linkProps and children navigates and toggles together on label click", async () => { + it("navigates and toggles together on label click when a row has both a link and children", async () => { const user = userEvent.setup(); - renderSecondaryNavContent(); + renderSubNavContent(); const link = screen.getByRole("link", { name: "Expandable link" }); expect(link).toHaveAttribute("href", "https://www.example.com"); @@ -180,9 +184,9 @@ describe("SecondaryNavContent", () => { expect(screen.getByText("Child")).toBeVisible(); }); - it("a row with both linkProps and children can also be toggled via the chevron alone", async () => { + it("also toggles via the chevron alone when a row has both a link and children", async () => { const user = userEvent.setup(); - renderSecondaryNavContent(); + renderSubNavContent(); expect(screen.queryByText("Child")).not.toBeInTheDocument(); @@ -193,16 +197,13 @@ describe("SecondaryNavContent", () => { }); it("auto-expands an item that is selected or has a selected child", () => { - renderSecondaryNavContent({ + renderSubNavContent({ groups: [ { items: [ { - id: "analysis", label: "Analysis", - children: [ - { id: "analysis-a", label: "Analysis A", selected: true }, - ], + children: [{ label: "Analysis A", selected: true }], }, ], }, @@ -212,24 +213,24 @@ describe("SecondaryNavContent", () => { expect(screen.getByText("Analysis A")).toBeVisible(); }); - // 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. + // A consumer (e.g. SidebarNav'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 () => { + it("lets a plain link row's click bubble up to an ancestor", async () => { const user = userEvent.setup(); const onOuterClick = vi.fn(); - renderSecondaryNavContent({}, { onOuterClick }); + renderSubNavContent({}, { onOuterClick }); await user.click(screen.getByRole("link", { name: "Setup" })); expect(onOuterClick).toHaveBeenCalled(); }); - it("a toggle-only row's click does not bubble up to an ancestor", async () => { + it("stops a toggle-only row's click from bubbling up to an ancestor", async () => { const user = userEvent.setup(); const onOuterClick = vi.fn(); - renderSecondaryNavContent({}, { onOuterClick }); + renderSubNavContent({}, { onOuterClick }); await user.click(screen.getByText("Analysis")); @@ -237,20 +238,20 @@ describe("SecondaryNavContent", () => { expect(onOuterClick).not.toHaveBeenCalled(); }); - it("expanding via the chevron alone does not bubble up to an ancestor", async () => { + it("stops a chevron-only expand click from bubbling up to an ancestor", async () => { const user = userEvent.setup(); const onOuterClick = vi.fn(); - renderSecondaryNavContent({}, { onOuterClick }); + renderSubNavContent({}, { onOuterClick }); await user.click(screen.getByRole("button", { name: "Expand Analysis" })); expect(onOuterClick).not.toHaveBeenCalled(); }); - it("a row that is both a link and expandable still bubbles up on click", async () => { + it("still bubbles up a click on a row that is both a link and expandable", async () => { const user = userEvent.setup(); const onOuterClick = vi.fn(); - renderSecondaryNavContent({}, { onOuterClick }); + renderSubNavContent({}, { onOuterClick }); await user.click(screen.getByRole("link", { name: "Expandable link" })); diff --git a/src/components/navigation/SecondaryNav.tsx b/src/components/navigation/SubNav.tsx similarity index 73% rename from src/components/navigation/SecondaryNav.tsx rename to src/components/navigation/SubNav.tsx index 0efca1f4..0f9838aa 100644 --- a/src/components/navigation/SecondaryNav.tsx +++ b/src/components/navigation/SubNav.tsx @@ -3,14 +3,12 @@ import { Collapse, Divider, IconButton, - InputAdornment, List, ListItem, ListItemButton, ListItemIcon, ListItemText, ListSubheader, - TextField, Typography, } from "@mui/material"; import { Theme } from "@mui/material/styles"; @@ -23,44 +21,25 @@ import { } from "react"; import ArrowBackIcon from "@mui/icons-material/ArrowBack"; import ExpandMoreIcon from "@mui/icons-material/ExpandMore"; -import SearchIcon from "@mui/icons-material/Search"; -import type { LinkProps } from "./types"; +import type { NavItem, NavItemWithChildren } from "./types"; -type SecondaryNavGroup = { +type SubNavGroup = { /** Rendered as an overline ListSubheader when present; omit for an ungrouped list. */ subheader?: string; - items: SecondaryNavItemDefinition[]; + items: NavItemWithChildren[]; }; -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 SecondaryNavContentProps = { +type SubNavContentProps = { title?: string; - search?: { - value: string; - onChange: (value: string) => void; - placeholder?: string; - }; + /** Rendered in the header, above the item list; typically a search field. */ + searchSlot?: ReactNode; - groups: SecondaryNavGroup[]; + groups: SubNavGroup[]; /** * Renders a back affordance above the title/search when provided. - * NavigationLayout supplies this on mobile only; omit for standalone use. + * SidebarNav supplies this on mobile only; omit for standalone use. */ onBack?: () => void; @@ -71,15 +50,14 @@ type SecondaryNavContentProps = { /** * 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. + * panel, responsive switching) is SidebarNav's job, not this component's. */ -function SecondaryNavContent(props: SecondaryNavContentProps) { +function SubNavContent(props: SubNavContentProps) { const dense = props.dense ?? true; return ( - + {props.groups.map((group, groupIndex) => ( @@ -98,8 +76,8 @@ function SecondaryNavContent(props: SecondaryNavContentProps) { {group.subheader} )} - {group.items.map((item) => ( - + {group.items.map((item, itemIndex) => ( + ))} ))} @@ -109,8 +87,8 @@ function SecondaryNavContent(props: SecondaryNavContentProps) { ); } -function SecondaryNavHeader(props: SecondaryNavContentProps) { - const hasHeader = props.onBack || props.title || props.search; +function SubNavHeader(props: SubNavContentProps) { + const hasHeader = props.onBack || props.title || props.searchSlot; if (!hasHeader) { return null; @@ -145,24 +123,7 @@ function SecondaryNavHeader(props: SecondaryNavContentProps) { )} - {props.search && ( - props.search!.onChange(e.target.value)} - placeholder={props.search.placeholder ?? "Search"} - slotProps={{ - input: { - startAdornment: ( - - - - ), - }, - }} - /> - )} + {props.searchSlot} ); } @@ -187,11 +148,11 @@ function getItemButtonSx(dense: boolean) { }; } -function SecondaryNavItem({ +function SubNavItem({ item, dense, }: { - item: SecondaryNavItemDefinition; + item: NavItemWithChildren; dense: boolean; }) { const hasChildren = !!item.children?.length; @@ -213,7 +174,7 @@ function SecondaryNavItem({ }; // 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 + // SidebarNav'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 @@ -281,12 +242,8 @@ function SecondaryNavItem({ {hasChildren && ( - {item.children!.map((child) => ( - + {item.children!.map((child, childIndex) => ( + ))} @@ -295,13 +252,7 @@ function SecondaryNavItem({ ); } -function SecondaryNavChildItem({ - item, - dense, -}: { - item: SecondaryNavChildItemDefinition; - dense: boolean; -}) { +function SubNavChildItem({ item, dense }: { item: NavItem; dense: boolean }) { const iconSize = dense ? 24 : 28; return ( @@ -335,10 +286,5 @@ function SecondaryNavChildItem({ ); } -export { SecondaryNavContent }; -export type { - SecondaryNavContentProps, - SecondaryNavGroup, - SecondaryNavItemDefinition, - SecondaryNavChildItemDefinition, -}; +export { SubNavContent }; +export type { SubNavContentProps, SubNavGroup }; diff --git a/src/components/navigation/types.ts b/src/components/navigation/types.ts index 0460933a..33898c56 100644 --- a/src/components/navigation/types.ts +++ b/src/components/navigation/types.ts @@ -1,4 +1,4 @@ -import type { ElementType } from "react"; +import type { ElementType, ReactNode } from "react"; /** Shared link-prop union for any link-bearing row across navigation components. */ export type LinkProps = ExternalLinkProps | InternalLinkProps; @@ -16,3 +16,22 @@ export type InternalLinkProps = { to: string; href?: never; }; + +/** Baseline open-state width (px) shared by the primary and secondary nav panels; add 1 where a consumer also draws a border. */ +export const NAV_WIDTH = 256; + +/** A single navigable row, shared by SidebarNav and SubNav. */ +export type NavItem = { + label: string; + icon?: ReactNode; + linkProps?: LinkProps; + selected?: boolean; +}; + +/** A NavItem that may itself expand to reveal one level of child NavItems. */ +export type NavItemWithChildren = NavItem & { + /** One level only - children cannot themselves expand. */ + children?: NavItem[]; + /** Initial Collapse state for this item; uncontrolled thereafter. */ + defaultExpanded?: boolean; +}; diff --git a/src/index.ts b/src/index.ts index 1d33825c..6841e1a3 100644 --- a/src/index.ts +++ b/src/index.ts @@ -3,9 +3,8 @@ export * from "./components/navigation/Breadcrumbs"; export * from "./components/navigation/Footer"; export * from "./components/navigation/Navbar"; export * from "./components/navigation/NavMenu"; -export * from "./components/navigation/NavigationLayout"; -export * from "./components/navigation/SecondaryNav"; export * from "./components/navigation/SidebarNav"; +export * from "./components/navigation/SubNav"; export * from "./components/navigation/types"; // components/controls From f5e858cd9a9d9cc0c55107c61ddcec59c0bf7269 Mon Sep 17 00:00:00 2001 From: Zohar Manor-Abel Date: Mon, 24 Aug 2026 16:14:59 +0100 Subject: [PATCH 5/5] Add SubNav slots and make AppBar spacer opt-in - Replace `SubNav`'s `searchSlot` with `beforeNavSlot`, `afterNavSlot`, and `footerSlot` - Add `hasAppBar` to `SidebarNav`, defaulting to `false`, so the Toolbar spacer is only rendered when needed - Align subheaders, expandable-item children, and slot content with nav item text - Replace the expand/collapse chevron with Lucide's `ChevronDown` - Expand SidebarNav and SubNav story docs with usage guidance --- .../navigation/SidebarNav.stories.tsx | 20 ++- src/components/navigation/SidebarNav.test.tsx | 25 ++- src/components/navigation/SidebarNav.tsx | 29 +++- src/components/navigation/SubNav.stories.tsx | 158 +++++++++++++++--- src/components/navigation/SubNav.test.tsx | 28 +++- src/components/navigation/SubNav.tsx | 89 +++++----- 6 files changed, 266 insertions(+), 83 deletions(-) diff --git a/src/components/navigation/SidebarNav.stories.tsx b/src/components/navigation/SidebarNav.stories.tsx index 096c4457..61a3b187 100644 --- a/src/components/navigation/SidebarNav.stories.tsx +++ b/src/components/navigation/SidebarNav.stories.tsx @@ -49,7 +49,21 @@ const meta: Meta = { fullBleed: true, docs: { description: { - component: `Your app's primary navigation, with an optional contextual secondary panel alongside it. Without \`subNav\` this renders the collapsing/expanding primary drawer alone - a permanent drawer toggling between two widths (icon and text, or just icon) on normal screen sizes, and a temporary (overlaid) drawer on smaller screens. With \`subNav\`, SidebarNav also owns the responsive coordination between the two panels: 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 - SidebarNav only owns the responsive mechanics, not when the panel opens.`, + component: `A collapsing/expanding sidebar for your app's primary navigation, with an optional contextual secondary panel alongside it. + +- **\`hasAppBar\`**: set this when your app renders a fixed AppBar above SidebarNav, so its drawers/panels reserve space for it. Omit it if there's no AppBar, or content is pushed down by an empty gap. +- **Without \`subNav\`**: renders the primary drawer alone. For normal screen sizes, the implementation uses MUI's permanent drawer toggling between two widths, showing either icon and text or just icon. For smaller screens, we use the temporary variant instead. +- **With \`subNav\`**: SidebarNav also owns the responsive coordination between the primary drawer and a secondary panel. On mobile only one drawer is visible at a time: opening the secondary panel drills in and hides the primary sidebar, with a back affordance to drill 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 — SidebarNav only owns the responsive mechanics, not when the panel opens. + +**Using an AppBar** +- Render your \`AppBar\` as a sibling of SidebarNav, with \`position="fixed"\` and a \`zIndex\` above \`theme.zIndex.drawer\` — MUI wants to draw a Drawer above everything else otherwise. +- Set \`hasAppBar\` on SidebarNav so its drawers/panels line up below the AppBar, rather than starting at the very top of the screen. +- Wire the AppBar's own menu button to toggle \`open\`, as shown in \`WithAppBar\`. + +**Using SubNav** +- Only pass \`subNav\` for the primary items that actually have their own sub-navigation. Most items can have none. +- Drive \`subNavOpen\`/\`setSubNavOpen\` from whichever primary item is currently selected (see \`WithAppBarAndSubNav\`), not from SidebarNav itself. It only owns the responsive mechanics, not when the panel opens. +- Use \`SubNav\`'s own \`beforeNavSlot\`/\`afterNavSlot\`/\`footerSlot\` for content like search or quick actions inside the secondary panel, rather than reaching into its internals.`, }, story: { height: "600px", @@ -417,6 +431,7 @@ export const WithAppBar: Story = { navigation={reactRouterNavigation} open={open} setOpen={setOpen} + hasAppBar /> {/* spacer equal to the AppBar's height */} @@ -431,7 +446,7 @@ export const WithAppBar: Story = { docs: { description: { story: - "MUI wants to draw a Drawer above everything, so in this example the AppBar's zIndex is increased. Clicking the menu icon toggles the sidebar open and closed.", + "MUI wants to draw a Drawer above everything, so in this example the AppBar's zIndex is increased. Clicking the menu icon toggles the sidebar open and closed. hasAppBar reserves space at the top of the drawer for the AppBar - omit it when there's no AppBar.", }, }, }, @@ -631,6 +646,7 @@ export const WithAppBarAndSubNav: Story = { navigation={navigation} open={sidebarOpen} setOpen={setSidebarOpen} + hasAppBar subNav={{ title: "Setup", groups: setupGroupsWithChildLinks }} subNavOpen={subNavOpen} setSubNavOpen={setSubNavOpen} diff --git a/src/components/navigation/SidebarNav.test.tsx b/src/components/navigation/SidebarNav.test.tsx index 2c1cf55f..d5bb8946 100644 --- a/src/components/navigation/SidebarNav.test.tsx +++ b/src/components/navigation/SidebarNav.test.tsx @@ -44,12 +44,21 @@ describe("SidebarNav", () => { }, ]; - function renderSidenav(open: boolean, setOpen = vi.fn()) { + function renderSidenav( + open: boolean, + setOpen = vi.fn(), + hasAppBar = false, + ) { const router = createMemoryRouter([ { path: "/", element: ( - + ), }, ]); @@ -125,6 +134,18 @@ describe("SidebarNav", () => { expect(divider).toBeInTheDocument(); }); + it("does not reserve space for an AppBar when hasAppBar is not set", () => { + renderSidenav(true); + expect( + document.querySelector(".MuiToolbar-root"), + ).not.toBeInTheDocument(); + }); + + it("reserves space for an AppBar when hasAppBar is set", () => { + renderSidenav(true, vi.fn(), true); + expect(document.querySelector(".MuiToolbar-root")).toBeInTheDocument(); + }); + it("renders afterNavSlot after the navigation items", () => { const router = createMemoryRouter([ { diff --git a/src/components/navigation/SidebarNav.tsx b/src/components/navigation/SidebarNav.tsx index 89bcba66..a03f895b 100644 --- a/src/components/navigation/SidebarNav.tsx +++ b/src/components/navigation/SidebarNav.tsx @@ -48,6 +48,12 @@ type SidebarNavProps = { open: boolean; setOpen: (open: boolean) => void; + /** + * Set when a fixed AppBar sits above this component, so its drawers and + * panels reserve space for it. Omit if there's no AppBar. + */ + hasAppBar?: boolean; + /** Rendered after the navigation items, inside the scrollable area. */ afterNavSlot?: ReactNode; /** Rendered pinned to the bottom of the drawer, outside the scrollable area. */ @@ -122,12 +128,13 @@ function SidebarNav(props: SidebarNavProps) { navigation={props.navigation} open={effectiveOpen} setOpen={props.setOpen} + hasAppBar={props.hasAppBar} afterNavSlot={props.afterNavSlot} footerSlot={props.footerSlot} /> {props.subNav && (desktopLayout ? ( - + ) : ( @@ -149,7 +156,7 @@ function SidebarNav(props: SidebarNavProps) { }, }} > - + {props.hasAppBar && } props.setSubNavOpen?.(false)} @@ -166,7 +173,11 @@ function SidebarNav(props: SidebarNavProps) { * render on top of each other. Transitions width between 0 and full, reusing * the primary drawer's width-transition mechanism. */ -function SubNavPanel(props: { open: boolean; children: ReactNode }) { +function SubNavPanel(props: { + open: boolean; + hasAppBar?: boolean; + children: ReactNode; +}) { const theme = useTheme(); const width = props.open ? NAV_WIDTH + 1 : 0; // +1 pixel for the border @@ -185,7 +196,8 @@ function SubNavPanel(props: { open: boolean; children: ReactNode }) { borderColor: "divider", }} > - {/* spacer equal to the AppBar's height */} + {props.hasAppBar && }{" "} + {/* spacer equal to the AppBar's height */} {/* flex:1 (not a percentage height) so this fills the panel without depending on an ancestor having a "definite" height to resolve against - percentage heights chained through a flex row were the @@ -201,6 +213,8 @@ type PrimaryNavProps = { navigation: Navigation; open: boolean; setOpen: (open: boolean) => void; + /** Set when a fixed AppBar sits above the drawer, reserving space for it. */ + hasAppBar?: boolean; /** Rendered after the navigation items, inside the scrollable area. */ afterNavSlot?: ReactNode; /** Rendered pinned to the bottom of the drawer, outside the scrollable area. */ @@ -238,7 +252,8 @@ function PermanentDrawer(props: PrimaryNavProps) { }, })} > - {/* spacer equal to the AppBar's height*/} + {props.hasAppBar && }{" "} + {/* spacer equal to the AppBar's height*/} ); @@ -264,7 +279,7 @@ function TemporaryDrawer(props: PrimaryNavProps) { }, }} > - + {props.hasAppBar && } ); @@ -284,7 +299,7 @@ function DrawerContent(props: PrimaryNavProps) { {props.footerSlot && ( - {props.footerSlot} + {props.footerSlot} )} diff --git a/src/components/navigation/SubNav.stories.tsx b/src/components/navigation/SubNav.stories.tsx index b107dcbc..46da7446 100644 --- a/src/components/navigation/SubNav.stories.tsx +++ b/src/components/navigation/SubNav.stories.tsx @@ -1,6 +1,22 @@ -import { Abc, ArrowForward, GraphicEq } from "@mui/icons-material"; +import { + Abc, + ArrowForward, + GraphicEq, + Insights, + Settings, +} from "@mui/icons-material"; import SearchIcon from "@mui/icons-material/Search"; -import { InputAdornment, TextField } from "@mui/material"; +import { + Box, + InputAdornment, + List, + ListItem, + ListItemButton, + ListItemIcon, + ListItemText, + TextField, + Typography, +} from "@mui/material"; import { SubNavContent } from "./SubNav"; import { Meta, StoryObj } from "@storybook/react"; import React from "react"; @@ -20,7 +36,14 @@ const meta: Meta = { parameters: { docs: { description: { - component: `The content of a contextual secondary navigation panel: a header (title/search slot/back) plus a grouped, optionally-expandable list. Presentation-agnostic - it renders no Drawer or panel of its own. Use SidebarNav's \`subNav\` prop to get the responsive mobile drill-down / desktop side-by-side behaviour around it.`, + component: `The content of a contextual secondary navigation panel: a header (\`title\`/back) plus a grouped, optionally-expandable list. Presentation-agnostic — it renders no Drawer or panel of its own. Use SidebarNav's \`subNav\` prop to get the responsive mobile drill-down / desktop side-by-side behaviour around it. + +- **\`beforeNavSlot\` / \`afterNavSlot\`**: rendered inside the scrollable area, before/after the item list. +- **\`footerSlot\`**: pinned to the bottom of the panel, outside the scrollable area. + +**Using slots** +- There's no dedicated search prop. Put a search field in \`beforeNavSlot\` (see \`WithSlots\`) and filter \`groups\` yourself in response to its value. +- Reach for \`footerSlot\` for persistent actions (like settings or account links) that should stay visible regardless of scroll position; use \`afterNavSlot\` for content that's part of the same scrollable list.`, }, }, }, @@ -55,7 +78,7 @@ export const Basic: Story = { parameters: { docs: { description: { - story: "dense defaults to true - rows are compact by default.", + story: "`dense` defaults to `true` - rows are compact by default.", }, }, }, @@ -69,36 +92,111 @@ export const Comfortable: Story = { parameters: { docs: { description: { - story: "Set dense={false} for taller, more touch-friendly rows.", + story: "Set `dense={false}` for taller, more touch-friendly rows.", }, }, }, }; -export const WithTitleAndSearch: Story = { +export const WithTitle: Story = { + args: { + title: "Experiments", + groups: basicGroups, + }, + parameters: { + docs: { + description: { + story: "Shows the header's `title`, with no back button or slots.", + }, + }, + }, +}; + +/** A dashed, tinted wrapper so it's obvious in the story which content is coming from a slot vs. the `groups` prop. */ +const SlotOutline = ({ + label, + children, +}: { + label: string; + children: React.ReactNode; +}) => ( + + + {label} + + {children} + +); + +const slotListItem = (icon: React.ReactNode, label: string, href: string) => ( + + + + {icon} + + + + +); + +export const WithSlots: Story = { render: () => { const [value, setValue] = React.useState(""); return ( setValue(e.target.value)} - placeholder="Search items" - slotProps={{ - input: { - startAdornment: ( - - - - ), - }, - }} - /> + beforeNavSlot={ + + setValue(e.target.value)} + placeholder="Search items" + sx={{ p: 1 }} + slotProps={{ + input: { + startAdornment: ( + + + + ), + }, + }} + /> + + } + afterNavSlot={ + + {slotListItem( + , + "Documentation", + "https://www.example.com/docs", + )} + + } + footerSlot={ + + {slotListItem( + , + "Settings", + "#settings", + )} + } /> ); @@ -107,7 +205,7 @@ export const WithTitleAndSearch: Story = { docs: { description: { story: - "searchSlot takes any ReactNode - SubNavContent has no search logic of its own, so filtering `groups` in response to the value is entirely up to the consumer.", + "The dashed boxes are only there to highlight what each slot renders - they aren't part of the component. `beforeNavSlot` (here, a search field) and `afterNavSlot` render inside the scrollable area, right before and after the item list; `SubNavContent` has no search logic of its own, so filtering `groups` in response to the value is entirely up to the consumer. `footerSlot` is pinned to the bottom of the panel, outside the scroll area.", }, }, }, @@ -145,6 +243,14 @@ export const GroupedWithSubheaders: Story = { args: { groups: groupedGroups, }, + parameters: { + docs: { + description: { + story: + "Set `subheader` on a group in `groups` to label it - a divider is added between groups automatically.", + }, + }, + }, }; const expandableGroups = [ @@ -174,7 +280,7 @@ export const WithExpandableItems: Story = { 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.", + "One level of expand/collapse only, via each item's `children`. A row with both `linkProps` 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; set `defaultExpanded` to start a row open.", }, }, }, @@ -190,7 +296,7 @@ export const WithBackButton: Story = { docs: { description: { story: - "onBack is normally supplied by SidebarNav on mobile to drill back to the primary sidebar, shown here in isolation.", + "`onBack` is normally supplied by SidebarNav on mobile to drill back to the primary sidebar, shown here in isolation.", }, }, }, diff --git a/src/components/navigation/SubNav.test.tsx b/src/components/navigation/SubNav.test.tsx index 732f1a43..9064afc8 100644 --- a/src/components/navigation/SubNav.test.tsx +++ b/src/components/navigation/SubNav.test.tsx @@ -95,12 +95,12 @@ describe("SubNavContent", () => { expect(screen.queryByRole("textbox")).not.toBeInTheDocument(); }); - it("renders whatever is passed as searchSlot, with no search logic of its own", async () => { + it("renders whatever is passed as beforeNavSlot, with no search logic of its own", async () => { const user = userEvent.setup(); const onChange = vi.fn(); renderSubNavContent({ - searchSlot: ( + beforeNavSlot: ( onChange(e.target.value)} @@ -118,6 +118,30 @@ describe("SubNavContent", () => { expect(screen.getByRole("link", { name: "Setup" })).toBeVisible(); }); + it("renders beforeNavSlot before the item list", () => { + renderSubNavContent({ + beforeNavSlot:
Quick actions
, + }); + + expect(screen.getByTestId("before-content")).toBeVisible(); + }); + + it("renders afterNavSlot after the item list", () => { + renderSubNavContent({ + afterNavSlot:
Extra links
, + }); + + expect(screen.getByTestId("after-content")).toBeVisible(); + }); + + it("renders footerSlot", () => { + renderSubNavContent({ + footerSlot:
User menu
, + }); + + expect(screen.getByTestId("footer")).toBeVisible(); + }); + it("renders a back button when onBack is provided", async () => { const user = userEvent.setup(); const onBack = vi.fn(); diff --git a/src/components/navigation/SubNav.tsx b/src/components/navigation/SubNav.tsx index 0f9838aa..2fba8056 100644 --- a/src/components/navigation/SubNav.tsx +++ b/src/components/navigation/SubNav.tsx @@ -20,7 +20,7 @@ import { type ReactNode, } from "react"; import ArrowBackIcon from "@mui/icons-material/ArrowBack"; -import ExpandMoreIcon from "@mui/icons-material/ExpandMore"; +import { ChevronDown } from "lucide-react"; import type { NavItem, NavItemWithChildren } from "./types"; type SubNavGroup = { @@ -32,13 +32,17 @@ type SubNavGroup = { type SubNavContentProps = { title?: string; - /** Rendered in the header, above the item list; typically a search field. */ - searchSlot?: ReactNode; - groups: SubNavGroup[]; + /** Rendered before the item list, inside the scrollable area - typically a search field. */ + beforeNavSlot?: ReactNode; + /** Rendered after the item list, inside the scrollable area. */ + afterNavSlot?: ReactNode; + /** Rendered pinned to the bottom of the panel, outside the scrollable area. */ + footerSlot?: ReactNode; + /** - * Renders a back affordance above the title/search when provided. + * Renders a back affordance above the title when provided. * SidebarNav supplies this on mobile only; omit for standalone use. */ onBack?: () => void; @@ -48,9 +52,9 @@ type SubNavContentProps = { }; /** - * 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 SidebarNav's job, not this component's. + * Just the contextual nav's content - a header (title/back) plus a grouped, + * optionally-expandable list. Presentation (Drawer vs. side-by-side panel, + * responsive switching) is SidebarNav's job, not this component's. */ function SubNavContent(props: SubNavContentProps) { const dense = props.dense ?? true; @@ -58,7 +62,8 @@ function SubNavContent(props: SubNavContentProps) { return ( - + + {props.beforeNavSlot} {props.groups.map((group, groupIndex) => ( @@ -71,6 +76,7 @@ function SubNavContent(props: SubNavContentProps) { color: "text.secondary", bgcolor: "transparent", lineHeight: 2.5, + pl: 5, }} > {group.subheader} @@ -82,48 +88,42 @@ function SubNavContent(props: SubNavContentProps) { ))} + {props.afterNavSlot} + {props.footerSlot && ( + + + {props.footerSlot} + + )} ); } function SubNavHeader(props: SubNavContentProps) { - const hasHeader = props.onBack || props.title || props.searchSlot; - - if (!hasHeader) { + if (!props.onBack && !props.title) { return null; } return ( - {(props.onBack || props.title) && ( - - {props.onBack && ( - - - - )} - {props.title && ( - - {props.title} - - )} - - )} - - {props.searchSlot} + + {props.onBack && ( + + + + )} + {props.title && ( + + {props.title} + + )} + ); } @@ -138,7 +138,8 @@ function SectionDivider() { function getItemButtonSx(dense: boolean) { return { - p: dense ? 0.5 : 1, + px: 1, + py: dense ? 0.5 : 1, borderRadius: 2, gap: dense ? 1 : 1.5, "&.active, &.Mui-selected": { @@ -207,7 +208,7 @@ function SubNavItem({ theme.transitions.create("transform"), }} > - + ) } @@ -217,7 +218,7 @@ function SubNavItem({ onClick={onRowClick} selected={item.selected} dense={dense} - sx={{ ...buttonSx, pr: hasChildren ? 5 : buttonSx.p }} + sx={{ ...buttonSx, pr: hasChildren ? 5 : buttonSx.px }} aria-label={item.label} > {item.icon && ( @@ -241,7 +242,7 @@ function SubNavItem({ {hasChildren && ( - + {item.children!.map((child, childIndex) => ( ))}