diff --git a/components/ui/__tests__/context-menu-placement.test.tsx b/components/ui/__tests__/context-menu-placement.test.tsx index a856a4268..d1345b377 100644 --- a/components/ui/__tests__/context-menu-placement.test.tsx +++ b/components/ui/__tests__/context-menu-placement.test.tsx @@ -98,6 +98,25 @@ describe('a menu from a button', () => { expect(openAt({ x: 0, y: 0, anchor: { left: 100, top: 600, right: 132, bottom: 632 } }).top).toBe(600 - 4 - MENU.height); }); + it('too tall for either side: stays under the button, as tall as the room, and scrolls', () => { + // The ⋮ menu at 150 % zoom: 480 CSS pixels of window. + Object.defineProperty(window, 'innerHeight', { value: 480, configurable: true }); + MENU.height = 460; + try { + render( + {}}> + {}} /> + , + ); + const menu = screen.getByRole('menu'); + // Clamped instead, it sat at 10 and covered its own button. + expect(menu.style.top).toBe('48px'); + expect((menu.firstElementChild as HTMLElement).style.maxHeight).toBe(`${480 - 10 - 48 - 2}px`); + } finally { + MENU.height = 300; + } + }); + it('mirrors in RTL: start is the right edge', () => { document.documentElement.dir = 'rtl'; const at = openAt({ x: 0, y: 0, anchor: { left: 500, top: 40, right: 532, bottom: 72 } }); diff --git a/components/ui/context-menu.tsx b/components/ui/context-menu.tsx index 84e0c359c..0498a3dc3 100644 --- a/components/ui/context-menu.tsx +++ b/components/ui/context-menu.tsx @@ -242,7 +242,7 @@ function useMenuLevel(recentTouch: () => boolean): MenuLevel { export const ContextMenu = forwardRef( ({ isOpen, position, onClose, children, label }, ref) => { const [mounted, setMounted] = useState(false); - const [adjustedPosition, setAdjustedPosition] = useState(null); + const [adjustedPosition, setAdjustedPosition] = useState<(Position & { maxHeight?: number }) | null>(null); const localRef = useRef(null); const onCloseRef = useRef(onClose); useEffect(() => { @@ -326,18 +326,25 @@ export const ContextMenu = forwardRef( const clamp = () => { const rect = node.getBoundingClientRect(); + // The height it would have uncapped: a cap set last time mustn't decide the next. + const list = node.firstElementChild as HTMLElement | null; + const height = list && list.scrollHeight > list.clientHeight + ? rect.height + list.scrollHeight - list.clientHeight + : rect.height; // Next to the button it opened from, or at the pointer (lib/menu-placement.ts). - const { x, y } = placeMenu({ + const { x, y, maxHeight } = placeMenu({ point: { x: position.x, y: position.y }, anchor, - size: { width: rect.width, height: rect.height }, + size: { width: rect.width, height }, viewport: { width: window.innerWidth, height: window.innerHeight }, rtl: getComputedStyle(node).direction === "rtl", margin: VIEWPORT_MARGIN, gap: ANCHOR_GAP, }); - setAdjustedPosition((prev) => (prev && prev.x === x && prev.y === y ? prev : { x, y })); + setAdjustedPosition((prev) => + prev && prev.x === x && prev.y === y && prev.maxHeight === maxHeight ? prev : { x, y, maxHeight } + ); }; clamp(); if (typeof ResizeObserver === "undefined") return; @@ -528,7 +535,14 @@ export const ContextMenu = forwardRef( }} > {/* Taller than the window: scroll inside the menu. */} -
+
{children}
diff --git a/lib/menu-placement.ts b/lib/menu-placement.ts index a8397d1ee..146c6b391 100644 --- a/lib/menu-placement.ts +++ b/lib/menu-placement.ts @@ -50,8 +50,9 @@ export function placeMenu({ rtl = false, margin = MENU_VIEWPORT_MARGIN, gap = MENU_ANCHOR_GAP, -}: MenuPlacementInput): { x: number; y: number } { - const { width: w, height: h } = size; +}: MenuPlacementInput): { x: number; y: number; maxHeight?: number } { + const { width: w } = size; + let h = size.height; const { width: vw, height: vh } = viewport; const fitsX = (x: number) => x >= margin && x + w <= vw - margin; const fitsY = (y: number) => y >= margin && y + h <= vh - margin; @@ -69,8 +70,16 @@ export function placeMenu({ const above = anchor.top - gap - h; if (fitsY(below)) y = below; else if (fitsY(above)) y = above; - // Neither fits whole: the side with more room, and the menu scrolls. - else y = vh - anchor.bottom >= anchor.top ? below : above; + else { + // Neither fits whole: the side with more room, as tall as that room, and + // the menu scrolls. Clamped instead, it would cover its own button. + const roomBelow = vh - margin - below; + const roomAbove = anchor.top - gap - margin; + const maxHeight = Math.max(roomBelow, roomAbove); + h = Math.min(h, maxHeight); + y = roomBelow >= roomAbove ? below : anchor.top - gap - h; + return { x: clamp(x, w, vw, margin), y: Math.max(margin, y), maxHeight }; + } } else { const towardEnd = rtl ? point.x - w : point.x; const towardStart = rtl ? point.x : point.x - w;