fix: keep a tall menu under its button and scroll it, instead of covering the button

This commit is contained in:
Linus Rath
2026-11-04 05:35:57 +01:00
parent 2525c27a7d
commit 4508eddccb
3 changed files with 51 additions and 9 deletions
@@ -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(
<ContextMenu isOpen position={{ x: 0, y: 0, anchor: { left: 805, top: 12, right: 837, bottom: 44, align: 'end' } }} onClose={() => {}}>
<ContextMenuItem label="Reply" onClick={() => {}} />
</ContextMenu>,
);
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 } });
+19 -5
View File
@@ -242,7 +242,7 @@ function useMenuLevel(recentTouch: () => boolean): MenuLevel {
export const ContextMenu = forwardRef<HTMLDivElement, ContextMenuProps>(
({ isOpen, position, onClose, children, label }, ref) => {
const [mounted, setMounted] = useState(false);
const [adjustedPosition, setAdjustedPosition] = useState<Position | null>(null);
const [adjustedPosition, setAdjustedPosition] = useState<(Position & { maxHeight?: number }) | null>(null);
const localRef = useRef<HTMLDivElement | null>(null);
const onCloseRef = useRef(onClose);
useEffect(() => {
@@ -326,18 +326,25 @@ export const ContextMenu = forwardRef<HTMLDivElement, ContextMenuProps>(
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<HTMLDivElement, ContextMenuProps>(
}}
>
{/* Taller than the window: scroll inside the menu. */}
<div className="py-1 overflow-y-auto" style={{ maxHeight: `calc(100vh - ${2 * VIEWPORT_MARGIN + 2}px)` }}>
<div
className="py-1 overflow-y-auto"
style={{
maxHeight: adjustedPosition?.maxHeight !== undefined
? `${Math.max(0, adjustedPosition.maxHeight - 2)}px`
: `calc(100vh - ${2 * VIEWPORT_MARGIN + 2}px)`,
}}
>
{children}
</div>
</div>
+13 -4
View File
@@ -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;