diff --git a/.changeset/menu-popover-sizing.md b/.changeset/menu-popover-sizing.md new file mode 100644 index 000000000..9fc57f063 --- /dev/null +++ b/.changeset/menu-popover-sizing.md @@ -0,0 +1,56 @@ +--- +"@pretable/react": patch +"@pretable/ui": patch +--- + +The grid's list-shaped menus — the tool panel's column kebab (pin placement + +auto width), the `+ Add group` menu and the header's `⋮` — now size to their +own labels, dim the item that is already the current state, and rule off the +mode bit from the commands. + +All three shared `popoverStyle`, which stamps a fixed 240px width: the right +call for `FilterMenu`, a dialog whose form controls stretch to their container, +and wrong for a menu of four short labels, which was drawn as a mostly empty +rectangle spilling well past the grid. Menus now take `menuPopoverStyle` — +content width between a 160px floor and the dialog's 240px, still clamped +horizontally against 240 so the right edge stays safe without measuring. + +The pin menu disables the placement the column is already in, but +`[data-pretable-menu-item]` had no disabled treatment at all: the disabled item +kept the enabled color, the pointer cursor, and the hover highlight, so the one +item that cannot be chosen read as the obvious one to click. It now takes the +tool pane's standard disabled treatment (`--pretable-text-dim`, default +cursor), and the hover rule skips it. + +`ColumnRowMenu` gained a `role="separator"` between the one-shot pin commands +(which close the menu) and the auto-width checkbox (which stays open) — +`[data-pretable-menu-separator]`, styled by `grid.css`, and not a focus stop. + +The same pass over the rest of the portaled surfaces, which cannot inherit +anything from the grid: + +- The header's filter dialog declared `font: inherit`, a shorthand that pulls + the HOST PAGE's size and line-height in — so it drew at the consumer's body + font (16px on our own site) while every other popover sat at + `--pretable-font-size-cell`. It now declares the whole trio, as the enum + listbox and date popover already did, and `[data-pretable-column-menu]` + gained the `color` it was missing. +- The dialog's operator `` were 28px and 33px in a + stacked pair, because Chrome forces `line-height: normal` on a select and + ignores what it is given. Both now take one explicit `block-size`, the way + the tool pane's identical controls already do. +- Those fields kept the UA focus ring, which takes the CONSUMER's + `accent-color` — a different colour, width and offset from the ring on the + same controls in the tool pane. They now take `--pretable-focus-ring`. +- The column-reorder ghost is a copy of a header cell but declared neither + size nor weight and took the cell colour, so the label under the cursor was + bigger and lighter than the column it came from. It now mirrors the header. +- The date editor's month steppers disable at the calendar's min/max month + with no disabled treatment and a live hover accent — the menu items' defect + in a second place. Fixed the same way. + +`grid.css`'s "portaled popovers declare the sans font themselves" guard is why +only half of this was caught: it checked `font-family` alone. It now checks +size and colour too, rejects the `font: inherit` shorthand, and is joined by +guards for the ghost's header type, the disabled treatment, the hover +guards, and the dialog's focus ring — each mutation-tested to fail. diff --git a/packages/react/src/__tests__/popover-position.test.ts b/packages/react/src/__tests__/popover-position.test.ts index dd6ab4481..a9c7a1c2a 100644 --- a/packages/react/src/__tests__/popover-position.test.ts +++ b/packages/react/src/__tests__/popover-position.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, it } from "vitest"; -import { popoverStyle } from "../overlay/popover-position"; +import { menuPopoverStyle, popoverStyle } from "../overlay/popover-position"; const originalWidth = window.innerWidth; const originalHeight = window.innerHeight; @@ -102,3 +102,49 @@ describe("popoverStyle", () => { expect(popoverStyle(rect(100, 200, 120, 300)).maxHeight).toBeUndefined(); }); }); + +describe("menuPopoverStyle", () => { + it("sizes to its content instead of the dialog's fixed column", () => { + setViewport(1024, 768); + const style = menuPopoverStyle(rect(100, 200, 120, 300)); + + // The defect this exists for: a four-item pin menu drawn 240px wide. + expect(style.width).toBe("max-content"); + expect(style.maxWidth).toBe(240); + expect(popoverStyle(rect(100, 200, 120, 300)).width).toBe(240); + }); + + it("keeps a floor, so a one-word menu is still menu-shaped", () => { + setViewport(1024, 768); + expect(menuPopoverStyle(rect(100, 200, 120, 300)).minWidth).toBe(160); + }); + + it("places itself exactly as a dialog does", () => { + setViewport(1024, 768); + const anchor = rect(100, 200, 120, 300); + const { width, minWidth, maxWidth, ...placement } = + menuPopoverStyle(anchor); + const { width: dialogWidth, ...dialogPlacement } = popoverStyle(anchor); + + expect(placement).toEqual(dialogPlacement); + expect(dialogWidth).toBe(240); + expect(width).toBe("max-content"); + expect(minWidth).toBe(160); + expect(maxWidth).toBe(240); + }); + + it("clamps against the widest it could be, never past the right edge", () => { + setViewport(400, 768); + // Same clamp as the dialog: a content-sized menu can only be narrower, + // so the bound holds without measuring the rendered menu. + expect(menuPopoverStyle(rect(100, 380, 120, 400)).left).toBe(152); + }); + + it("flips upward when there is no room below", () => { + setViewport(1024, 768); + const style = menuPopoverStyle(rect(720, 200, 740, 300)); + + expect(style.top).toBeUndefined(); + expect(style.bottom).toBe(768 - 720 + 4); + }); +}); diff --git a/packages/react/src/__tests__/tool-panel.test.tsx b/packages/react/src/__tests__/tool-panel.test.tsx index 1e209652e..9f3cfa38f 100644 --- a/packages/react/src/__tests__/tool-panel.test.tsx +++ b/packages/react/src/__tests__/tool-panel.test.tsx @@ -820,6 +820,31 @@ describe("columns section pin menu", () => { ]); }); + it("rules off the one-shot commands from the mode-bit checkbox", () => { + const h = mountColumnsSection(); + openKebab(h.kebabFor("Bravo")!); + + const menu = document.querySelector("[data-pretable-column-menu]")!; + const separators = menu.querySelectorAll("[data-pretable-menu-separator]"); + expect(separators).toHaveLength(1); + expect(separators[0]).toHaveAttribute("role", "separator"); + + // Between the last command and the checkbox, not anywhere in the list: + // the two kinds of item are what it divides. + const children = Array.from(menu.children); + const items = h.menuItems(); + expect(children.indexOf(separators[0]!)).toBe( + children.indexOf(items[2]!) + 1, + ); + expect(children.indexOf(items[3]!)).toBe( + children.indexOf(separators[0]!) + 1, + ); + + // A separator is not an item: the roving contract queries menu items, so + // it must not become a focus stop. + expect(separators[0]).not.toHaveAttribute("data-pretable-menu-item"); + }); + it("disables the matching pin item for an already-pinned column", () => { const h = mountColumnsSection(); openKebab(h.kebabFor("Alpha")!); diff --git a/packages/react/src/overlay/popover-position.ts b/packages/react/src/overlay/popover-position.ts index 53e782161..fc126f316 100644 --- a/packages/react/src/overlay/popover-position.ts +++ b/packages/react/src/overlay/popover-position.ts @@ -1,6 +1,15 @@ import type { CSSProperties } from "react"; +/** + * The DIALOG width — one column of form controls (`FilterMenu`'s operator + * select over its value input) and the cell editors' panels. It doubles as + * the horizontal-clamp bound for every popover, menus included: clamping + * against the widest a popover can be is what makes the right edge safe + * without measuring anything. + */ const WIDTH = 240; +/** A list-shaped menu narrower than this reads as a stray chip, not a menu. */ +const MENU_MIN_WIDTH = 160; /** Gap between the anchor and the popover. */ const GAP = 4; /** Minimum breathing room kept against every viewport edge. */ @@ -9,7 +18,7 @@ const MARGIN = 8; const MIN_SPACE = 160; /** - * Fixed-position style from the anchor rect. + * Where the popover sits: `position: fixed` coordinates from the anchor rect. * * Horizontally the popover is *clamped* into the viewport (never flipped). * Vertically it opens below the anchor, and flips above it when there is not @@ -18,10 +27,14 @@ const MIN_SPACE = 160; * never has to be measured. No `max-height` is set — each popover's CSS owns * its own height cap. */ -export function popoverStyle(rect: DOMRect): CSSProperties { +function placement(rect: DOMRect): CSSProperties { const vw = typeof window !== "undefined" ? window.innerWidth : 1024; const vh = typeof window !== "undefined" ? window.innerHeight : 768; + // Clamped against WIDTH even for a content-sized menu, which can only be + // narrower: the popover is then further from the right edge than it needed + // to be, never past it. Measuring the real width would mean a layout pass + // and a second paint at a corrected position. const left = Math.min(rect.left, vw - WIDTH - MARGIN); const spaceBelow = vh - rect.bottom - GAP - MARGIN; @@ -32,7 +45,34 @@ export function popoverStyle(rect: DOMRect): CSSProperties { position: "fixed", ...(flip ? { bottom: vh - rect.top + GAP } : { top: rect.bottom + GAP }), left: Math.max(MARGIN, left), - width: WIDTH, zIndex: 50, }; } + +/** + * A DIALOG-shaped popover: a fixed {@link WIDTH} column, because the form + * controls inside it stretch to their container and a shrink-wrapped one + * would be as narrow as its widest option string. + */ +export function popoverStyle(rect: DOMRect): CSSProperties { + return { ...placement(rect), width: WIDTH }; +} + +/** + * A MENU-shaped popover: sized to its own longest label instead of the + * dialog's column. `Pin right` and `Group by this column` are ~60px and + * ~150px of text; both were drawn in a 240px box, which left an item's click + * target and its words in different halves of a mostly empty rectangle. + * + * A floor and a cap rather than free-running content width — the labels are + * caller data (a column header, for `AddGroupMenu`), so neither end can be + * left to them. + */ +export function menuPopoverStyle(rect: DOMRect): CSSProperties { + return { + ...placement(rect), + width: "max-content", + minWidth: MENU_MIN_WIDTH, + maxWidth: WIDTH, + }; +} diff --git a/packages/react/src/pretable-surface.tsx b/packages/react/src/pretable-surface.tsx index 2918f8241..df7741338 100644 --- a/packages/react/src/pretable-surface.tsx +++ b/packages/react/src/pretable-surface.tsx @@ -166,7 +166,7 @@ import { MenuButton } from "./column-menu/MenuButton"; import { FilterMenu, FunnelButton } from "./filter-menu"; import { resolveColumnOptions } from "./filter-menu/filter-operators"; import { OverlayPortal } from "./overlay/OverlayPortal"; -import { popoverStyle } from "./overlay/popover-position"; +import { menuPopoverStyle, popoverStyle } from "./overlay/popover-position"; import { useHeaderPopover } from "./overlay/useHeaderPopover"; import { useHydrated } from "./use-hydrated"; import { @@ -7843,7 +7843,7 @@ export function PretableSurface< columnId={menuOpenState.columnId} grouped={snapshot.rowGroups.includes(menuOpenState.columnId)} label={col.header ?? menuOpenState.columnId} - style={popoverStyle(menuOpenState.rect)} + style={menuPopoverStyle(menuOpenState.rect)} onClose={closePopover} onSelect={selectColumnMenuAction} /> diff --git a/packages/react/src/tool-panel/ColumnRowMenu.tsx b/packages/react/src/tool-panel/ColumnRowMenu.tsx index f8cf50138..c8bb18e88 100644 --- a/packages/react/src/tool-panel/ColumnRowMenu.tsx +++ b/packages/react/src/tool-panel/ColumnRowMenu.tsx @@ -93,6 +93,12 @@ export function ColumnRowMenu({ {messages.toolPanelPinLabel({ pinned: item.pinned })} ))} + {/* Divides the one-shot placement COMMANDS above from the mode bit + below — two kinds of item with two activation behaviors, which + without a rule between them read as one flat list of four. A real + role="separator": `useMenuKeyboard` roves over + [data-pretable-menu-item] only, so it is skipped by construction. */} +
{/* "Let the grid manage this column's width" — a mode bit over the auto-width store, NOT a fit-to-content action (spec B1/Fact 2). The check glyph trails the label so the label's position is diff --git a/packages/react/src/tool-panel/ColumnsSection.tsx b/packages/react/src/tool-panel/ColumnsSection.tsx index 611206276..6c1b0ce89 100644 --- a/packages/react/src/tool-panel/ColumnsSection.tsx +++ b/packages/react/src/tool-panel/ColumnsSection.tsx @@ -14,7 +14,7 @@ import { GROUP_COLUMN_ID } from "@pretable/core"; import { ROW_SELECT_COLUMN_ID } from "../constants"; import { CheckIcon, GripIcon, OverflowIcon } from "../icons"; import type { AutoWidthSetReader } from "../pretable-model"; -import { popoverStyle } from "../overlay/popover-position"; +import { menuPopoverStyle } from "../overlay/popover-position"; import { useHeaderPopover } from "../overlay/useHeaderPopover"; import { ColumnRowMenu } from "./ColumnRowMenu"; import type { ToolPanelColumnsMessages } from "./messages"; @@ -559,7 +559,7 @@ export function ColumnsSection({ label={open.label} pinned={open.entry.pinned ?? null} messages={messages} - style={popoverStyle(menu.rect)} + style={menuPopoverStyle(menu.rect)} onClose={closeMenu} // The menu stays open (its comment carries the checkbox-vs- // command rationale), so no pending-focus arming here: the row diff --git a/packages/react/src/tool-panel/grouping/GroupingSection.tsx b/packages/react/src/tool-panel/grouping/GroupingSection.tsx index 521a39983..a54acb16a 100644 --- a/packages/react/src/tool-panel/grouping/GroupingSection.tsx +++ b/packages/react/src/tool-panel/grouping/GroupingSection.tsx @@ -10,7 +10,7 @@ import { import type { ColumnType } from "@pretable/core"; import { CloseIcon, GripIcon } from "../../icons"; -import { popoverStyle } from "../../overlay/popover-position"; +import { menuPopoverStyle } from "../../overlay/popover-position"; import { useHeaderPopover } from "../../overlay/useHeaderPopover"; import type { GroupingSectionMessages } from "../messages"; import type { ToolDropTarget, ToolRowRect } from "../tool-panel-drop-target"; @@ -459,7 +459,7 @@ export function GroupingSection({ { applyRowGroups([...groupedIds, columnId]); diff --git a/packages/ui/grid.css b/packages/ui/grid.css index 71a5d06ca..a6146b3db 100644 --- a/packages/ui/grid.css +++ b/packages/ui/grid.css @@ -784,10 +784,16 @@ opacity: 0.6; z-index: 10; user-select: none; - /* Portaled into — see the filter menu below. The font-family and - color declared on the scroll viewport are no longer inherited. */ + /* Portaled into — see the filter menu below. Nothing declared on + the scroll viewport is inherited, and what this draws is a copy of a + HEADER CELL, so it takes the header's trio rather than the cell's: the + ghost was rendering the host page's body font (16px/400 against the + header's 12.5px/500 on our own site) in the cell colour, so the label + under the cursor did not match the column it came from. */ font-family: var(--pretable-font-sans); - color: var(--pretable-text-cell); + font-size: var(--pretable-font-size-header); + font-weight: 500; + color: var(--pretable-text-header); } :where([data-pretable-reorder-drop-indicator]) { position: absolute; @@ -889,10 +895,16 @@ gap: 8px; padding: 10px; box-shadow: var(--pretable-shadow-overlay); - font: inherit; - /* Portaled into , so the font-family declared on the scroll - viewport is no longer inherited — declare it here. */ + /* Portaled into , so NOTHING declared on the scroll viewport is + inherited — the whole trio has to be declared here, as every other + portaled surface does. `font: inherit` used to stand in for this and + did the opposite of what it looks like: the shorthand pulled the HOST + PAGE's size and line-height in, so the dialog rendered at whatever the + consumer's body font is (16px on our own site) while every other + popover sat at the cell size. */ font-family: var(--pretable-font-sans); + font-size: var(--pretable-font-size-cell); + color: var(--pretable-text-cell); } :where([data-pretable-filter-menu]) select, :where([data-pretable-filter-menu]) input[type="text"], @@ -900,13 +912,32 @@ :where([data-pretable-filter-menu]) input:not([type]) { width: 100%; box-sizing: border-box; - padding: 5px 7px; + /* An explicit box, like the tool pane's identical controls: Chrome forces + `line-height: normal` on a