From 4780107ec4ff945c1b427792f48da2d5a14fa60c Mon Sep 17 00:00:00 2001 From: Gabriel Horacio Cutrini Date: Sun, 11 Oct 2026 16:13:59 -0700 Subject: [PATCH] fix(menu): rebuild the sidebar with an MUI Drawer The sidebar opened on mouse hover, which broke on iPads. After picking an item near the left edge, tapping the strip again did nothing. At exactly 768px wide the menu could not open at all. The menu is now an MUI Drawer with no CSS module. It keeps the old look: a 40px rail with a chevron from 768px up, the burger button below that, and a 260px white panel with no dark backdrop. A tap or click on the rail or burger opens it. A real mouse also opens it by moving onto the rail and closes it by leaving the panel; a hover open leaves focus and page scroll alone. Touch and pen never trigger hover. Flatten the nested @media rule for .primary-layout in landing.css so the 40px page margin also applies on browsers without CSS nesting, such as iPadOS before 16.5. --- src/components/menu/__tests__/menu.test.js | 174 ++++++++++++++++++++- src/components/menu/index.js | 172 ++++++++++++++------ src/components/menu/menu.module.less | 85 ---------- src/i18n/en.json | 2 + src/styles/landing.css | 4 +- 5 files changed, 302 insertions(+), 135 deletions(-) delete mode 100644 src/components/menu/menu.module.less diff --git a/src/components/menu/__tests__/menu.test.js b/src/components/menu/__tests__/menu.test.js index 129e01e4c..c5570aab0 100644 --- a/src/components/menu/__tests__/menu.test.js +++ b/src/components/menu/__tests__/menu.test.js @@ -1,5 +1,12 @@ import React from "react"; -import { render, screen, fireEvent } from "@testing-library/react"; +import { + act, + createEvent, + render, + screen, + fireEvent, + waitFor +} from "@testing-library/react"; import "@testing-library/jest-dom"; import Menu from "../index"; @@ -12,9 +19,11 @@ jest.mock("react-router-dom", () => ({ withRouter: (component) => component })); -jest.mock("../../../models/member", () => jest.fn().mockImplementation(() => ({ +jest.mock("../../../models/member", () => + jest.fn().mockImplementation(() => ({ hasAccess: () => true - }))); + })) +); jest.mock("../menu-definition", () => ({ getGlobalItems: () => [{ name: "directory", linkUrl: "directory" }], @@ -29,6 +38,23 @@ const mockHistory = { const renderMenu = (props = {}) => render(); +// jsdom has no PointerEvent, so pointer fields are set on the event by hand. +// React builds onPointerEnter/onPointerLeave from pointerover/pointerout. +const firePointer = (type, element, fields) => { + const event = createEvent[type](element); + Object.entries(fields).forEach(([key, value]) => + Object.defineProperty(event, key, { value }) + ); + fireEvent(element, event); +}; + +const MOUSE_MOVE = { + pointerType: "mouse", + buttons: 0, + movementX: 5, + movementY: 0 +}; + describe("Menu", () => { beforeEach(() => { jest.clearAllMocks(); @@ -60,4 +86,146 @@ describe("Menu", () => { fireEvent.click(link); expect(mockHistory.push).toHaveBeenCalledWith("/app/directory"); }); + + test("opens the drawer when the menu button is clicked", () => { + renderMenu(); + expect(screen.queryByRole("navigation")).not.toBeInTheDocument(); + fireEvent.click(screen.getByRole("button", { name: "menu.open_menu" })); + expect(screen.getByRole("navigation")).toBeInTheDocument(); + }); + + test("closes the drawer after a menu item is clicked", async () => { + renderMenu(); + fireEvent.click(screen.getByRole("button", { name: "menu.open_menu" })); + fireEvent.click(screen.getByText("menu.directory")); + await waitFor(() => + expect(screen.queryByRole("navigation")).not.toBeInTheDocument() + ); + }); + + describe("rail", () => { + const getRail = () => + screen.getByTestId("ChevronRightIcon").closest("button"); + const getDrawer = () => screen.getByRole("navigation").parentElement; + const isOpen = () => !!screen.queryByRole("navigation"); + + const hoverOpen = () => { + firePointer("pointerOver", getRail(), { pointerType: "mouse" }); + firePointer("pointerMove", getRail(), MOUSE_MOVE); + act(() => jest.advanceTimersByTime(200)); + }; + + beforeEach(() => jest.useFakeTimers()); + afterEach(() => jest.useRealTimers()); + + test("opens the drawer when a mouse moves onto it", () => { + renderMenu(); + firePointer("pointerOver", getRail(), { pointerType: "mouse" }); + firePointer("pointerMove", getRail(), MOUSE_MOVE); + expect(isOpen()).toBe(false); + act(() => jest.advanceTimersByTime(200)); + expect(isOpen()).toBe(true); + }); + + test("does not open the drawer when a mouse leaves it before the delay", () => { + renderMenu(); + firePointer("pointerMove", getRail(), MOUSE_MOVE); + firePointer("pointerOut", getRail(), { pointerType: "mouse" }); + act(() => jest.runOnlyPendingTimers()); + expect(isOpen()).toBe(false); + }); + + test("does not open the drawer when it ends up under a still mouse", () => { + renderMenu(); + firePointer("pointerOver", getRail(), { pointerType: "mouse" }); + firePointer("pointerMove", getRail(), { + ...MOUSE_MOVE, + movementX: 0, + movementY: 0 + }); + act(() => jest.runOnlyPendingTimers()); + expect(isOpen()).toBe(false); + }); + + test("does not open the drawer when a mouse drags over it", () => { + renderMenu(); + firePointer("pointerMove", getRail(), { ...MOUSE_MOVE, buttons: 1 }); + act(() => jest.runOnlyPendingTimers()); + expect(isOpen()).toBe(false); + }); + + test.each(["touch", "pen"])( + "does not open the drawer when a %s pointer passes over it", + (pointerType) => { + renderMenu(); + firePointer("pointerOver", getRail(), { pointerType }); + firePointer("pointerMove", getRail(), { ...MOUSE_MOVE, pointerType }); + act(() => jest.runOnlyPendingTimers()); + expect(isOpen()).toBe(false); + } + ); + + test("opens the drawer when tapped", () => { + renderMenu(); + fireEvent.click(getRail()); + expect(isOpen()).toBe(true); + }); + + test("opens the drawer again after an item is picked", async () => { + renderMenu(); + fireEvent.click(getRail()); + fireEvent.click(screen.getByText("menu.directory")); + await waitFor(() => expect(isOpen()).toBe(false)); + fireEvent.click(getRail()); + expect(isOpen()).toBe(true); + }); + + test("closes the drawer when the mouse leaves it", async () => { + renderMenu(); + fireEvent.click(getRail()); + firePointer("pointerOut", getDrawer(), { pointerType: "mouse" }); + await waitFor(() => expect(isOpen()).toBe(false)); + }); + + test.each(["touch", "pen"])( + "keeps the drawer open when a %s pointer leaves it", + (pointerType) => { + renderMenu(); + fireEvent.click(getRail()); + firePointer("pointerOut", getDrawer(), { pointerType }); + act(() => jest.runOnlyPendingTimers()); + expect(isOpen()).toBe(true); + } + ); + + test("moves focus into the drawer and locks scroll when tapped", () => { + renderMenu(); + fireEvent.click(getRail()); + expect(getDrawer().contains(document.activeElement)).toBe(true); + expect(document.body.style.overflow).toBe("hidden"); + }); + + test("leaves focus and page scroll alone when opened by hover", async () => { + const input = document.createElement("input"); + document.body.appendChild(input); + input.focus(); + renderMenu(); + hoverOpen(); + expect(isOpen()).toBe(true); + expect(document.activeElement).toBe(input); + expect(document.body.style.overflow).not.toBe("hidden"); + firePointer("pointerOut", getDrawer(), { pointerType: "mouse" }); + await waitFor(() => expect(isOpen()).toBe(false)); + expect(document.activeElement).toBe(input); + input.remove(); + }); + + test("clears a pending hover open on unmount", () => { + const { unmount } = renderMenu(); + firePointer("pointerMove", getRail(), MOUSE_MOVE); + expect(jest.getTimerCount()).toBe(1); + unmount(); + expect(jest.getTimerCount()).toBe(0); + }); + }); }); diff --git a/src/components/menu/index.js b/src/components/menu/index.js index 2ad078c55..d0269a4ec 100644 --- a/src/components/menu/index.js +++ b/src/components/menu/index.js @@ -11,11 +11,13 @@ * limitations under the License. * */ -import React, { useState } from "react"; +import React, { useEffect, useRef, useState } from "react"; import T from "i18n-react/dist/i18n-react"; import { withRouter } from "react-router-dom"; import Box from "@mui/material/Box"; +import ButtonBase from "@mui/material/ButtonBase"; import Divider from "@mui/material/Divider"; +import Drawer from "@mui/material/Drawer"; import IconButton from "@mui/material/IconButton"; import MenuIcon from "@mui/icons-material/Menu"; import ChevronRightIcon from "@mui/icons-material/ChevronRight"; @@ -25,15 +27,58 @@ import ExpandableItem from "./expandable-item"; import Member from "../../models/member"; import { getGlobalItems, getSummitItems } from "./menu-definition"; -import styles from "./menu.module.less"; +const DRAWER_WIDTH = 260; +const RAIL_WIDTH = 40; +// Rail from this width up, burger button below. Matches landing.css. +const RAIL_MIN_WIDTH = 768; +// So a quick click on the rail does not land on a menu item sliding in. +const HOVER_OPEN_DELAY_MS = 200; +// Above page content, below Bootstrap 3 modals (1040+) and the Drawer (1200). +const MENU_TRIGGER_Z_INDEX = 1030; + +// Hover only opens and closes the menu for a real mouse or trackpad. Touch and +// pen pointers fire enter/leave on every tap, so they use taps alone. +const isMouse = (ev) => ev.pointerType === "mouse"; const Menu = ({ currentSummit, member, history }) => { - const [menuOpen, setMenuOpen] = useState(false); + // byHover only changes together with open: the Drawer reads it when it opens. + const [menu, setMenu] = useState({ open: false, byHover: false }); + const hoverTimer = useRef(null); const memberObj = new Member(member); const globalItems = getGlobalItems(); const summitItems = currentSummit ? getSummitItems(currentSummit.id) : []; - const closeMenu = () => setMenuOpen(false); + const cancelHoverOpen = () => { + clearTimeout(hoverTimer.current); + hoverTimer.current = null; + }; + + useEffect(() => cancelHoverOpen, []); + + const openMenu = (byHover) => { + cancelHoverOpen(); + setMenu({ open: true, byHover }); + }; + + const onTriggerClick = () => openMenu(false); + + const closeMenu = () => { + cancelHoverOpen(); + setMenu((prev) => ({ ...prev, open: false })); + }; + + // Browsers fire enter, and sometimes a move with no movement, when the rail + // ends up under a still cursor (after the drawer or page loader goes away, or + // a scroll). Only real mouse movement without a button held starts the timer. + const onRailPointerMove = (ev) => { + const stillCursor = ev.movementX === 0 && ev.movementY === 0; + if (!isMouse(ev) || ev.buttons || stillCursor || hoverTimer.current) return; + hoverTimer.current = setTimeout(() => openMenu(true), HOVER_OPEN_DELAY_MS); + }; + + const onDrawerPointerLeave = (ev) => { + if (isMouse(ev)) closeMenu(); + }; const onMenuItemClick = (ev, url) => { ev.preventDefault(); @@ -74,50 +119,85 @@ const Menu = ({ currentSummit, member, history }) => { return ( <> - {menuOpen && ( - - )} - ({ + position: "fixed", + top: 5, + left: 5, + zIndex: MENU_TRIGGER_Z_INDEX, + [theme.breakpoints.up(RAIL_MIN_WIDTH)]: { display: "none" } + })} > - - (menuOpen ? closeMenu() : setMenuOpen(true))} - > - - - - setMenuOpen(true)} - onMouseLeave={() => setMenuOpen(false)} - > - - - - - - - {globalItems.map(drawMenuItem)} - - - {!!currentSummit?.id && ( - <> - - - {summitItems.map(drawMenuItem)} - - - )} - + + + ({ + display: "none", + [theme.breakpoints.up(RAIL_MIN_WIDTH)]: { display: "flex" }, + position: "fixed", + top: 0, + left: 0, + width: RAIL_WIDTH, + height: "100vh", + zIndex: MENU_TRIGGER_Z_INDEX, + bgcolor: "background.paper", + borderRight: 1, + borderColor: "divider", + "&.Mui-focusVisible": { + outline: `2px solid ${theme.palette.primary.main}`, + outlineOffset: "-2px" + } + })} + > + + + + + + {globalItems.map(drawMenuItem)} + + + {!!currentSummit?.id && ( + <> + + + {summitItems.map(drawMenuItem)} + + + )} - + ); }; diff --git a/src/components/menu/menu.module.less b/src/components/menu/menu.module.less deleted file mode 100644 index 5664cc2d9..000000000 --- a/src/components/menu/menu.module.less +++ /dev/null @@ -1,85 +0,0 @@ -.wrapper { - top: 0; - left: 0; - position: fixed; - height: 100vh; - z-index: 99999; - - &.opened { - .burgerButton { - display: none; - } - - .expandButton { - display: none; - } - - .menuWrapper { - width: 260px; - } - - .menuItemsWrapper { - visibility: visible; - } - } - - &.closed { - overflow-y: hidden; - .burgerButton { - display: block; - pointer-events: auto; - - @media (min-width: 768px) { - display: none; - } - } - - .menuWrapper { - width: 0; - - .expandButton { - display: none; - } - - @media (min-width: 768px) { - width: 40px; - - .menuItemsWrapper { - visibility: hidden; - } - - .expandButton { - display: flex; - } - } - } - @media (max-width: 768px) { - pointer-events: none; - } - } - - .menuWrapper { - background-color: #ffffff; - border-right: 1px solid #e0e0e0; - cursor: pointer; - transition: width 200ms linear; - height: 100%; - - .expandButton { - justify-content: center; - align-items: center; - height: 100%; - } - - .menuItemsWrapper { - padding-bottom: 20px; - transition: visibility 200ms linear 200ms; - overflow-y: auto; - height: 100%; - } - } - - .burgerButton { - padding: 5px 0 0 5px; - } -} diff --git a/src/i18n/en.json b/src/i18n/en.json index 16f200582..2bd18bbdf 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -139,6 +139,8 @@ "not_logged_in": "You are not logged in. Please log in to continue:" }, "menu": { + "open_menu": "Open menu", + "main_menu": "Main menu", "general": "General", "directory": "Directory", "dashboard": "Dashboard", diff --git a/src/styles/landing.css b/src/styles/landing.css index f1219725d..fd644a826 100644 --- a/src/styles/landing.css +++ b/src/styles/landing.css @@ -52,8 +52,10 @@ body { .primary-layout { margin: 30px 0; +} - @media (min-width: 768px) { +@media (min-width: 768px) { + .primary-layout { margin: 30px 0 30px 40px; } }