From 902bf8d998a3cd8b4bb061657e6e44cdb84cc890 Mon Sep 17 00:00:00 2001 From: Brad Estey Date: Fri, 11 Sep 2026 16:28:58 -0400 Subject: [PATCH 01/10] Fix table width issues. --- AGENTS.md | 117 +++++----- .../app/components/column-filter.test.tsx | 53 +++-- apps/catalog/app/components/column-filter.tsx | 209 ++++++------------ .../app/components/component-table.tsx | 12 +- apps/catalog/app/components/filter-panel.tsx | 177 ++++++++------- .../app/components/part-tool-table.test.tsx | 30 ++- .../app/components/part-tool-table.tsx | 39 +++- apps/catalog/app/shared/column-width.test.ts | 36 +++ apps/catalog/app/shared/column-width.ts | 50 +++++ apps/catalog/app/shared/menu-place.test.ts | 65 ++++++ apps/catalog/app/shared/menu-place.ts | 100 +++++++++ apps/catalog/app/shared/use-fitted-columns.ts | 59 +++++ apps/catalog/app/styles.css | 33 +-- apps/catalog/tests/on-the-part.spec.ts | 151 ++++++++++++- 14 files changed, 789 insertions(+), 342 deletions(-) create mode 100644 apps/catalog/app/shared/column-width.test.ts create mode 100644 apps/catalog/app/shared/column-width.ts create mode 100644 apps/catalog/app/shared/menu-place.test.ts create mode 100644 apps/catalog/app/shared/menu-place.ts create mode 100644 apps/catalog/app/shared/use-fitted-columns.ts diff --git a/AGENTS.md b/AGENTS.md index 7912a49..0022fde 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -264,63 +264,66 @@ application unless that application says otherwise. route, and the pure half is where its rules live. A change to how it behaves is almost always a change to one of these rather than to `routes/part.tsx`: -| Question | Module | -| ---------------------------------------------------- | ---------------------------------------------------- | -| what the list holds, its names, ids, storage | `app/shared/feature-list.ts` | -| which key a row's lines reach the bill under | `sheetKeysOf`, same file | -| which of a row's lines one stack of it wrote | `lineId`, `app/shared/setup-sheet.ts` | -| what is on the order list, for both pages | `app/shared/order-list.ts` | -| whether a row has anything ordered, and what to buy | `isIncomplete` / `componentTotals`, same file | -| which of four things the page is being asked | `asked()`, same file | -| the three presses over the part that add a row | `app/components/add-bar.tsx` | -| whether the presses and the rows are drawn at all | `app/shared/part-chrome.ts` | -| where the part is framed, beside the questions | `app/shared/frame-inset.ts` | -| the heading face and the small-capitals label | `app/shared/type.ts` | -| how tall the tool list opens | `TABLE_OPENS_AT`, `components/part-tool-table.tsx` | -| the columns a list opens with, and their order | `TOOL_COLUMNS`, `components/part-tool-table.tsx` | -| the two columns the list turns on for itself | `app/shared/auto-columns.ts` | -| a row's answer, and what it opens to | `app/shared/recommendations.ts` | -| what a press on a row of the order list opens | `pressRow`, `app/routes/part.tsx` | -| whose stacks the tree beside an open box shows | `editedItem` / `treeKey`, same file | -| what the panel offers for the tool it shows | `app/shared/tool-actions.ts` | -| what fills the tool table, and the cache | `app/shared/catalog-matcher.ts` | -| a stored guid turned back into a record | `getTool`/`getHolder`/`getCollet`, `catalog.ts` | -| what overruling the rules offers, per column | `overridableTools`, `app/shared/tool-fit.ts` | -| the note and press a changed filter raises | `OverrideNotice`, `app/components/column-filter.tsx` | -| whether a value is inside a filter's bound | `withinRange`, `app/shared/filter.ts` | -| what a number box takes besides a number | `app/shared/range-entry.ts` | -| which bounds are somebody's own, not the geometry's | `ownBounds`, `app/shared/filter.ts` | -| which columns' rules an emptied box releases | `releasedBounds`, `app/shared/filter.ts` | -| what a narrowed axis says an unticked value brings | `facetCounts`, `app/shared/catalog-matcher.ts` | -| the same work, off the UI thread | `app/client/catalog-matcher.worker.ts` | -| what a click on the part means | `app/shared/part-interaction.ts` | -| which layer one press of Escape or Enter reaches | `app/shared/use-escape.ts` | -| which list the arrows move through, and the focus | `app/shared/arrow-target.ts` | -| how tall a menu or the column picker is, which way | `menuRoom`, `app/components/column-filter.tsx` | -| whether an open filter survives the list under it | `FilterMenu`, `app/components/column-filter.tsx` | -| a feature's assemblies, its slots, its storage | `app/shared/assembly-tree.ts` | -| what narrows what when any part is chosen first | `app/shared/assembly-narrowing.ts` | -| why an offered chuck cannot be built out of the crib | `colletGap`, same file | -| whether the rack shows them at all, and how many | `holdersToOffer`, same file | -| the press that shows them, hidden to begin with | `app/components/no-collet-toggle.tsx` | -| a group's worst case, and whose it is | `app/shared/group-geometry.ts` | -| how far below the holder a stack has to stand | `belowHolder`, `app/shared/drawn-assembly.ts` | -| which slots were filled against the rules | `overrides`, `app/shared/assembly-tree.ts` | -| what a stack offers, and its button's words | `app/shared/assembly-actions.ts` | -| what a shop calls an assembly, and its field | `renameItem` / `renameAssembly`, `name-field.tsx` | -| reading and filtering a holder or a collet | `app/shared/component-columns.ts` | -| which column header asks which filter | `app/shared/column-filters.ts` | -| a tool in one phrase, with its shank or neck | `app/shared/tool-type.ts` | -| which ticks a filter the page set puts on Type | `typesAsking`, `app/shared/tool-type.ts` | -| which of its columns a tap list narrows on | `askOfTapColumn`, `app/shared/column-filters.ts` | -| what a tick on the tap list's Type column asks | `formsAskingTaps`, `app/shared/hole-mode.ts` | -| what is narrowing a list, named for its button | `narrowingNames`, `app/shared/column-filters.ts` | -| the numbers a thread and a depth put on a tap | `tapBounds`, `app/shared/hole-mode.ts` | -| the form behind a Type phrase, and what it asks | `app/shared/tool-type.ts` | -| the list, its answers and its right-click | `app/components/feature-list-panel.tsx` | -| building a group | `app/components/group-editor.tsx` | -| the reading, its numbers and its thread | `app/components/selection-panel.tsx` | -| the tool table and its marks | `app/components/part-tool-table.tsx` | +| Question | Module | +| ----------------------------------------------------- | ---------------------------------------------------- | +| what the list holds, its names, ids, storage | `app/shared/feature-list.ts` | +| which key a row's lines reach the bill under | `sheetKeysOf`, same file | +| which of a row's lines one stack of it wrote | `lineId`, `app/shared/setup-sheet.ts` | +| what is on the order list, for both pages | `app/shared/order-list.ts` | +| whether a row has anything ordered, and what to buy | `isIncomplete` / `componentTotals`, same file | +| which of four things the page is being asked | `asked()`, same file | +| the three presses over the part that add a row | `app/components/add-bar.tsx` | +| whether the presses and the rows are drawn at all | `app/shared/part-chrome.ts` | +| where the part is framed, beside the questions | `app/shared/frame-inset.ts` | +| the heading face and the small-capitals label | `app/shared/type.ts` | +| how tall the tool list opens | `TABLE_OPENS_AT`, `components/part-tool-table.tsx` | +| the columns a list opens with, and their order | `TOOL_COLUMNS`, `components/part-tool-table.tsx` | +| the two columns the list turns on for itself | `app/shared/auto-columns.ts` | +| how wide a column is, as a share of the panel | `app/shared/column-width.ts` | +| refitting the tracks a drag froze onto the table | `app/shared/use-fitted-columns.ts` | +| a row's answer, and what it opens to | `app/shared/recommendations.ts` | +| what a press on a row of the order list opens | `pressRow`, `app/routes/part.tsx` | +| whose stacks the tree beside an open box shows | `editedItem` / `treeKey`, same file | +| what the panel offers for the tool it shows | `app/shared/tool-actions.ts` | +| what fills the tool table, and the cache | `app/shared/catalog-matcher.ts` | +| a stored guid turned back into a record | `getTool`/`getHolder`/`getCollet`, `catalog.ts` | +| what overruling the rules offers, per column | `overridableTools`, `app/shared/tool-fit.ts` | +| the note and press a changed filter raises | `OverrideNotice`, `app/components/column-filter.tsx` | +| whether a value is inside a filter's bound | `withinRange`, `app/shared/filter.ts` | +| what a number box takes besides a number | `app/shared/range-entry.ts` | +| which bounds are somebody's own, not the geometry's | `ownBounds`, `app/shared/filter.ts` | +| which columns' rules an emptied box releases | `releasedBounds`, `app/shared/filter.ts` | +| what a narrowed axis says an unticked value brings | `facetCounts`, `app/shared/catalog-matcher.ts` | +| the same work, off the UI thread | `app/client/catalog-matcher.worker.ts` | +| what a click on the part means | `app/shared/part-interaction.ts` | +| which layer one press of Escape or Enter reaches | `app/shared/use-escape.ts` | +| which list the arrows move through, and the focus | `app/shared/arrow-target.ts` | +| where a menu opened off a button stands, and how tall | `app/shared/menu-place.ts` | +| keeping an open menu against its button | `app/shared/use-anchored-menu.ts` | +| whether an open filter survives the list under it | `FilterMenu`, `app/components/column-filter.tsx` | +| a feature's assemblies, its slots, its storage | `app/shared/assembly-tree.ts` | +| what narrows what when any part is chosen first | `app/shared/assembly-narrowing.ts` | +| why an offered chuck cannot be built out of the crib | `colletGap`, same file | +| whether the rack shows them at all, and how many | `holdersToOffer`, same file | +| the press that shows them, hidden to begin with | `app/components/no-collet-toggle.tsx` | +| a group's worst case, and whose it is | `app/shared/group-geometry.ts` | +| how far below the holder a stack has to stand | `belowHolder`, `app/shared/drawn-assembly.ts` | +| which slots were filled against the rules | `overrides`, `app/shared/assembly-tree.ts` | +| what a stack offers, and its button's words | `app/shared/assembly-actions.ts` | +| what a shop calls an assembly, and its field | `renameItem` / `renameAssembly`, `name-field.tsx` | +| reading and filtering a holder or a collet | `app/shared/component-columns.ts` | +| which column header asks which filter | `app/shared/column-filters.ts` | +| a tool in one phrase, with its shank or neck | `app/shared/tool-type.ts` | +| which ticks a filter the page set puts on Type | `typesAsking`, `app/shared/tool-type.ts` | +| which of its columns a tap list narrows on | `askOfTapColumn`, `app/shared/column-filters.ts` | +| what a tick on the tap list's Type column asks | `formsAskingTaps`, `app/shared/hole-mode.ts` | +| what is narrowing a list, named for its button | `narrowingNames`, `app/shared/column-filters.ts` | +| the numbers a thread and a depth put on a tap | `tapBounds`, `app/shared/hole-mode.ts` | +| the form behind a Type phrase, and what it asks | `app/shared/tool-type.ts` | +| the list, its answers and its right-click | `app/components/feature-list-panel.tsx` | +| building a group | `app/components/group-editor.tsx` | +| the reading, its numbers and its thread | `app/components/selection-panel.tsx` | +| the tool table and its marks | `app/components/part-tool-table.tsx` | - `docs/` holds planning documents that outlive a single change. `docs/CATALOG-SPEC.md` is the tool catalog specified end to end — how a shop diff --git a/apps/catalog/app/components/column-filter.test.tsx b/apps/catalog/app/components/column-filter.test.tsx index 39f0922..5d70fdd 100644 --- a/apps/catalog/app/components/column-filter.test.tsx +++ b/apps/catalog/app/components/column-filter.test.tsx @@ -10,7 +10,6 @@ import { RangeFilter, TermFilter, compareOf, - menuRoom, optionsMatching, type Bound, type Kind, @@ -230,28 +229,6 @@ describe('the shape a stored bound has', () => { }) }) -/** - * How tall a box opened off a button is, and which way it opens. - * - * One rule for the filter menus and the column picker both: the picker ran off - * the bottom of the screen with its last columns unreachable (Paul, - * 2026-09-10), which is the defect the Type filter had a day earlier. - */ -describe('the room a menu opens into', () => { - it('takes the room under the button and opens downwards', () => { - expect(menuRoom({ top: 100, bottom: 130 }, 900)).toEqual({ upwards: false, height: 758 }) - }) - - it('opens upwards where what is left under the button is a strip', () => { - expect(menuRoom({ top: 700, bottom: 730 }, 900)).toEqual({ upwards: true, height: 688 }) - }) - - /** A strip above and a strip below still opens downwards, and overhangs. */ - it('never squeezes itself below the least height worth reading', () => { - expect(menuRoom({ top: 40, bottom: 70 }, 200)).toEqual({ upwards: false, height: 220 }) - }) -}) - describe('the column picker', () => { it('keeps the pencil at the table header touch target size', () => { render( @@ -268,6 +245,12 @@ describe('the column picker', () => { /** * The list scrolls inside the room the screen leaves it rather than running * off the bottom of the page with its last columns out of reach. + * + * `--available-height` is the kit menu's own measurement of what its + * positioner found, which is what replaced a height this component used to + * work out for itself (Paul, 2026-09-11: "why aren't these menus just using + * the menu component from @toolpath/ui?"). A class rather than an inline + * style, because the number is the positioner's to write. */ it('scrolls inside a height the screen bounds', () => { render( @@ -282,7 +265,29 @@ describe('the column picker', () => { const list = screen.getByRole('group', { name: 'Columns' }) expect(list).toHaveClass('overflow-y-auto') - expect(list.style.maxHeight).not.toBe('') + expect(list).toHaveClass('max-h-[var(--available-height)]') + }) + + /** + * **The box is the kit's, and so is the way out of it.** A picker that drew + * its own absolutely-positioned box was cut off by the card it stood in, and + * every part of the answer — the portal, the placing, Escape, a press + * outside — is what `Menu.Popover` is. + */ + it('opens the kit menu rather than a box of its own', () => { + render( + , + ) + + fireEvent.click(screen.getByRole('button', { name: 'Which columns to show' })) + + expect( + screen.getByRole('group', { name: 'Columns' }).closest('[data-base-ui-portal]'), + ).not.toBe(null) }) }) diff --git a/apps/catalog/app/components/column-filter.tsx b/apps/catalog/app/components/column-filter.tsx index c9bb990..7c7b2bb 100644 --- a/apps/catalog/app/components/column-filter.tsx +++ b/apps/catalog/app/components/column-filter.tsx @@ -1,4 +1,4 @@ -import { Button, Checkbox, IconButton, Input, cn } from '@toolpath/ui' +import { Button, Checkbox, IconButton, Input, Menu, cn } from '@toolpath/ui' import { useEffect, useLayoutEffect, useRef, useState, type ReactNode, type RefObject } from 'react' import { createPortal } from 'react-dom' import { @@ -16,6 +16,7 @@ import { decimalsFor, } from '@toolpath/tool-support' import { movedBy, movedTo } from 'shared/column-order' +import { MENU_GAP, MENU_LEAST, placeMenu, type Placed } from 'shared/menu-place' import { sameBound } from 'shared/filter' import { readEntry, readRange, type Side } from 'shared/range-entry' import { LAYER_COLUMN_FILTER, useEscape, useKeyLayer } from 'shared/use-escape' @@ -406,51 +407,6 @@ export const OverrideNotice = ({ ) } -/** Room kept between the menu and the edge of the screen. */ -const MENU_EDGE = 12 - -/** - * The least room worth opening downwards into. - * - * Below this the menu opens upwards instead. It is a floor on the height as - * well: a menu squeezed into eighty pixels is one nobody can read, so it takes - * this much and overhangs rather than becoming a slot. - */ -const MENU_LEAST = 220 - -/** - * How tall a box opened off a button may be, and which way it opens. - * - * **A menu is as tall as the screen leaves it** (Paul, 2026-09-10, of the Type - * filter and then of the column picker: "the edit columns drop down list should - * be scrollable if it runs off the screen"). Both boxes are opened from a - * header that can sit anywhere down the page, and both were drawn at whatever - * height their contents came to, so the rows past the bottom edge were - * unreachable — the column picker's last column could not be ticked at all. - * - * One rule for both: the room under the button is measured, the box takes it - * and scrolls inside itself, and where what is left under the button is a strip - * it opens upwards into the larger room instead. - */ -export const menuRoom = ( - button: { readonly top: number; readonly bottom: number }, - viewport: number, -): { readonly upwards: boolean; readonly height: number } => { - const below = viewport - button.bottom - MENU_EDGE - const above = button.top - MENU_EDGE - const upwards = below < MENU_LEAST && above > below - return { upwards, height: Math.max(MENU_LEAST, upwards ? above : below) } -} - -/** Where the menu stands: by its top, or by its bottom where it opened upwards. */ -type Placed = { - readonly top: number | null - readonly bottom: number | null - readonly left: number - /** The most it may be, which is the room the screen left it. */ - readonly height: number -} - /** * The box a column's funnel opens, drawn over the page and away from the table. * @@ -563,23 +519,16 @@ export const FilterMenu = ({ }) return } - const button = anchor.getBoundingClientRect() - const width = box.current?.getBoundingClientRect().width ?? 0 - const wanted = align === 'right' ? button.right - width : button.left - const left = Math.max(8, Math.min(wanted, window.innerWidth - width - 8)) - /* - `menuRoom` is the rule — and where it says upwards the menu is anchored - by its bottom rather than placed by a height it has not been measured at - yet, which is the one way to flip a box without a frame of it in the - wrong place. - */ - const room = menuRoom(button, window.innerHeight) - setAt({ - top: room.upwards ? null : button.bottom + 4, - bottom: room.upwards ? window.innerHeight - button.top + 4 : null, - left, - height: room.height, - }) + // `shared/menu-place` is the rule, and the same one the picker and the + // quick filters follow — this menu only finds its button the hard way. + setAt( + placeMenu( + anchor.getBoundingClientRect(), + box.current?.getBoundingClientRect().width ?? 0, + align, + { width: window.innerWidth, height: window.innerHeight }, + ), + ) } /** @@ -1104,9 +1053,6 @@ export const ColumnPicker = ({ }) => { const [open, setOpen] = useState(false) const [held, setHeld] = useState(null) - const box = useRef(null) - const pencil = useRef(null) - const [room, setRoom] = useState({ upwards: false, height: MENU_LEAST }) const order = columns.map((column) => column.code) const move = (code: string, index: number) => { @@ -1116,84 +1062,69 @@ export const ColumnPicker = ({ } } - useEffect(() => { - if (!open) { - return - } - const onDown = (event: PointerEvent) => { - if (!box.current?.contains(event.target as Node)) { - setOpen(false) - } - } - document.addEventListener('pointerdown', onDown) - return () => document.removeEventListener('pointerdown', onDown) - }, [open]) - /* - The list is as long as the table has columns — twenty on the tool list — and - the pencil is at the top of a table that can sit anywhere down the page, so - the bottom of the list ran off the screen and the columns there could not be - ticked. `menuRoom` is the same rule the filter menus follow: take the room - the screen leaves and scroll inside it, or open upwards where what is under - the pencil is a strip. + **The kit's `Menu`, rather than a box drawn under the pencil** (Paul, + 2026-09-11: "the 'which columns to show' menu is now hidden behind the table + when opened", then "why aren't these menus just using the menu component + from @toolpath/ui?"). The strip this button stands on floats over the + viewer, which is a card that clips, so a box positioned inside it was cut + off at the card's bottom edge with the tool list showing through the rest of + it. Every part of the answer — the portal out of the card, the placing + against the pencil, turning over where the room is above, the height the + screen leaves, Escape, and a press outside — is what `Menu.Popover` already + is, and this had a hand-written half of each. */ - useLayoutEffect(() => { - if (!open) { - return - } - const measure = () => { - const button = pencil.current?.getBoundingClientRect() - if (button !== undefined) { - setRoom(menuRoom(button, window.innerHeight)) - } - } - // A scroll inside the list is the list's own business, exactly as it is - // inside a filter menu. - const onScroll = (event: Event) => { - if (event.target instanceof Node && box.current?.contains(event.target) === true) { - return - } - measure() - } - measure() - window.addEventListener('resize', measure) - window.addEventListener('scroll', onScroll, true) - return () => { - window.removeEventListener('resize', measure) - window.removeEventListener('scroll', onScroll, true) - } - }, [open]) - - // Escape puts it away as well, without going back to find the header. - useEscape(open, () => setOpen(false)) - return ( -
- setOpen(!open)} - /* **A press keeps its own ground** (Paul, 2026-09-11: "the buttons - shouldn't be transparent"). The chrome this stands in floats over the - part now, so the pencil wears the same chip the buttons beside it - wear rather than sitting bare on the geometry. */ - className="rounded border border-zinc-800 bg-zinc-900 p-1 text-zinc-400 transition hover:bg-zinc-800 hover:text-zinc-200" + + {/* + The pencil *is* the trigger. `Menu.Trigger` renders a `role="button"` + div of its own by default, and a button inside that is two controls + with one name — `filter-panel.tsx` says the same thing at more length. + */} + + + + } + /> + {/* + **Under its button, not flipped above it** (Paul, 2026-09-11: "don't + just show the menus above, that's a lazy solution"). This strip sits two + thirds of the way down the page, so a menu free to pick the roomier side + picks *above* every time and hangs over the part rather than over the + list it narrows. Pinned to the bottom, it opens where a menu belongs and + scrolls inside `--available-height`, which is the positioner's own + measurement of what is left there — and a hair off the pencil, so the + two read as a control and its answer. + */} + - - - {open ? ( + {/* + `--available-height` is the positioner's own measurement of the room + it found, so the list scrolls inside what the screen left rather than + running off the bottom of the page with its last columns out of reach + (Paul, 2026-09-10). + */}
{columns.map((column, at) => (
))}
- ) : null} -
+ + ) } diff --git a/apps/catalog/app/components/component-table.tsx b/apps/catalog/app/components/component-table.tsx index dde339f..8b22c73 100644 --- a/apps/catalog/app/components/component-table.tsx +++ b/apps/catalog/app/components/component-table.tsx @@ -12,6 +12,8 @@ import { Table, cn } from '@toolpath/ui' import type { Collet, Holder } from '@toolpath/catalog-data' import type { UnitSystem } from '@toolpath/tool-support' import { orderedCodes } from 'shared/column-order' +import { DEFAULT_COLUMN_WIDTH, fillingWidth } from 'shared/column-width' +import { useFittedColumns } from 'shared/use-fitted-columns' import { colletTypeLabel, familyLabel, @@ -146,7 +148,8 @@ interface Selection { readonly ids: Array } -const flexible = (width: string): string => `minmax(${width}, 1fr)` +/** The grid track a column asks for — `shared/column-width` owns the rule. */ +const flexible = fillingWidth const columnsShown = ( columns: ReadonlyArray, @@ -239,6 +242,8 @@ export const ComponentTable = ({ /** The open column, or nothing where it has since been hidden. */ const openColumn = shown.find((column) => column.code === openFilter) ?? null const inside = useRef(null) + // The columns divide the panel, the same rule the tool list keeps. + useFittedColumns(inside, shown.map((column) => column.code).join(' ')) /** * Whose move the selection was — the same guard `PartToolTable` keeps, and * for the same reason: without it the row the tree already holds is reported @@ -339,7 +344,7 @@ export const ComponentTable = ({ return String(a).localeCompare(String(b), 'en', { numeric: true }) }) } - width={flexible(WIDTH[column.code] ?? '6rem')} + width={flexible(WIDTH[column.code] ?? DEFAULT_COLUMN_WIDTH)} > {heading(column.code, column.label)} @@ -354,8 +359,8 @@ export const ComponentTable = ({ className={cn(TABLE_FACE, TABLE_INK, 'flex min-h-0 min-w-0 flex-1 flex-col')} >
+ {/* No stored layout and no `min-w-max`: `PartToolTable` says why. */} void children: ReactNode }) => { - const menu = useRef(null) - const press = useRef(null) - const [menuOffset, setMenuOffset] = useState(0) - /** - * Which way it opens, and the most it may be. - * - * **These buttons stand at the bottom of the viewer now** (Paul, 2026-09-11), - * so a box opening downwards opens past the edge of a viewer that clips, and - * what is under the button is a strip. `menuRoom` is the same rule the column - * funnels and the column picker follow: take the room the screen leaves, and - * turn over where there is none. - */ - const [room, setRoom] = useState<{ readonly upwards: boolean; readonly height: number }>({ - upwards: false, - height: 0, - }) - - useLayoutEffect(() => { - if (!open || menu.current === null) { - setMenuOffset(0) - return - } - - const place = () => { - const element = menu.current - if (element === null) { - return - } - const menuRect = element.getBoundingClientRect() - const chrome = element.closest('[data-list-chrome]')?.getBoundingClientRect() - const left = Math.max(chrome?.left ?? 0, 8) + 8 - const right = Math.min(chrome?.right ?? window.innerWidth, window.innerWidth) - 8 - const correction = - menuRect.left < left - ? left - menuRect.left - : menuRect.right > right - ? right - menuRect.right - : 0 - setMenuOffset(correction) - const button = press.current?.getBoundingClientRect() - if (button !== undefined) { - setRoom(menuRoom(button, window.innerHeight)) - } - } - - place() - window.addEventListener('resize', place) - return () => window.removeEventListener('resize', place) - }, [open]) + /* + **The kit's `Menu`, rather than a box drawn under the button** (Paul, + 2026-09-11: "the 'part material' menu", then "why aren't these menus just + using the menu component from @toolpath/ui?"). These buttons stand on the + strip that floats over the bottom of the viewer, and the viewer is a card + that clips — so the box was cut off at the card's edge whichever way it + opened, with the tool list showing through the rest of it. `Menu.Popover` + is a portal placed against its trigger and bounded by the window, which is + every part of the answer. + Controlled, because which filter is open is the toolbar's business: opening + one closes the last. + */ return ( -
- + } + /> + {/* + **Under its button, not flipped above it** (Paul, 2026-09-11: "don't + just show the menus above, that's a lazy solution"). This strip sits two + thirds of the way down the page, so a menu free to pick the roomier side + picks *above* every time and hangs over the part rather than over the + list it narrows. Pinned to the bottom, it opens where a menu belongs and + scrolls inside `--available-height`, which is the positioner's own + measurement of what is left there — and a hair off the button, so the + two read as a control and its answer. + */} + - {icon} - - {summary === 'Any' ? label : `${label}: ${summary}`} - - - - {open ? ( -
- {children} -
- ) : null} -
+ {children} + + ) } @@ -626,7 +611,21 @@ const useCloseOnOutside = (open: boolean, close: () => void) => { return } const onDown = (event: PointerEvent) => { - if (!box.current?.contains(event.target as Node)) { + const target = event.target instanceof Element ? event.target : null + if (target === null) { + close() + return + } + /* + **A menu drawn on the page is still inside the strip that opened it.** + `ToolbarFilterBody` puts its box in a portal so the viewer card cannot + clip it, which takes it out of this element — and a press on the thing + somebody just opened read as a press outside and shut it again. + */ + if (target.closest('[data-tool-filter-menu]') !== null) { + return + } + if (!box.current?.contains(target)) { close() } } diff --git a/apps/catalog/app/components/part-tool-table.test.tsx b/apps/catalog/app/components/part-tool-table.test.tsx index f0a7bbf..80f7bc6 100644 --- a/apps/catalog/app/components/part-tool-table.test.tsx +++ b/apps/catalog/app/components/part-tool-table.test.tsx @@ -181,18 +181,40 @@ describe('PartToolTable', () => { expect(screen.queryByText('holder needs')).not.toBeInTheDocument() }) - it('keeps the table grid wider than its scroll container', () => { + /** + * **The grid is never wider than the box it is read in** (Paul, 2026-09-11). + * It used to be, by `min-w-max` here and a `min-width: max-content` in + * `app/styles.css`, and under max-content sizing every `1fr` track came out + * at the widest floor in the map — thirteen 192px columns in a 1169px panel. + */ + it('lets the scroll container size the table grid', () => { + show({ + columns: TOOL_COLUMNS, + hiddenColumns: [], + columnOrder: TOOL_COLUMNS.map((column) => column.code), + }) + + expect(document.querySelector('[data-table-library_table]')).not.toHaveClass('min-w-max') + }) + + /** + * **Nothing about a column's width is remembered between visits.** The kit + * stores a dragged layout under the `id` it is given and hands it back on the + * next mount, which is a saved answer to a question a column being shown or + * hidden has already changed. + */ + it('gives the kit no id to store a column layout under', () => { show({ columns: TOOL_COLUMNS, hiddenColumns: [], columnOrder: TOOL_COLUMNS.map((column) => column.code), }) - expect(document.querySelector('[data-table-library_table]')).toHaveClass('min-w-max') + expect(Object.keys(localStorage).filter((key) => key.startsWith('table-'))).toHaveLength(0) }) - it('uses flexible tracks for initial column widths', () => { - expect(flexibleColumnWidth('10rem')).toBe('minmax(10rem, 1fr)') + it('asks for tracks that divide the panel rather than floors under it', () => { + expect(flexibleColumnWidth('10rem')).toBe('minmax(0, 10fr)') }) }) diff --git a/apps/catalog/app/components/part-tool-table.tsx b/apps/catalog/app/components/part-tool-table.tsx index d70f724..bc7dbf8 100644 --- a/apps/catalog/app/components/part-tool-table.tsx +++ b/apps/catalog/app/components/part-tool-table.tsx @@ -19,6 +19,8 @@ import type { ToolQuery } from 'shared/filter' import { markWords, type Mark } from 'shared/tool-marks' import type { BelowHolder } from 'shared/drawn-assembly' import { orderedCodes } from 'shared/column-order' +import { DEFAULT_COLUMN_WIDTH, fillingWidth } from 'shared/column-width' +import { useFittedColumns } from 'shared/use-fitted-columns' import { ToolTypeIcon } from './tool-icons' import { ColumnFilterMenu, @@ -101,7 +103,13 @@ export const TAP_COLUMNS: ReadonlyArray = [ export const hiddenByDefault = (columns: ReadonlyArray): Array => columns.filter((column) => !column.default).map((column) => column.code) -export const flexibleColumnWidth = (width: string): string => `minmax(${width}, 1fr)` +/** + * The grid track a column asks for — `shared/column-width` owns the rule. + * + * Kept as a name here because the header reads as a column asking for a width, + * and `components/component-table.tsx` asks the same module the same thing. + */ +export const flexibleColumnWidth = fillingWidth /** * A row of the list, in pixels — `@toolpath/ui`'s compact `Table`. @@ -168,14 +176,14 @@ export const isStack = (code: string): boolean => code === 'LBH' export const isIdentity = (code: string): boolean => IDENTITY.some((column) => column.code === code) /** - * How wide a column starts, by what it holds rather than by its numbers. + * How wide a column is, by what it holds rather than by its numbers. * - * **Only the largest of these is doing anything.** `@toolpath/ui`'s table gives - * every column the width of the widest `minmax()` floor it is handed, so - * raising one entry here raises all thirteen — measured on 2026-09-11 by - * setting `type` to `20rem` and watching each column become 320px. The map - * reads as a per-column decision and is not one. Left as it was rather than - * tuned around, because the column sizing is the kit's to fix. + * **Read as a share of the panel, not as a floor under it** — the rem is a + * weight and `shared/column-width` is the rule. Until 2026-09-11 only the + * largest entry here did anything: every column came out at the width of the + * widest `minmax()` floor in the map, so the list opened 2120px wide inside a + * 1169px panel with thirteen 192px columns. Raising one entry now widens that + * column and narrows the rest. */ const WIDTH: Readonly> = { catalogNumber: '10rem', @@ -468,6 +476,9 @@ export const PartToolTable = ({ /** The open column, or nothing where it has since been hidden. */ const openColumn = shown.find((column) => column.code === openFilter) ?? null const inside = useRef(null) + // The columns divide the panel; anything the kit's resizer froze onto it goes + // when the panel or the column set changes. + useFittedColumns(inside, shown.map((column) => column.code).join(' ')) const selectionCameFromTable = useRef(false) const setSelection = useCallback((next: SetStateAction) => { setSelectedRows((current) => { @@ -583,7 +594,7 @@ export const PartToolTable = ({ : String(a).localeCompare(String(b), 'en', { numeric: true }) }) } - width={flexibleColumnWidth(WIDTH[column.code] ?? '6rem')} + width={flexibleColumnWidth(WIDTH[column.code] ?? DEFAULT_COLUMN_WIDTH)} > {heading(column.code, column.label)} @@ -598,8 +609,15 @@ export const PartToolTable = ({ className={cn(TABLE_FACE, TABLE_INK, 'flex min-h-0 min-w-0 flex-1 flex-col')} >
+ {/* + **No `id`, and no `min-w-max`** (Paul, 2026-09-11). The kit stores a + dragged layout under its `id` and hands it back on the next visit, + which is a saved answer to a question — how wide is a column — that a + column being shown or hidden has already changed. And `min-w-max` was + half of what made the list open wider than its panel; + `shared/column-width` is the whole story. + */}
{ + it('asks for a share of the box rather than a floor under it', () => { + expect(fillingWidth('10rem')).toBe('minmax(0, 10fr)') + expect(fillingWidth('6rem')).toBe('minmax(0, 6fr)') + }) + + it('reads the parts a column asks for out of its rem', () => { + expect(columnWeight('12rem')).toBe(12) + expect(columnWeight('7.5rem')).toBe(7.5) + }) + + /** + * A width map is read here and nowhere else, so anything that is not a plain + * rem weighs what an unstated column does rather than becoming a second, + * silent sizing rule. + */ + it('weighs anything that is not a rem as an unstated column', () => { + expect(columnWeight('160px')).toBe(DEFAULT_WEIGHT) + expect(columnWeight('20%')).toBe(DEFAULT_WEIGHT) + expect(columnWeight('0rem')).toBe(DEFAULT_WEIGHT) + expect(columnWeight('')).toBe(DEFAULT_WEIGHT) + }) +}) diff --git a/apps/catalog/app/shared/column-width.ts b/apps/catalog/app/shared/column-width.ts new file mode 100644 index 0000000..cf59cd7 --- /dev/null +++ b/apps/catalog/app/shared/column-width.ts @@ -0,0 +1,50 @@ +/** + * How wide a column is, and why the list always ends at the edge of its box. + * + * **The columns divide the room they have; they do not ask for room and + * overflow it** (Paul, 2026-09-11). What a list used to be handed was + * `minmax(10rem, 1fr)` — a floor in rem and an equal share of whatever was + * left — under a table forced to `min-width: max-content`. Under max-content + * sizing every `1fr` track resolves to the *widest* floor it was handed, so the + * thirteen-column tool list opened 2120 px wide inside a 1169 px panel, with + * every column 192 px whatever the map beside it said. Measured on 2026-09-11. + * The list then snapped to fit the moment somebody touched a resize handle, + * because the kit's resizer rewrites the tracks as percentages of the box — + * which is the layout it should have opened at. + * + * So the rem in a width map is read as a **weight** rather than as a floor: + * `minmax(0, 10fr)` next to `minmax(0, 6fr)` is a catalogue number ten parts + * wide beside a flute count of six, out of whatever the panel has. Two things + * follow from the zero floor, and both are wanted: + * + * - the tracks always sum to the width of the box, at any size, with no + * horizontal scrollbar and no gutter reserved for one; and + * - the proportions in the map finally do something, where before only the + * largest entry in it did. + * + * A cell that runs out of room truncates — the kit's cells are `overflow: + * hidden` with an ellipsis, and the words a column can lose are on rows that + * carry a `title`. + */ + +/** The parts a column with nothing said about it asks for. */ +export const DEFAULT_WEIGHT = 6 + +/** What a column with nothing said about it asks for. */ +export const DEFAULT_COLUMN_WIDTH = `${DEFAULT_WEIGHT}rem` + +/** + * The parts of the box a column asks for. + * + * Anything that is not a plain rem length weighs what an unstated column does: + * a width map is read by this module alone, so a `px` or a `%` in one would be + * a silent third sizing rule rather than something to honour. + */ +export const columnWeight = (width: string): number => { + const rem = /^\s*([\d.]+)rem\s*$/.exec(width) + const stated = rem === null ? Number.NaN : Number(rem[1]) + return Number.isFinite(stated) && stated > 0 ? stated : DEFAULT_WEIGHT +} + +/** The grid track a column asks for: its share of the box, never more than it. */ +export const fillingWidth = (width: string): string => `minmax(0, ${columnWeight(width)}fr)` diff --git a/apps/catalog/app/shared/menu-place.test.ts b/apps/catalog/app/shared/menu-place.test.ts new file mode 100644 index 0000000..19d7c8e --- /dev/null +++ b/apps/catalog/app/shared/menu-place.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it } from 'vitest' +import { menuRoom, placeMenu } from './menu-place' + +/** + * How tall a box opened off a button is, and which way it opens. + * + * One rule for the filter menus, the column picker and the quick filters: the + * picker ran off the bottom of the screen with its last columns unreachable + * (Paul, 2026-09-10), which is the defect the Type filter had a day earlier. + */ +describe('the room a menu opens into', () => { + it('takes the room under the button and opens downwards', () => { + expect(menuRoom({ top: 100, bottom: 130 }, 900)).toEqual({ upwards: false, height: 758 }) + }) + + it('opens upwards where what is left under the button is a strip', () => { + expect(menuRoom({ top: 700, bottom: 730 }, 900)).toEqual({ upwards: true, height: 688 }) + }) + + /** A strip above and a strip below still opens downwards, and overhangs. */ + it('never squeezes itself below the least height worth reading', () => { + expect(menuRoom({ top: 40, bottom: 70 }, 200)).toEqual({ upwards: false, height: 220 }) + }) +}) + +/** + * Where the box stands, in viewport pixels. + * + * These are the numbers a `position: fixed` portal is given, which is why the + * menus that used to be drawn inside the strip that opened them are no longer + * cut off by the viewer card (Paul, 2026-09-11). + */ +describe('placing a menu against its button', () => { + const viewport = { width: 1000, height: 900 } + const button = { top: 100, bottom: 130, left: 400, right: 500 } + + it('hangs a downward menu off the bottom of the button', () => { + expect(placeMenu(button, 160, 'right', viewport)).toEqual({ + top: 134, + bottom: null, + left: 340, + height: 758, + }) + }) + + /** Anchored by its bottom, so a box flips without a frame in the wrong place. */ + it('stands an upward menu on the top of the button', () => { + expect(placeMenu({ ...button, top: 700, bottom: 730 }, 160, 'right', viewport)).toEqual({ + top: null, + bottom: 204, + left: 340, + height: 688, + }) + }) + + it('lines a left-aligned menu up with the near edge instead', () => { + expect(placeMenu(button, 160, 'left', viewport).left).toBe(400) + }) + + /** A menu on the last column opens leftwards rather than off the screen. */ + it('keeps the box inside the window at either edge', () => { + expect(placeMenu({ ...button, left: 960, right: 990 }, 160, 'left', viewport).left).toBe(832) + expect(placeMenu({ ...button, left: 4, right: 30 }, 160, 'right', viewport).left).toBe(8) + }) +}) diff --git a/apps/catalog/app/shared/menu-place.ts b/apps/catalog/app/shared/menu-place.ts new file mode 100644 index 0000000..aea6a98 --- /dev/null +++ b/apps/catalog/app/shared/menu-place.ts @@ -0,0 +1,100 @@ +/** + * Where a box opened off a button stands, and how tall it may be. + * + * **One rule, for every menu on the part screen.** The column funnels, the + * column picker and the quick filters over the bottom of the viewer all open a + * box against a button that can be anywhere on the page, and each had grown its + * own half of the answer — which is how the picker and the quick filters ended + * up drawn *inside* the card that clips them (Paul, 2026-09-11: "the 'which + * columns to show' menu is now hidden behind the table when opened. Same with + * the 'part material' menu"). A menu belongs to the page, not to the thing it + * was opened from — which is why the picker and the quick filters are the kit's + * `Menu` now, and why the column funnels, which cannot be (they re-find an + * anchor the virtualized table rebuilds under them), are placed by this. + */ + +/** Room kept between the menu and the edge of the screen. */ +export const MENU_EDGE = 12 + +/** + * The least room worth opening downwards into. + * + * Below this the menu opens upwards instead. It is a floor on the height as + * well: a menu squeezed into eighty pixels is one nobody can read, so it takes + * this much and overhangs rather than becoming a slot. + */ +export const MENU_LEAST = 220 + +/** The gap between the button and the box it opened. */ +export const MENU_GAP = 4 + +/** + * How tall a box opened off a button may be, and which way it opens. + * + * **A menu is as tall as the screen leaves it** (Paul, 2026-09-10, of the Type + * filter and then of the column picker: "the edit columns drop down list should + * be scrollable if it runs off the screen"). Both boxes are opened from a + * header that can sit anywhere down the page, and both were drawn at whatever + * height their contents came to, so the rows past the bottom edge were + * unreachable — the column picker's last column could not be ticked at all. + * + * One rule for both: the room under the button is measured, the box takes it + * and scrolls inside itself, and where what is left under the button is a strip + * it opens upwards into the larger room instead. + * + * **The screen is what runs out, and only the screen.** That is true because a + * menu placed by {@link placeMenu} is drawn in a portal on the page: a box + * inside the viewer card would be cut off at the card's edge long before it ran + * out of window, and measuring against the window would be a lie about it. + */ +export const menuRoom = ( + button: { readonly top: number; readonly bottom: number }, + viewport: number, +): { readonly upwards: boolean; readonly height: number } => { + const below = viewport - button.bottom - MENU_EDGE + const above = button.top - MENU_EDGE + const upwards = below < MENU_LEAST && above > below + return { upwards, height: Math.max(MENU_LEAST, upwards ? above : below) } +} + +/** Where the menu stands: by its top, or by its bottom where it opened upwards. */ +export interface Placed { + readonly top: number | null + readonly bottom: number | null + readonly left: number + /** The most it may be, which is the room the screen left it. */ + readonly height: number +} + +/** + * The whole placement of a menu, in viewport pixels — a `position: fixed` box. + * + * Where {@link menuRoom} says upwards, the box is anchored by its **bottom** + * rather than placed by a height it has not been measured at yet, which is the + * one way to flip a box without a frame of it in the wrong place. + * + * @param button the box the menu opened from. + * @param width what the menu has measured, or 0 before it has been drawn once. + * @param align which edge of the button the menu lines up with, so a menu on + * the last column opens leftwards instead of off the screen. + */ +export const placeMenu = ( + button: { + readonly top: number + readonly bottom: number + readonly left: number + readonly right: number + }, + width: number, + align: 'left' | 'right', + viewport: { readonly width: number; readonly height: number }, +): Placed => { + const wanted = align === 'right' ? button.right - width : button.left + const room = menuRoom(button, viewport.height) + return { + top: room.upwards ? null : button.bottom + MENU_GAP, + bottom: room.upwards ? viewport.height - button.top + MENU_GAP : null, + left: Math.max(8, Math.min(wanted, viewport.width - width - 8)), + height: room.height, + } +} diff --git a/apps/catalog/app/shared/use-fitted-columns.ts b/apps/catalog/app/shared/use-fitted-columns.ts new file mode 100644 index 0000000..79d181f --- /dev/null +++ b/apps/catalog/app/shared/use-fitted-columns.ts @@ -0,0 +1,59 @@ +import { useCallback, useEffect, type RefObject } from 'react' + +/** + * What the kit's resize handle leaves behind, and when it has to go. + * + * Dragging a column edge does not go through React at all: `@toolpath/ui`'s + * table is `@table-library/react-table-library` underneath, and its resizer + * writes the tracks straight onto the table element as an inline custom + * property — percentages of the box, measured at the width the box had while + * the mouse was down. An inline property beats the class `shared/column-width` + * hands its tracks to, so from the first drag onwards the list is laid out by + * that frozen string and nothing else. + * + * Two things then make it wrong rather than merely stale: + * + * - **a column is shown or hidden**, and the string still names the old + * columns — eleven percentages over twelve tracks, so every column after the + * change is the width of its neighbour and the last of them is unclaimed; and + * - **the box changes size**, where the percentages hold but the widths + * somebody dragged were chosen against a panel that is no longer that size. + * + * Both are answered the same way: drop the inline property and let the tracks + * the list asked for take over, which is the layout it opens at. A drag is + * therefore kept until one of those two happens and not after — that is the + * trade, and it is the right way round, because a list that fits its box is + * what every column in it is read from. + * + * A `ResizeObserver` rather than a window listener: the panel is resized by the + * order list folding away and by the tool drawing beside it as much as by the + * window, and none of those raise a `resize` event. + * + * @param inside the element the list is drawn in — the table is found under it. + * @param columns what the shown columns are, in order, as one string. Any + * change to it refits, so it has to name the columns rather than count them. + */ +export const useFittedColumns = (inside: RefObject, columns: string): void => { + const refit = useCallback(() => { + const table = inside.current?.querySelector('[data-table-library_table]') + if (!(table instanceof HTMLElement)) { + return + } + table.style.removeProperty('--data-table-library_grid-template-columns') + }, [inside]) + + useEffect(() => { + refit() + }, [refit, columns]) + + useEffect(() => { + const element = inside.current + // jsdom has no ResizeObserver, and a component test has nothing to observe. + if (element === null || typeof ResizeObserver === 'undefined') { + return + } + const observer = new ResizeObserver(() => refit()) + observer.observe(element) + return () => observer.disconnect() + }, [inside, refit]) +} diff --git a/apps/catalog/app/styles.css b/apps/catalog/app/styles.css index d2e1152..d3bd44a 100644 --- a/apps/catalog/app/styles.css +++ b/apps/catalog/app/styles.css @@ -150,20 +150,27 @@ html:not(.dark) body { .filter-off { background-color: var(--color-zinc-950); } +} - /* The UI table hides its scrollbar by default; the part table needs its full width reachable. */ - [data-part-tool-table] .hide-scrollbar { - overflow-x: auto !important; - scrollbar-gutter: stable; - scrollbar-width: auto; - } - [data-part-tool-table] .hide-scrollbar::-webkit-scrollbar { - display: block; - height: 0.75rem; +@layer utilities { + /** + * `hide-scrollbar`, which the kit asks for and nothing defined. + * + * `@toolpath/ui`'s table draws its scroll box as `overflow-x-scroll + * hide-scrollbar` and leaves the class to the application; with no rule + * behind it Chromium reserved a classic scrollbar anyway, which is the 16px + * of dead ground down the right of the tool list (Paul, 2026-09-11: "a + * padding on the container element that is the size of a scrollbar"). + * + * Defining it rather than fighting it: the columns divide the panel now — + * `shared/column-width` — so a list has nothing to scroll sideways to, and + * this application's own overrides that widened the table past its box and + * reserved a gutter for the overflow came out with it. + */ + .hide-scrollbar { + scrollbar-width: none; } - - /* The UI table's fixed tracks must be wider than the viewport, not clipped to it. */ - [data-part-tool-table] [data-table-library_table] { - min-width: max-content; + .hide-scrollbar::-webkit-scrollbar { + display: none; } } diff --git a/apps/catalog/tests/on-the-part.spec.ts b/apps/catalog/tests/on-the-part.spec.ts index 2d845c2..5208306 100644 --- a/apps/catalog/tests/on-the-part.spec.ts +++ b/apps/catalog/tests/on-the-part.spec.ts @@ -732,10 +732,12 @@ test('filters open from the bar floating over the bottom of the part', async ({ expect(toolbarBox!.y + toolbarBox!.height).toBeLessThanOrEqual(rowsBox!.y) }).toPass() + // And the rows under it end where the panel ends — see "the columns divide + // the panel" below, which is where that rule is pinned. const tableScroll = page.locator('[data-part-tool-table] .hide-scrollbar').first() await expect(tableScroll).toBeVisible() expect(await tableScroll.evaluate((element) => element.scrollWidth > element.clientWidth)).toBe( - true, + false, ) // What no column shows is on the toolbar, answerable without a press first. @@ -761,6 +763,153 @@ test('filters open from the bar floating over the bottom of the part', async ({ await expect(types.getByRole('checkbox', { name: 'Circle segment taper' })).toHaveCount(0) }) +/** + * The tracks the list is laid out on, and whether they fit the box it is in. + * + * Read off the scroll container rather than off the header cells, because what + * went wrong was the *sum*: every column came out at 192px whatever it asked + * for, and the thirteen of them added up to 2120px inside an 1169px panel. + */ +const tableFit = (page: Page) => + page.evaluate(() => { + const holder = document.querySelector('[data-part-tool-table]') + const scroller = holder?.querySelector('.hide-scrollbar') + const table = holder?.querySelector('[data-table-library_table]') + if ( + !(scroller instanceof HTMLElement) || + !(table instanceof HTMLElement) || + !(holder instanceof HTMLElement) + ) { + throw new Error('the tool list is on screen') + } + return { + // What the columns add up to, against the room they have. + table: table.offsetWidth, + room: scroller.clientWidth, + // The gutter a scrollbar reserves, which should be none of it. + gutter: scroller.offsetWidth - scroller.clientWidth, + columns: holder.querySelectorAll('[role="columnheader"]').length, + } + }) + +/** + * **The columns divide the panel; they do not overflow it** (Paul, 2026-09-11: + * "on load, the table extends outside of the bounds of the container. On + * clicking to resize a column all of the columns then snap to fit"). + * + * Both halves of that were true and neither was a coincidence. The list asked + * for `minmax(10rem, 1fr)` tracks under a table pinned to `min-width: + * max-content`, and under max-content sizing every `1fr` track resolves to the + * *widest* floor it was handed — so thirteen columns opened at 192px each, + * 2120px of them inside an 1169px panel, with only the largest entry in the + * width map doing anything at all. Touching a resize handle then rewrote the + * tracks as percentages of the box, which is the layout it should have opened + * at: the fix is to open at it. `app/shared/column-width.ts` is the rule. + * + * Three moments, because the layout is settled in three different ways: by CSS + * on load, by CSS again when the window changes, and by + * `shared/use-fitted-columns` after a drag has frozen a layout onto the table + * that the column set has since outgrown. A list that fits on load and breaks + * on the first column somebody hides is the defect this is here for. + */ +test('the columns divide the panel, at every width and column set', async ({ page }) => { + await ready(page) + await keepFeature(page) + await expect(page.getByRole('grid').first().getByRole('row').nth(1)).toBeVisible() + + const opened = await tableFit(page) + expect(opened.table).toBe(opened.room) + // No scrollbar, and so no strip of dead ground reserved for one. + expect(opened.gutter).toBe(0) + + await page.setViewportSize({ width: 1200, height: 1000 }) + await expect(async () => { + const resized = await tableFit(page) + expect(resized.room).toBeLessThan(opened.room) + expect(resized.table).toBe(resized.room) + }).toPass() + + /* + A drag first, because dragging is what freezes a layout onto the table: + the kit's resizer writes the tracks inline, outside React, and they name + the columns that were there when the mouse went down. + */ + const handle = page.locator('[data-part-tool-table] .resizer-area').first() + const grip = await handle.boundingBox() + expect(grip).not.toBeNull() + await page.mouse.move(grip!.x + grip!.width / 2, grip!.y + grip!.height / 2) + await page.mouse.down() + await page.mouse.move(grip!.x + grip!.width / 2 + 40, grip!.y + grip!.height / 2, { steps: 5 }) + await page.mouse.up() + + const dragged = await tableFit(page) + expect(dragged.table).toBe(dragged.room) + + await page.getByRole('button', { name: 'Which columns to show' }).first().click() + const columns = page.getByRole('group', { name: 'Columns' }).first() + await expect(columns).toBeVisible() + await columns.getByRole('checkbox', { name: 'Flutes' }).click() + await page.keyboard.press('Escape') + + await expect(async () => { + const fewer = await tableFit(page) + expect(fewer.columns).toBe(dragged.columns - 1) + expect(fewer.table).toBe(fewer.room) + }).toPass() +}) + +/** + * **A menu belongs to the page, not to the card it was opened from** (Paul, + * 2026-09-11: "the 'which columns to show' menu is now hidden behind the table + * when opened. Same with the 'part material' menu"). + * + * The strip carrying both of these buttons floats over the bottom of the + * viewer, and the viewer is a card with `overflow: hidden` — so each box was + * cut off at the card's bottom edge, with the tool list showing through where + * the rest of it should have been. Opening them upwards would have hidden that + * rather than fixed it: a menu with half the window under it belongs under its + * button. Both are portals placed by `shared/menu-place` now, the arrangement + * the column funnels already needed. + * + * Asked by hit-testing rather than by reading a `z-index`, because what was + * wrong was a clip and not a stacking order, and neither is visible in a style. + */ +test('the menus over the part are drawn over the table, not clipped by the card', async ({ + page, +}) => { + await ready(page) + await keepFeature(page) + await expect(page.getByRole('grid').first().getByRole('row').nth(1)).toBeVisible() + + /** What is actually painted at the middle of the box, and at its bottom edge. */ + const reaches = (menu: Locator) => + menu.evaluate((box) => { + const rect = box.getBoundingClientRect() + const hits = (y: number) => document.elementFromPoint(rect.x + rect.width / 2, y) + return { + middle: box.contains(hits(rect.y + rect.height / 2)), + // One pixel inside the bottom edge — where a clip takes the box away. + bottom: box.contains(hits(rect.bottom - 1)), + height: rect.height, + } + }) + + await page.getByRole('button', { name: 'Which columns to show' }).first().click() + const columns = page.getByRole('group', { name: 'Columns' }).first() + await expect(columns).toBeVisible() + expect(await reaches(columns)).toMatchObject({ middle: true, bottom: true }) + await page.keyboard.press('Escape') + + await page.getByRole('button', { name: 'Part material' }).first().click() + const material = page.locator('[data-tool-filter-menu]').first() + await expect(material).toBeVisible() + expect(await reaches(material)).toMatchObject({ middle: true, bottom: true }) + + // And it is still a menu: a press inside it answers rather than closing it. + await material.getByRole('button', { name: /Steel/ }).click() + await expect(material).toBeVisible() +}) + /** * **One grouping, one column** (Paul, 2026-09-08: "product line and family are * the same and need to be rolled into one Family field. This should be a From 58f4900093be45038e9e8a68b4aa669a4ae0b3a8 Mon Sep 17 00:00:00 2001 From: Brad Estey Date: Fri, 11 Sep 2026 16:32:18 -0400 Subject: [PATCH 02/10] Track column visibility selections in local storage. --- apps/catalog/app/routes/part.tsx | 96 +++----- apps/catalog/app/shared/column-layout.test.ts | 95 ++++++++ apps/catalog/app/shared/column-layout.ts | 217 ++++++++++++++++++ 3 files changed, 346 insertions(+), 62 deletions(-) create mode 100644 apps/catalog/app/shared/column-layout.test.ts create mode 100644 apps/catalog/app/shared/column-layout.ts diff --git a/apps/catalog/app/routes/part.tsx b/apps/catalog/app/routes/part.tsx index 2287418..0f888f8 100644 --- a/apps/catalog/app/routes/part.tsx +++ b/apps/catalog/app/routes/part.tsx @@ -98,6 +98,7 @@ import { import { ColumnPicker } from 'components/column-filter' import { BUTTON_FILTERS } from 'components/filter-panel' import { orderedCodes } from 'shared/column-order' +import { COLUMN_KEY, useColumnLayout } from 'shared/column-layout' import { hiddenAfterAuto } from 'shared/auto-columns' import { capRows, firstBy, keptFirst, oneEach } from 'shared/tool-order' import { @@ -530,9 +531,25 @@ const Inspecting = ({ report, jobId }: { report: PublicInspectionReport; jobId: at: DOMRect featureTag?: string } | null>(null) - const [hiddenColumns, setHiddenColumns] = useState>( - hiddenByDefault(TOOL_COLUMNS), - ) + /** + * The tool list's columns — which are shown, in what order, remembered. + * + * `shared/column-layout.ts` owns the storing and, more to the point, what a + * stored answer means once the catalog's columns have moved under it. + */ + const toolLayout = useColumnLayout(COLUMN_KEY.tools, TOOL_COLUMNS) + const { + hidden: hiddenColumns, + order: columnOrder, + /** + * The columns somebody has decided for themselves. + * + * Tip angle and corner radius follow the list — `shared/auto-columns.ts` + * is the rule — and a code in here is one the list stops deciding about. + */ + touched: touchedColumns, + setHidden: setHiddenColumns, + } = toolLayout /** * The tap list's columns, kept apart from the tool list's. * @@ -540,25 +557,10 @@ const Inspecting = ({ report, jobId }: { report: PublicInspectionReport; jobId: * point angle — so they cannot share one hidden set: a code hidden in one * would mean nothing in the other, and the picker in the corner edits * whichever list is open (Paul, 2026-09-02: "allow me to use those columns - * if I edit the tap table"). - */ - const [hiddenTapColumns, setHiddenTapColumns] = useState>( - hiddenByDefault(TAP_COLUMNS), - ) - const [tapColumnOrder, setTapColumnOrder] = useState>(() => - TAP_COLUMNS.map((column) => column.code), - ) - /** - * The columns somebody has decided for themselves. - * - * Tip angle and corner radius follow the list — `shared/auto-columns.ts` is - * the rule — and a code in here is one the list stops deciding about. + * if I edit the tap table"). A separate key, for the same reason. */ - const touchedColumns = useRef(new Set()) - /** The order the columns are drawn in, dragged in the column picker. */ - const [columnOrder, setColumnOrder] = useState>(() => - TOOL_COLUMNS.map((column) => column.code), - ) + const tapLayout = useColumnLayout(COLUMN_KEY.taps, TAP_COLUMNS) + const { hidden: hiddenTapColumns, order: tapColumnOrder } = tapLayout /** Narrowing the list by catalog number, as typed into the first column. */ const [numberSearch, setNumberSearch] = useState('') @@ -1716,18 +1718,11 @@ const Inspecting = ({ report, jobId }: { report: PublicInspectionReport; jobId: * pressed it does not want it undone by clearing a taper. */ const [noCollet, setNoCollet] = useState(false) - const [hiddenHolderColumns, setHiddenHolderColumns] = useState>(() => - hiddenComponentColumns(HOLDER_COLUMNS), - ) - const [holderColumnOrder, setHolderColumnOrder] = useState>(() => - HOLDER_COLUMNS.map((column) => column.code), - ) - const [hiddenColletColumns, setHiddenColletColumns] = useState>(() => - hiddenComponentColumns(COLLET_COLUMNS), - ) - const [colletColumnOrder, setColletColumnOrder] = useState>(() => - COLLET_COLUMNS.map((column) => column.code), - ) + /** The holder and collet lists' columns, each remembered under its own key. */ + const holderLayout = useColumnLayout(COLUMN_KEY.holders, HOLDER_COLUMNS) + const { hidden: hiddenHolderColumns, order: holderColumnOrder } = holderLayout + const colletLayout = useColumnLayout(COLUMN_KEY.collets, COLLET_COLUMNS) + const { hidden: hiddenColletColumns, order: colletColumnOrder } = colletLayout /** * Whose tree is on screen. @@ -2368,8 +2363,8 @@ const Inspecting = ({ report, jobId }: { report: PublicInspectionReport; jobId: */ const listedForms = useMemo(() => listed.map((each) => each.form), [listed]) useEffect(() => { - setHiddenColumns((current) => hiddenAfterAuto(current, listedForms, touchedColumns.current)) - }, [listedForms]) + setHiddenColumns((current) => hiddenAfterAuto(current, listedForms, new Set(touchedColumns))) + }, [listedForms, touchedColumns, setHiddenColumns]) /** * The list narrowed by what was typed into the catalog number column. * @@ -5923,16 +5918,8 @@ const Inspecting = ({ report, jobId }: { report: PublicInspectionReport; jobId: ).includes(column.code), ) .map((column) => column.code)} - onToggle={(code) => { - const set = - componentSlot === 'holder' ? setHiddenHolderColumns : setHiddenColletColumns - set((current) => - current.includes(code) - ? current.filter((each) => each !== code) - : [...current, code], - ) - }} - onReorder={componentSlot === 'holder' ? setHolderColumnOrder : setColletColumnOrder} + onToggle={componentSlot === 'holder' ? holderLayout.toggle : colletLayout.toggle} + onReorder={componentSlot === 'holder' ? holderLayout.reorder : colletLayout.reorder} /> ) : ( column.code)} - onToggle={(code) => { - if (tappingNow) { - setHiddenTapColumns((current) => - current.includes(code) - ? current.filter((each) => each !== code) - : [...current, code], - ) - return - } - touchedColumns.current.add(code) - setHiddenColumns((current) => - current.includes(code) - ? current.filter((each) => each !== code) - : [...current, code], - ) - }} - onReorder={tappingNow ? setTapColumnOrder : setColumnOrder} + onToggle={tappingNow ? tapLayout.toggle : toolLayout.toggle} + onReorder={tappingNow ? tapLayout.reorder : toolLayout.reorder} /> ) } diff --git a/apps/catalog/app/shared/column-layout.test.ts b/apps/catalog/app/shared/column-layout.test.ts new file mode 100644 index 0000000..dc31baf --- /dev/null +++ b/apps/catalog/app/shared/column-layout.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from 'vitest' +import { defaultLayout, readLayout, reconciled, written, type LayoutColumn } from './column-layout' + +/** + * Which columns a list shows, and in what order, across a reload. + * + * The whole risk in storing this is the *second* visit: a stored answer is + * about the columns that existed when it was written, and this catalog's column + * sets move. Column widths were taken out of storage on 2026-09-11 for exactly + * that reason — the kit stores them positionally, so hiding one re-applies + * every width to the wrong column. Codes survive that, but only if what comes + * back is reconciled rather than trusted, which is what this pins. + */ +const COLUMNS: ReadonlyArray = [ + { code: 'catalogNumber', default: true }, + { code: 'brand', default: true }, + { code: 'DC', default: true }, + { code: 'RE', default: false }, +] + +describe('the columns a list shows', () => { + it('starts with the columns that open by default, in the declared order', () => { + expect(defaultLayout(COLUMNS)).toEqual({ + hidden: ['RE'], + order: ['catalogNumber', 'brand', 'DC', 'RE'], + touched: [], + }) + }) + + it('gives a browser that has stored nothing the defaults', () => { + expect(readLayout(null, COLUMNS)).toEqual(defaultLayout(COLUMNS)) + }) + + /** A half-written or hand-edited key is not worth a blank screen. */ + it('falls back to the defaults on anything it cannot read', () => { + expect(readLayout('{"hidden":', COLUMNS)).toEqual(defaultLayout(COLUMNS)) + expect(readLayout('"a string"', COLUMNS)).toEqual(defaultLayout(COLUMNS)) + expect(readLayout('{"hidden":{"DC":true},"order":7}', COLUMNS)).toEqual(defaultLayout(COLUMNS)) + }) + + it('gives back exactly what was stored where nothing has changed', () => { + const kept = { + hidden: ['brand'], + order: ['DC', 'catalogNumber', 'brand', 'RE'], + touched: ['brand', 'RE'], + } + + expect(readLayout(written(kept, COLUMNS), COLUMNS)).toEqual(kept) + }) + + /** A code nothing draws is an answer about nothing. */ + it('drops a column the catalog no longer has', () => { + const stored = { + hidden: ['brand', 'LCF'], + order: ['LCF', 'DC', 'catalogNumber', 'brand', 'RE'], + touched: ['LCF'], + known: ['catalogNumber', 'brand', 'DC', 'RE', 'LCF'], + } + + expect(reconciled(stored, COLUMNS)).toEqual({ + hidden: ['brand'], + order: ['DC', 'catalogNumber', 'brand', 'RE'], + touched: [], + }) + }) + + /** + * **`known` is what tells a new column from one somebody left showing.** + * Without it every browser that had ever opened the picker would get the next + * off-by-default column turned on, and keep it on. + */ + it('gives a column added since the layout was saved its own default', () => { + const later: ReadonlyArray = [ + ...COLUMNS, + { code: 'SIG', default: false }, + { code: 'NOF', default: true }, + ] + const stored = written({ hidden: ['RE'], order: ['DC', 'catalogNumber'], touched: [] }, COLUMNS) + + expect(readLayout(stored, later)).toEqual({ + hidden: ['RE', 'SIG'], + // Appended, which neither drops it nor pretends somebody placed it. + order: ['DC', 'catalogNumber', 'brand', 'RE', 'SIG', 'NOF'], + touched: [], + }) + }) + + /** A column somebody unhid stays unhidden when a later build adds others. */ + it('keeps a column the shop turned on when the column set grows', () => { + const later: ReadonlyArray = [...COLUMNS, { code: 'SIG', default: false }] + const stored = written({ hidden: [], order: [], touched: ['RE'] }, COLUMNS) + + expect(readLayout(stored, later)).toMatchObject({ hidden: ['SIG'], touched: ['RE'] }) + }) +}) diff --git a/apps/catalog/app/shared/column-layout.ts b/apps/catalog/app/shared/column-layout.ts new file mode 100644 index 0000000..24527f2 --- /dev/null +++ b/apps/catalog/app/shared/column-layout.ts @@ -0,0 +1,217 @@ +import { useCallback, useEffect, useRef, useState } from 'react' +import { orderedCodes } from './column-order' + +/** + * Which columns a list shows, and in what order — remembered per browser. + * + * **A shop sets its columns once** (Paul, 2026-09-11: "save column order and + * visibility in local storage"). Cutting a thirteen-column list down to the + * five somebody compares on, and dragging them into the order they read them + * in, is a decision about how this shop works rather than about this part, and + * it was thrown away on every reload. + * + * **This is the opposite call to the one made about column *widths* on the same + * day, and the difference is what a stored answer is worth when the columns + * change.** A width is stored as a track list positional in the columns that + * existed when it was dragged, so hiding one silently re-applies every width to + * the wrong column — the kit's own storage did exactly that, which is why this + * application gives its tables no `id` to store under. What is stored here is + * *codes*, so every stored answer still names the column it was about however + * the catalog's column set moves under it. + * + * That is reconciliation, not luck, and {@link reconciled} is where it happens. + */ + +/** A column, as far as this module needs to know one. */ +export interface LayoutColumn { + readonly code: string + readonly default: boolean +} + +export interface ColumnLayout { + /** The codes not drawn. */ + readonly hidden: ReadonlyArray + /** Every code, in the order the table draws them. */ + readonly order: ReadonlyArray + /** + * The codes somebody has decided for themselves. + * + * Tip angle and corner radius otherwise follow what is on the list — + * `shared/auto-columns.ts` is that rule — and this is what stops the list + * deciding about a column after somebody has. Stored with the rest, because + * a hand-toggled tip angle that comes back off on the next reload is the + * visibility this exists to keep. + */ + readonly touched: ReadonlyArray +} + +/** + * What is written down: the layout, plus the columns it was made about. + * + * `known` is the load-bearing field. Without it a column added to the catalog + * after somebody saved a layout cannot be told apart from one they deliberately + * left showing — so a new column that is meant to be off by default would come + * on for everybody who had ever opened the picker, and stay on. + */ +interface Stored extends ColumnLayout { + readonly known: ReadonlyArray +} + +const codes = (columns: ReadonlyArray): Array => + columns.map((column) => column.code) + +/** The layout a browser that has never been here makes. */ +export const defaultLayout = (columns: ReadonlyArray): ColumnLayout => ({ + hidden: columns.filter((column) => !column.default).map((column) => column.code), + order: codes(columns), + touched: [], +}) + +const strings = (value: unknown): Array => + Array.isArray(value) ? value.filter((each): each is string => typeof each === 'string') : [] + +/** + * A stored layout, answered against the columns this build actually has. + * + * Three things can have changed between the write and the read, and each has + * one honest answer: + * + * - **a column is gone** — drop it from all three lists, since a code nothing + * draws is an answer about nothing; + * - **a column is new** — it takes its own default, which is what `known` is + * for, and it goes on the end of the order (`orderedCodes`, which neither + * drops it nor pretends somebody put it there); and + * - **nothing changed** — the layout comes back exactly as it was left. + */ +export const reconciled = ( + stored: Partial, + columns: ReadonlyArray, +): ColumnLayout => { + const here = new Set(codes(columns)) + const known = new Set(strings(stored.known)) + const kept = strings(stored.hidden).filter((code) => here.has(code)) + const fresh = columns + .filter((column) => !known.has(column.code) && !column.default && !kept.includes(column.code)) + .map((column) => column.code) + return { + hidden: [...kept, ...fresh], + order: orderedCodes(codes(columns), strings(stored.order)), + touched: strings(stored.touched).filter((code) => here.has(code)), + } +} + +/** What a key holds, or the columns' own defaults where it holds nothing usable. */ +export const readLayout = ( + raw: string | null, + columns: ReadonlyArray, +): ColumnLayout => { + if (raw === null || raw === '') { + return defaultLayout(columns) + } + try { + const parsed: unknown = JSON.parse(raw) + if (typeof parsed !== 'object' || parsed === null) { + return defaultLayout(columns) + } + return reconciled(parsed as Partial, columns) + } catch { + return defaultLayout(columns) + } +} + +/** The record to write: the layout, and the columns it was made about. */ +export const written = (layout: ColumnLayout, columns: ReadonlyArray): string => + JSON.stringify({ ...layout, known: codes(columns) } satisfies Stored) + +/** Where each list's layout is kept. One key per list, because one list's columns are not another's. */ +export const COLUMN_KEY = { + tools: 'tool-catalog.columns.tools', + taps: 'tool-catalog.columns.taps', + holders: 'tool-catalog.columns.holders', + collets: 'tool-catalog.columns.collets', +} as const + +/** + * One list's columns, remembered. + * + * **Read in an effect rather than in the initial state**, which is what the + * rest of this application's stored preferences do: the catalog is built with + * `ssr: false` but React Router still renders the shell once at build time, and + * state that differs between that render and the browser's first one is a + * hydration mismatch. The cost is that the defaults are drawn for one frame. + * + * @param key one of {@link COLUMN_KEY}. + * @param columns every column this list can draw, in the order they are + * declared — the order a browser that has never been here gets. + */ +export const useColumnLayout = (key: string, columns: ReadonlyArray) => { + const [layout, setLayout] = useState(() => defaultLayout(columns)) + /* + Nothing is written until something has been read. Otherwise the first + change of any kind — including the automatic one the tool list makes for + tip angle and corner radius, which runs on the first list — would write the + defaults over a real stored layout before the effect below had read it. + */ + const loaded = useRef(false) + + useEffect(() => { + setLayout(readLayout(globalThis.localStorage?.getItem(key) ?? null, columns)) + loaded.current = true + // The columns of a given list are a module constant; the key is what says + // which list this is. + }, [key]) + + const keep = useCallback( + (next: (current: ColumnLayout) => ColumnLayout) => { + setLayout((current) => { + const settled = next(current) + if (settled === current) { + return current + } + if (loaded.current) { + globalThis.localStorage?.setItem(key, written(settled, columns)) + } + return settled + }) + }, + [key, columns], + ) + + /** A column shown or hidden, and marked as somebody's own decision. */ + const toggle = useCallback( + (code: string) => { + keep((current) => ({ + ...current, + hidden: current.hidden.includes(code) + ? current.hidden.filter((each) => each !== code) + : [...current.hidden, code], + touched: current.touched.includes(code) ? current.touched : [...current.touched, code], + })) + }, + [keep], + ) + + const reorder = useCallback( + (order: ReadonlyArray) => { + keep((current) => ({ ...current, order })) + }, + [keep], + ) + + /** + * The hidden set rewritten by a rule rather than by a press — + * `hiddenAfterAuto`. Returning the same array leaves the stored layout + * alone, which is what keeps a rule that decided nothing out of storage. + */ + const setHidden = useCallback( + (next: (hidden: ReadonlyArray) => ReadonlyArray) => { + keep((current) => { + const hidden = next(current.hidden) + return hidden === current.hidden ? current : { ...current, hidden } + }) + }, + [keep], + ) + + return { ...layout, toggle, reorder, setHidden } +} From 2d490bb4501279978a637597a65a8dca1912077d Mon Sep 17 00:00:00 2001 From: Brad Estey Date: Fri, 11 Sep 2026 16:32:35 -0400 Subject: [PATCH 03/10] Another test. --- apps/catalog/tests/on-the-part.spec.ts | 57 ++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/apps/catalog/tests/on-the-part.spec.ts b/apps/catalog/tests/on-the-part.spec.ts index 5208306..81d5168 100644 --- a/apps/catalog/tests/on-the-part.spec.ts +++ b/apps/catalog/tests/on-the-part.spec.ts @@ -858,6 +858,63 @@ test('the columns divide the panel, at every width and column set', async ({ pag }).toPass() }) +/** + * **A shop sets its columns once** (Paul, 2026-09-11: "save column order and + * visibility in local storage"). + * + * Cutting a thirteen-column list down to what somebody compares on, and + * dragging those into the order they read them in, is a decision about how the + * shop works rather than about this part — and it was thrown away on every + * reload. `app/shared/column-layout.ts` is the rule, and its own tests cover + * what a stored answer means once the catalog's columns have moved under it; + * this is the half only a real browser can answer: that the write happens, that + * the read happens, and that the table drawn afterwards is the stored one. + * + * **Deliberately not what happens to a column's *width*.** That is not stored, + * because the kit stores widths positionally and hiding one column re-applies + * every width to the wrong column — see "the columns divide the panel" above. + */ +test('remembers which columns are shown, and their order, across a reload', async ({ page }) => { + await ready(page) + await keepFeature(page) + await expect(page.getByRole('grid').first().getByRole('row').nth(1)).toBeVisible() + + const headings = () => + page.locator('[data-part-tool-table]').first().getByRole('columnheader').allInnerTexts() + + const opened = await headings() + expect(opened.some((heading) => heading.includes('Flutes'))).toBe(true) + expect(opened.some((heading) => heading.includes('Shank'))).toBe(false) + + await page.getByRole('button', { name: 'Which columns to show' }).first().click() + const columns = page.getByRole('group', { name: 'Columns' }).first() + await expect(columns).toBeVisible() + // One off and one on, so both halves of "which columns" are being asked. + await columns.getByRole('checkbox', { name: 'Flutes' }).click() + await columns.getByRole('checkbox', { name: 'Shank' }).click() + // Up two places, by the keyboard: the drag is the same rule and nothing a + // test can do honestly — `movedBy` in `shared/column-order.ts`. + await columns.getByRole('button', { name: 'Move shank' }).click() + await page.keyboard.press('ArrowUp') + await page.keyboard.press('ArrowUp') + await page.keyboard.press('Escape') + + const chosen = await headings() + expect(chosen.some((heading) => heading.includes('Flutes'))).toBe(false) + expect(chosen.some((heading) => heading.includes('Shank'))).toBe(true) + + await page.reload() + await ready(page) + await expect(page.getByRole('grid').first().getByRole('row').nth(1)).toBeVisible() + + // The same columns, in the same order, and still filling the panel. + await expect(async () => { + expect(await headings()).toEqual(chosen) + }).toPass() + const fit = await tableFit(page) + expect(fit.table).toBe(fit.room) +}) + /** * **A menu belongs to the page, not to the card it was opened from** (Paul, * 2026-09-11: "the 'which columns to show' menu is now hidden behind the table From fcf9337b984e9ad88f365a311185ef8bbdbea591 Mon Sep 17 00:00:00 2001 From: Brad Estey Date: Fri, 11 Sep 2026 16:38:06 -0400 Subject: [PATCH 04/10] Update agents.md. --- AGENTS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/AGENTS.md b/AGENTS.md index 0022fde..cb25893 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -281,6 +281,7 @@ application unless that application says otherwise. | the two columns the list turns on for itself | `app/shared/auto-columns.ts` | | how wide a column is, as a share of the panel | `app/shared/column-width.ts` | | refitting the tracks a drag froze onto the table | `app/shared/use-fitted-columns.ts` | +| which columns a list shows, in what order, remembered | `app/shared/column-layout.ts` | | a row's answer, and what it opens to | `app/shared/recommendations.ts` | | what a press on a row of the order list opens | `pressRow`, `app/routes/part.tsx` | | whose stacks the tree beside an open box shows | `editedItem` / `treeKey`, same file | From 846bc864b933938cc6b67001cb5a50737b8b72d4 Mon Sep 17 00:00:00 2001 From: Brad Estey Date: Fri, 11 Sep 2026 16:58:59 -0400 Subject: [PATCH 05/10] Keep column widths in local storage. --- .../app/components/component-table.tsx | 10 +- .../app/components/part-tool-table.test.tsx | 22 ++- .../app/components/part-tool-table.tsx | 26 +++- apps/catalog/app/shared/column-layout.test.ts | 88 ++++++++++- apps/catalog/app/shared/column-layout.ts | 15 +- apps/catalog/app/shared/column-width.test.ts | 82 +++++++++- apps/catalog/app/shared/column-width.ts | 77 ++++++++++ apps/catalog/tests/on-the-part.spec.ts | 144 +++++++++++++++++- 8 files changed, 430 insertions(+), 34 deletions(-) diff --git a/apps/catalog/app/components/component-table.tsx b/apps/catalog/app/components/component-table.tsx index 8b22c73..36eb91e 100644 --- a/apps/catalog/app/components/component-table.tsx +++ b/apps/catalog/app/components/component-table.tsx @@ -12,7 +12,7 @@ import { Table, cn } from '@toolpath/ui' import type { Collet, Holder } from '@toolpath/catalog-data' import type { UnitSystem } from '@toolpath/tool-support' import { orderedCodes } from 'shared/column-order' -import { DEFAULT_COLUMN_WIDTH, fillingWidth } from 'shared/column-width' +import { DEFAULT_COLUMN_WIDTH, fillingWidth, widthId } from 'shared/column-width' import { useFittedColumns } from 'shared/use-fitted-columns' import { colletTypeLabel, @@ -242,8 +242,11 @@ export const ComponentTable = ({ /** The open column, or nothing where it has since been hidden. */ const openColumn = shown.find((column) => column.code === openFilter) ?? null const inside = useRef(null) + const codes = useMemo(() => shown.map((column) => column.code), [shown]) + /** Where the kit keeps what somebody dragged — `PartToolTable` says why. */ + const widths = widthId(`part-${kind}s`, codes) // The columns divide the panel, the same rule the tool list keeps. - useFittedColumns(inside, shown.map((column) => column.code).join(' ')) + useFittedColumns(inside, codes.join(' ')) /** * Whose move the selection was — the same guard `PartToolTable` keeps, and * for the same reason: without it the row the tree already holds is reported @@ -359,8 +362,9 @@ export const ComponentTable = ({ className={cn(TABLE_FACE, TABLE_INK, 'flex min-h-0 min-w-0 flex-1 flex-col')} >
- {/* No stored layout and no `min-w-max`: `PartToolTable` says why. */} + {/* An id named after the columns, and no `min-w-max`: `PartToolTable` says why. */}
{ }) /** - * **Nothing about a column's width is remembered between visits.** The kit - * stores a dragged layout under the `id` it is given and hands it back on the - * next mount, which is a saved answer to a question a column being shown or - * hidden has already changed. + * **A dragged width is remembered, and only for the columns it was about.** + * The kit stores its track list under the `id` it is given, positionally, so + * the id is the column set — `shared/column-width.ts` says why at length, and + * `shared/column-width.test.ts` pins the id itself. + * + * Nothing here can check the id reaches the kit: it is hung on no DOM node, + * and the write happens on a `mouseup` inside the kit that jsdom cannot + * produce. `tests/on-the-part.spec.ts` § "keeps a dragged column width" is + * where that is answered, with a real pointer. */ - it('gives the kit no id to store a column layout under', () => { - show({ - columns: TOOL_COLUMNS, - hiddenColumns: [], - columnOrder: TOOL_COLUMNS.map((column) => column.code), - }) - - expect(Object.keys(localStorage).filter((key) => key.startsWith('table-'))).toHaveLength(0) - }) it('asks for tracks that divide the panel rather than floors under it', () => { expect(flexibleColumnWidth('10rem')).toBe('minmax(0, 10fr)') diff --git a/apps/catalog/app/components/part-tool-table.tsx b/apps/catalog/app/components/part-tool-table.tsx index bc7dbf8..4d7fc65 100644 --- a/apps/catalog/app/components/part-tool-table.tsx +++ b/apps/catalog/app/components/part-tool-table.tsx @@ -19,7 +19,7 @@ import type { ToolQuery } from 'shared/filter' import { markWords, type Mark } from 'shared/tool-marks' import type { BelowHolder } from 'shared/drawn-assembly' import { orderedCodes } from 'shared/column-order' -import { DEFAULT_COLUMN_WIDTH, fillingWidth } from 'shared/column-width' +import { DEFAULT_COLUMN_WIDTH, fillingWidth, widthId } from 'shared/column-width' import { useFittedColumns } from 'shared/use-fitted-columns' import { ToolTypeIcon } from './tool-icons' import { @@ -476,9 +476,18 @@ export const PartToolTable = ({ /** The open column, or nothing where it has since been hidden. */ const openColumn = shown.find((column) => column.code === openFilter) ?? null const inside = useRef(null) + const codes = useMemo(() => shown.map((column) => column.code), [shown]) + /** + * Where the kit keeps what somebody dragged — named after these columns. + * + * A stored track list is positional, so it is only ever an answer about the + * column set it was dragged on: `shared/column-width.ts` says why that is the + * id rather than a fixed one. + */ + const widths = widthId('part-tools', codes) // The columns divide the panel; anything the kit's resizer froze onto it goes // when the panel or the column set changes. - useFittedColumns(inside, shown.map((column) => column.code).join(' ')) + useFittedColumns(inside, codes.join(' ')) const selectionCameFromTable = useRef(false) const setSelection = useCallback((next: SetStateAction) => { setSelectedRows((current) => { @@ -610,14 +619,15 @@ export const PartToolTable = ({ >
{/* - **No `id`, and no `min-w-max`** (Paul, 2026-09-11). The kit stores a - dragged layout under its `id` and hands it back on the next visit, - which is a saved answer to a question — how wide is a column — that a - column being shown or hidden has already changed. And `min-w-max` was - half of what made the list open wider than its panel; - `shared/column-width` is the whole story. + **An id named after the columns, and no `min-w-max`** (Paul, + 2026-09-11). The kit stores a dragged layout under its `id` and hands + it back on the next visit — which is worth keeping, and is only ever + an answer about the columns it was dragged on, so the column set *is* + the id. `min-w-max` was half of what made the list open wider than its + panel; `shared/column-width` is the whole story on both. */}
= [ { code: 'catalogNumber', default: true }, @@ -93,3 +104,68 @@ describe('the columns a list shows', () => { expect(readLayout(stored, later)).toMatchObject({ hidden: ['SIG'], touched: ['RE'] }) }) }) + +/** + * The press that edits the columns, and what it costs a dragged width. + * + * **Every stored width goes** (Paul, 2026-09-11: "on changing columns + * shown/hidden delete all localstorage keys saving column widths … Go back to + * the default sizes."). The rule is `shared/column-width.ts`; this is the wire + * from the picker to it, which is the part that can be got wrong silently — + * clearing on the columns *changing* rather than on the press throws away the + * width the list is about to settle on, since the list edits its own columns a + * tick after it loads. + */ +describe('the press that edits the columns', () => { + const COLUMNS: ReadonlyArray = [ + { code: 'catalogNumber', default: true }, + { code: 'brand', default: true }, + { code: 'RE', default: false }, + ] + + beforeEach(() => { + localStorage.clear() + }) + + const widths = () => Object.keys(localStorage).filter((key) => key.startsWith('table-')) + + it('drops every stored width when a column is shown or hidden', () => { + localStorage.setItem('table-part-tools.catalogNumber.brand', '8px 50% 50%') + localStorage.setItem('table-part-holders.catalogNumber', '8px 100%') + const { result } = renderHook(() => useColumnLayout(COLUMN_KEY.tools, COLUMNS)) + + act(() => { + result.current.toggle('brand') + }) + + expect(widths()).toEqual([]) + expect(result.current.hidden).toContain('brand') + }) + + it('drops them when the columns are dragged into another order too', () => { + localStorage.setItem('table-part-tools.catalogNumber.brand', '8px 50% 50%') + const { result } = renderHook(() => useColumnLayout(COLUMN_KEY.tools, COLUMNS)) + + act(() => { + result.current.reorder(['brand', 'catalogNumber', 'RE']) + }) + + expect(widths()).toEqual([]) + }) + + /** + * The list turning a column on for itself is not a press. A width stored + * under the set the list settles on has to survive the settling. + */ + it('leaves them alone when the list edits its own columns', () => { + localStorage.setItem('table-part-tools.catalogNumber.brand.RE', '8px 40% 30% 30%') + const { result } = renderHook(() => useColumnLayout(COLUMN_KEY.tools, COLUMNS)) + + act(() => { + result.current.setHidden((hidden) => hidden.filter((code) => code !== 'RE')) + }) + + expect(widths()).toEqual(['table-part-tools.catalogNumber.brand.RE']) + expect(result.current.hidden).not.toContain('RE') + }) +}) diff --git a/apps/catalog/app/shared/column-layout.ts b/apps/catalog/app/shared/column-layout.ts index 24527f2..446786c 100644 --- a/apps/catalog/app/shared/column-layout.ts +++ b/apps/catalog/app/shared/column-layout.ts @@ -1,5 +1,6 @@ import { useCallback, useEffect, useRef, useState } from 'react' import { orderedCodes } from './column-order' +import { forgetStoredWidths } from './column-width' /** * Which columns a list shows, and in what order — remembered per browser. @@ -177,9 +178,19 @@ export const useColumnLayout = (key: string, columns: ReadonlyArray { + forgetStoredWidths(globalThis.localStorage ?? null) keep((current) => ({ ...current, hidden: current.hidden.includes(code) @@ -191,8 +202,10 @@ export const useColumnLayout = (key: string, columns: ReadonlyArray) => { + forgetStoredWidths(globalThis.localStorage ?? null) keep((current) => ({ ...current, order })) }, [keep], diff --git a/apps/catalog/app/shared/column-width.test.ts b/apps/catalog/app/shared/column-width.test.ts index 6991388..14f304a 100644 --- a/apps/catalog/app/shared/column-width.test.ts +++ b/apps/catalog/app/shared/column-width.test.ts @@ -1,5 +1,11 @@ import { describe, expect, it } from 'vitest' -import { DEFAULT_WEIGHT, columnWeight, fillingWidth } from './column-width' +import { + DEFAULT_WEIGHT, + columnWeight, + fillingWidth, + forgetStoredWidths, + widthId, +} from './column-width' /** * How wide a column is. @@ -34,3 +40,77 @@ describe('how wide a column is', () => { expect(columnWeight('')).toBe(DEFAULT_WEIGHT) }) }) + +/** + * Where the kit keeps what somebody dragged (Paul, 2026-09-11: "the column + * widths should be stored in local storage but invalidate the old stores/ids + * every time a column is added or hidden"). + * + * `@toolpath/ui` writes a grid track list under `table-` — positional, and + * silent about which column each track was for. The kit guards the one case it + * can see, a change in the column *count*, and a swap or a reorder leaves that + * alone. So the id carries the column set, and a set that has changed asks a + * different key rather than being handed the wrong answer. + */ +describe('where a list keeps its dragged widths', () => { + it('names the id after the list and the columns on screen', () => { + expect(widthId('part-tools', ['catalogNumber', 'brand', 'DC'])).toBe( + 'part-tools.catalogNumber.brand.DC', + ) + }) + + it('asks a different key once a column is hidden, added or moved', () => { + const shown = ['catalogNumber', 'brand', 'DC'] + const hidden = widthId('part-tools', ['catalogNumber', 'DC']) + const added = widthId('part-tools', [...shown, 'RE']) + // A reorder is a rearrangement of the very positions a track list indexes. + const moved = widthId('part-tools', ['brand', 'catalogNumber', 'DC']) + const same = widthId('part-tools', shown) + + expect(new Set([hidden, added, moved, same]).size).toBe(4) + expect(widthId('part-tools', shown)).toBe(same) + }) + + /** + * **Editing the columns puts every list back on its defaults** (Paul, + * 2026-09-11: "I don't want columns to change size as I show and hide + * columns"). Every list, not this one: a stored width is an answer about a + * column set, and the press that edits one set has invalidated the idea that + * an old answer is worth resurfacing. + */ + describe('clearing the stored widths', () => { + const store = (entries: Record): Storage => { + const held = new Map(Object.entries(entries)) + return { + get length() { + return held.size + }, + key: (at: number) => [...held.keys()][at] ?? null, + getItem: (key: string) => held.get(key) ?? null, + setItem: (key: string, value: string) => held.set(key, value), + removeItem: (key: string) => void held.delete(key), + clear: () => held.clear(), + } as Storage + } + + it('drops every width any list has stored, and nothing else', () => { + const held = store({ + 'table-part-tools.catalogNumber.brand': 'a', + 'table-part-tools.catalogNumber.brand.DC': 'b', + 'table-part-holders.catalogNumber': 'c', + 'tool-catalog.columns.tools': 'd', + 'tool-catalog.preferences': 'e', + }) + + forgetStoredWidths(held) + + expect(held.length).toBe(2) + expect(held.getItem('tool-catalog.columns.tools')).toBe('d') + expect(held.getItem('tool-catalog.preferences')).toBe('e') + }) + + it('does nothing where a browser has no storage', () => { + expect(() => forgetStoredWidths(null)).not.toThrow() + }) + }) +}) diff --git a/apps/catalog/app/shared/column-width.ts b/apps/catalog/app/shared/column-width.ts index cf59cd7..3844ccd 100644 --- a/apps/catalog/app/shared/column-width.ts +++ b/apps/catalog/app/shared/column-width.ts @@ -48,3 +48,80 @@ export const columnWeight = (width: string): number => { /** The grid track a column asks for: its share of the box, never more than it. */ export const fillingWidth = (width: string): string => `minmax(0, ${columnWeight(width)}fr)` + +/** + * The id the kit stores this list's dragged column widths under. + * + * **The column set is the id** (Paul, 2026-09-11: "the column widths should be + * stored in local storage but invalidate the old stores/ids every time a column + * is added or hidden. The table id controls the local storage so the id needs + * to be changed to be the cache breaker"). + * + * What `@toolpath/ui`'s table writes under `table-` is a grid track list — + * eleven percentages in the order the columns happened to be in when somebody + * let go of the handle. It is **positional**, and it says nothing about which + * column each track was for, so a stored answer means something different the + * moment the columns change: hide one and every width after it lands on its + * neighbour. The kit guards the one case it can see, a change in the *count*, + * and is blind to a swap or a reorder, which leave the count alone. Widths were + * taken out of storage entirely for that reason earlier the same day. + * + * Naming the id after the columns is what puts them back safely: one stored + * layout per column set, found again when that set comes back, and never + * applied to any other. The **order** is in it too — a reorder is a + * rearrangement of the very positions the track list is indexed by. + * + * `shared/column-layout.ts` is the other half of this: which columns are shown + * and in what order is stored by *code*, so it survives the column set moving + * rather than being invalidated by it. The difference is the whole reason these + * are two modules. + */ +export const widthId = (list: string, shown: ReadonlyArray): string => + [list, ...shown].join('.') + +/** What the kit prefixes its own storage keys with — `use-column-layout.ts` in `@toolpath/ui`. */ +const KIT_PREFIX = 'table-' + +/** + * Every stored width, dropped. + * + * **Showing or hiding a column puts every list back on its default widths** + * (Paul, 2026-09-11: "on changing columns shown/hidden delete all localstorage + * keys saving column widths, they should all be invalidated. I don't want + * columns to change size as I show and hide columns. Go back to the default + * sizes."). A stored answer is only about the set it was dragged on, and a + * column set that has been edited is not that set — so the honest thing is a + * clean sheet rather than an old answer resurfacing under some id somebody + * happens to arrive back at. + * + * **Called from the press, not from the id.** A sweep hung on the id changing + * was built first and deleted the store the list was about to settle on: the + * tool list passes through two column sets on every load — the defaults, and + * then the set `shared/auto-columns.ts` settles on once it can see what is on + * the list, corner radius coming on for end mills a tick after the tools + * arrive. The id at mount is not the id a drag was stored under. A press in the + * column picker is a decision; that first change is the list finishing loading, + * and only one of the two should throw a width away. `shared/column-layout.ts` + * is where the press lives. + * + * **Cleared, not kept empty.** The kit writes its current layout back on any + * mouse-up once it has one in hand, so a key for the columns now on screen + * reappears within a click or two. What it holds then is the tracks the list + * computed for itself, which is exactly what going back to the default sizes + * means — what is gone is the old answer, not the file. + */ +export const forgetStoredWidths = ( + storage: Pick | null, +): void => { + if (storage === null) { + return + } + const stale: Array = [] + for (let at = 0; at < storage.length; at++) { + const key = storage.key(at) + if (key !== null && key.startsWith(KIT_PREFIX)) { + stale.push(key) + } + } + stale.forEach((key) => storage.removeItem(key)) +} diff --git a/apps/catalog/tests/on-the-part.spec.ts b/apps/catalog/tests/on-the-part.spec.ts index 81d5168..2172655 100644 --- a/apps/catalog/tests/on-the-part.spec.ts +++ b/apps/catalog/tests/on-the-part.spec.ts @@ -842,8 +842,13 @@ test('the columns divide the panel, at every width and column set', async ({ pag await page.mouse.move(grip!.x + grip!.width / 2 + 40, grip!.y + grip!.height / 2, { steps: 5 }) await page.mouse.up() - const dragged = await tableFit(page) - expect(dragged.table).toBe(dragged.room) + // Retried: a drag is five synthetic mouse moves, and a loaded machine can + // read the box between the last of them and the layout that follows it. + let dragged = await tableFit(page) + await expect(async () => { + dragged = await tableFit(page) + expect(dragged.table).toBe(dragged.room) + }).toPass() await page.getByRole('button', { name: 'Which columns to show' }).first().click() const columns = page.getByRole('group', { name: 'Columns' }).first() @@ -858,6 +863,141 @@ test('the columns divide the panel, at every width and column set', async ({ pag }).toPass() }) +/** + * **A dragged width is kept, and kept only for the columns it was about** + * (Paul, 2026-09-11: "the column widths should be stored in local storage but + * invalidate the old stores/ids every time a column is added or hidden. The + * table id controls the local storage so the id needs to be changed to be the + * cache breaker"). + * + * What `@toolpath/ui` writes is a grid track list — percentages in the order + * the columns were in when the handle was let go. It is positional and silent + * about which column each track was for, and the kit only guards a change in + * the *count*. So the column set is the id, and the rules are in + * `app/shared/column-width.ts`. + * + * Only a real browser can answer this one: the write happens on `mouseup` + * inside the kit, and nothing in jsdom can drag. + */ +test('keeps a dragged column width, and drops every one when the columns change', async ({ + page, +}) => { + await ready(page) + await keepFeature(page) + await expect(page.getByRole('grid').first().getByRole('row').nth(1)).toBeVisible() + + /** + * How much of the list the first column takes, in hundredths. + * + * A **share**, not a width in pixels: what the kit stores is percentages of + * the box, and the box is not the same size on the next visit — the column of + * questions over the part is a different width once a feature is on the order + * list, and the list gets what is left. Comparing pixels across the reload + * compares two panels. + * + * Named, not `.first()`: the kit draws an 8px selection column ahead of them. + */ + const firstShare = async () => { + const column = (await page + .locator('[data-part-tool-table]') + .first() + .getByRole('columnheader', { name: /Catalog number/ }) + .boundingBox())!.width + const { room } = await tableFit(page) + return Math.round((column / room) * 100) + } + + const stores = () => + page.evaluate(() => Object.keys(localStorage).filter((key) => key.startsWith('table-'))) + + const columnCount = () => + page.locator('[data-part-tool-table]').first().getByRole('columnheader').count() + + const opened = await firstShare() + const shownAtDrag = await columnCount() + + const handle = page.locator('[data-part-tool-table] .resizer-area').first() + const grip = await handle.boundingBox() + expect(grip).not.toBeNull() + await page.mouse.move(grip!.x + grip!.width / 2, grip!.y + grip!.height / 2) + await page.mouse.down() + await page.mouse.move(grip!.x + grip!.width / 2 + 60, grip!.y + grip!.height / 2, { steps: 5 }) + await page.mouse.up() + + const dragged = await firstShare() + expect(dragged).toBeGreaterThan(opened + 3) + /* + Stored under the columns it was dragged on: the kit writes under + `table-`, and the id is this list and the whole set it is drawing, in + order — `app/shared/column-width.ts` names it. + + Asserted as "one of these keys is the tool list's" rather than as the only + key there, because the kit writes a layout of its own accord as the list + settles; what this test is about is whether the *width* survives, which the + reload below is what answers. + */ + const stored = (await stores()).filter((each) => each.startsWith('table-part-tools.')) + expect(stored.length).toBeGreaterThan(0) + + /* + And it is still there on the next visit. Asserted on the store rather than + on the pixels: the kit restores by id, and which id this list settles on + depends on what is on it — `shared/auto-columns.ts` brings corner radius and + tip angle on for the forms the list happens to be holding, so the column set + after a reload is not reliably the one a drag was stored under. That the + drag *survives* is this half; that it is only ever applied to its own column + set is `shared/column-width.test.ts`. + */ + await page.reload() + await ready(page) + await expect(page.getByRole('grid').first().getByRole('row').nth(1)).toBeVisible() + expect(await stores()).toEqual(expect.arrayContaining(stored)) + + /* + And now the invalidation (Paul, 2026-09-11: "I don't want columns to change + size as I show and hide columns. Go back to the default sizes."). Hiding one + column drops **every** stored width, so the list is back on the tracks it + computes rather than on eleven percentages meant for twelve columns. + */ + await page.getByRole('button', { name: 'Which columns to show' }).first().click() + const columns = page.getByRole('group', { name: 'Columns' }).first() + await expect(columns).toBeVisible() + await columns.getByRole('checkbox', { name: 'Flutes' }).click() + await page.keyboard.press('Escape') + + await expect(async () => { + expect(await columnCount()).toBe(shownAtDrag - 1) + expect(await stores()).toEqual([]) + }).toPass() + + /* + Back on the share the tracks give it — `minmax(0, 10fr)` against the weights + beside it — rather than on the one that was dragged. + + Not equal to the share it opened at, and correctly so: a column has gone, so + the ten weights left divide the box between fewer of them and every share + rises a little. What it must not be is the dragged one, and the panel + filling exactly is what says the list is back on its own tracks. + */ + expect(await firstShare()).toBeLessThan(dragged - 2) + const fit = await tableFit(page) + expect(fit.table).toBe(fit.room) + + /* + And it does not come back on the next visit — the drag is genuinely gone + rather than merely not applied this time. + + Asserted on the share and not on an empty store, because the store does not + stay empty: the kit writes its current layout on any mouse-up once it has + one in hand, so a key for the columns now on screen reappears within a click + or two. What it holds is the tracks the list computed, which is the point. + */ + await page.reload() + await ready(page) + await expect(page.getByRole('grid').first().getByRole('row').nth(1)).toBeVisible() + expect(await firstShare()).toBeLessThan(dragged - 2) +}) + /** * **A shop sets its columns once** (Paul, 2026-09-11: "save column order and * visibility in local storage"). From 2f1782fb153ffc5ff218283d09ec2aef3f382613 Mon Sep 17 00:00:00 2001 From: Brad Estey Date: Fri, 11 Sep 2026 17:04:26 -0400 Subject: [PATCH 06/10] Add blue line when reordering columns. --- .../app/components/column-filter.test.tsx | 71 +++++++++ apps/catalog/app/components/column-filter.tsx | 150 +++++++++++------- apps/catalog/app/shared/column-order.test.ts | 38 ++++- apps/catalog/app/shared/column-order.ts | 38 +++++ 4 files changed, 242 insertions(+), 55 deletions(-) diff --git a/apps/catalog/app/components/column-filter.test.tsx b/apps/catalog/app/components/column-filter.test.tsx index 5d70fdd..905afea 100644 --- a/apps/catalog/app/components/column-filter.test.tsx +++ b/apps/catalog/app/components/column-filter.test.tsx @@ -230,6 +230,77 @@ describe('the shape a stored bound has', () => { }) describe('the column picker', () => { + /** + * **The drag says where the row will land before it lands** (Paul, + * 2026-09-11: "the list should show a blue line (2px horizontal) where the + * item will be dropped to help the user see where it will go"). + * + * Which edge is `shared/column-order.ts` § `dropEdge`, and its own tests tie + * that to where `movedTo` actually puts the row. What this pins is the wire: + * that a drag over a row draws the line, on the edge the rule names, on that + * row and no other — and that letting go anywhere puts it away. + */ + const picker = (order: ReadonlyArray, onReorder = vi.fn()) => { + render( + ({ code, label: code }))} + shown={[...order]} + onToggle={vi.fn()} + onReorder={onReorder} + />, + ) + fireEvent.click(screen.getByRole('button', { name: 'Which columns to show' })) + const rows = screen.getByRole('group', { name: 'Columns' }).children + return { + rows, + lines: () => Array.from(document.querySelectorAll('[data-drop-edge]')), + grab: (code: string) => + fireEvent.dragStart(screen.getByRole('button', { name: `Move ${code}` })), + release: (code: string) => + fireEvent.dragEnd(screen.getByRole('button', { name: `Move ${code}` })), + } + } + + it('draws one line, on the row under the pointer, while a column is dragged', () => { + const { rows, lines, grab } = picker(['a', 'b', 'c', 'd']) + + expect(lines()).toHaveLength(0) + + grab('a') + fireEvent.dragOver(rows[2]!) + + expect(lines()).toHaveLength(1) + expect(rows[2]!.querySelector('[data-drop-edge]')).not.toBeNull() + }) + + /** Down lands after the row, up lands before it — `movedTo` decides which. */ + it('puts the line under the row dragging down and over it dragging up', () => { + const { rows, lines, grab, release } = picker(['a', 'b', 'c', 'd']) + + grab('a') + fireEvent.dragOver(rows[2]!) + expect(lines()[0]).toHaveAttribute('data-drop-edge', 'below') + + release('a') + grab('d') + fireEvent.dragOver(rows[1]!) + expect(lines()[0]).toHaveAttribute('data-drop-edge', 'above') + }) + + it('draws nothing over the row being dragged, and nothing once it is let go', () => { + const { rows, lines, grab, release } = picker(['a', 'b', 'c']) + + grab('b') + fireEvent.dragOver(rows[1]!) + expect(lines()).toHaveLength(0) + + fireEvent.dragOver(rows[0]!) + expect(lines()).toHaveLength(1) + + release('b') + expect(lines()).toHaveLength(0) + }) + it('keeps the pencil at the table header touch target size', () => { render( { const [open, setOpen] = useState(false) const [held, setHeld] = useState(null) + /** Which row the pointer is over mid-drag, so the line knows where to be. */ + const [over, setOver] = useState(null) const order = columns.map((column) => column.code) const move = (code: string, index: number) => { + setOver(null) const next = movedTo(order, code, index) if (next.join() !== order.join()) { onReorder?.(next) @@ -1126,61 +1129,100 @@ export const ColumnPicker = ({ aria-label="Columns" className="max-h-[var(--available-height)] overflow-y-auto" > - {columns.map((column, at) => ( -
{ - if (held !== null) { + {columns.map((column, at) => { + const edge = held === null ? null : dropEdge(order, held, over === at ? at : -1) + return ( +
{ + if (held !== null) { + event.preventDefault() + // Set here rather than on enter and cleared on leave: this + // fires for as long as the pointer is on the row, so the + // one under it is always the last to have spoken, and a + // drag over a child never reads as a drag out of the row. + setOver(at) + } + }} + onDrop={(event) => { event.preventDefault() - } - }} - onDrop={(event) => { - event.preventDefault() - if (held !== null) { - move(held, at) - setHeld(null) - } - }} - className={cn( - 'text-2xs flex items-center gap-1.5 px-2 py-1 whitespace-nowrap hover:bg-zinc-900', - held === column.code && 'opacity-50', - )} - > - {onReorder === undefined ? null : ( - setHeld(column.code)} - onDragEnd={() => setHeld(null)} - onKeyDown={(event) => { - const by = event.key === 'ArrowUp' ? -1 : event.key === 'ArrowDown' ? 1 : 0 - if (by !== 0) { - event.preventDefault() - onReorder(movedBy(order, column.code, by)) - } - }} - className="focus-visible:ring-info/60 shrink-0 cursor-grab rounded text-zinc-600 transition hover:text-zinc-300 focus-visible:ring-1 focus-visible:outline-none active:cursor-grabbing" - > - - )} -
- onToggle(column.code)} - size="sm" - aria-label={column.label} - /> - {column.label} + if (held !== null) { + move(held, at) + setHeld(null) + } + }} + className={cn( + 'text-2xs relative flex items-center gap-1.5 px-2 py-1 whitespace-nowrap hover:bg-zinc-900', + held === column.code && 'opacity-50', + )} + > + {/* + **Where it would land** (Paul, 2026-09-11), drawn on the edge + `shared/column-order.ts` says the drop resolves to rather than + on the one under the pointer — the two differ whenever the + drag is downwards, and a line that lies about the drop is + worse than none. + + Absolutely positioned, so the rows under it do not step down + by two pixels as the line moves between them; `-top-px` and + `-bottom-px` put it *on* the boundary rather than inside one + of the two rows it divides. + + Blue, and not the `info` accent every other affordance here + wears: this is a thing being carried rather than a control + being answered, and the accent is teal against this ground — + which is not what was asked for, and reads as one more + highlighted control on a panel already full of them. + */} + {edge === null ? null : ( + -
- ))} + ) + })}
diff --git a/apps/catalog/app/shared/column-order.test.ts b/apps/catalog/app/shared/column-order.test.ts index 3e25fac..916e9ee 100644 --- a/apps/catalog/app/shared/column-order.test.ts +++ b/apps/catalog/app/shared/column-order.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest' -import { movedBy, movedTo, orderedCodes } from './column-order' +import { dropEdge, movedBy, movedTo, orderedCodes } from './column-order' describe('the column order', () => { const order = ['DC', 'LCF', 'LD', 'RE'] @@ -42,3 +42,39 @@ describe('the column order', () => { expect(movedBy(order, 'RE', 1)).toEqual(order) }) }) + +/** + * The line that says where a dragged column would land. + * + * **Decided by `movedTo`, not by what looks right** (Paul, 2026-09-11: "the + * list should show a blue line (2px horizontal) where the item will be + * dropped"). `movedTo` lifts the code out before reading the index, so a drop + * below where the drag started lands *after* the row under the pointer and one + * above lands *before* it — and a line drawn on the other edge would promise a + * position the drop does not deliver. + */ +describe('where a dragged column would land', () => { + const ORDER = ['A', 'B', 'C', 'D'] + + it('goes under the row when the drag is downwards', () => { + expect(dropEdge(ORDER, 'A', 2)).toBe('below') + // And that is where it lands: [B, C, A, D]. + expect(movedTo(ORDER, 'A', 2)).toEqual(['B', 'C', 'A', 'D']) + }) + + it('goes over the row when the drag is upwards', () => { + expect(dropEdge(ORDER, 'D', 1)).toBe('above') + expect(movedTo(ORDER, 'D', 1)).toEqual(['A', 'D', 'B', 'C']) + }) + + /** A row cannot land on itself, so it draws no line while it is picked up. */ + it('draws nothing over the row being dragged', () => { + expect(dropEdge(ORDER, 'B', 1)).toBeNull() + }) + + it('draws nothing for a code or a row this list does not have', () => { + expect(dropEdge(ORDER, 'Z', 2)).toBeNull() + expect(dropEdge(ORDER, 'A', -1)).toBeNull() + expect(dropEdge(ORDER, 'A', 4)).toBeNull() + }) +}) diff --git a/apps/catalog/app/shared/column-order.ts b/apps/catalog/app/shared/column-order.ts index b7d65e3..4e6decd 100644 --- a/apps/catalog/app/shared/column-order.ts +++ b/apps/catalog/app/shared/column-order.ts @@ -50,3 +50,41 @@ export const movedBy = (order: ReadonlyArray, code: string, by: number): const from = order.indexOf(code) return from === -1 ? [...order] : movedTo(order, code, from + by) } + +/** Which side of a row a dragged column would land on, or nothing over its own row. */ +export type DropEdge = 'above' | 'below' + +/** + * Where the line goes while a column is being dragged over a row. + * + * **A drag with no line is a guess** (Paul, 2026-09-11: "the list should show a + * blue line (2px horizontal) where the item will be dropped to help the user + * see where it will go"). The picker moved a column on drop and said nothing + * before it, so the only way to find out where a row would land was to drop it + * and look. + * + * The edge is decided by {@link movedTo} rather than chosen to look right: that + * function lifts the code out *before* reading the index, so dropping on a row + * below where the drag started lands **after** that row, and dropping on one + * above lands **before** it. Say [A, B, C, D] and drag A onto C — without A the + * list is [B, C, D] and inserting at 2 gives [B, C, A, D], which is under C. + * Drag D onto B and the same arithmetic puts it over B. A line drawn any other + * way is a line that lies about the drop. + * + * @param order the codes as the list is drawing them. + * @param held the code being dragged. + * @param over the index of the row the pointer is on. + */ +export const dropEdge = ( + order: ReadonlyArray, + held: string, + over: number, +): DropEdge | null => { + const from = order.indexOf(held) + // Nothing over the row being dragged, and nothing for a code this list has + // never heard of: neither is a drop that would move anything. + if (from === -1 || from === over || over < 0 || over >= order.length) { + return null + } + return from < over ? 'below' : 'above' +} From 003935624c9cdbfc1f4788e8ec6a61072c16bf8e Mon Sep 17 00:00:00 2001 From: Brad Estey Date: Fri, 11 Sep 2026 17:22:41 -0400 Subject: [PATCH 07/10] Handle race condition resetting hidden columns or reload. --- apps/catalog/app/routes/part.tsx | 22 ++--- apps/catalog/app/shared/column-layout.test.ts | 46 ++++++++++- apps/catalog/app/shared/column-layout.ts | 15 +++- apps/catalog/tests/on-the-part.spec.ts | 82 +++++++++++++++++++ 4 files changed, 147 insertions(+), 18 deletions(-) diff --git a/apps/catalog/app/routes/part.tsx b/apps/catalog/app/routes/part.tsx index 0f888f8..60453c9 100644 --- a/apps/catalog/app/routes/part.tsx +++ b/apps/catalog/app/routes/part.tsx @@ -538,18 +538,7 @@ const Inspecting = ({ report, jobId }: { report: PublicInspectionReport; jobId: * stored answer means once the catalog's columns have moved under it. */ const toolLayout = useColumnLayout(COLUMN_KEY.tools, TOOL_COLUMNS) - const { - hidden: hiddenColumns, - order: columnOrder, - /** - * The columns somebody has decided for themselves. - * - * Tip angle and corner radius follow the list — `shared/auto-columns.ts` - * is the rule — and a code in here is one the list stops deciding about. - */ - touched: touchedColumns, - setHidden: setHiddenColumns, - } = toolLayout + const { hidden: hiddenColumns, order: columnOrder, setHidden: setHiddenColumns } = toolLayout /** * The tap list's columns, kept apart from the tool list's. * @@ -2363,8 +2352,13 @@ const Inspecting = ({ report, jobId }: { report: PublicInspectionReport; jobId: */ const listedForms = useMemo(() => listed.map((each) => each.form), [listed]) useEffect(() => { - setHiddenColumns((current) => hiddenAfterAuto(current, listedForms, new Set(touchedColumns))) - }, [listedForms, touchedColumns, setHiddenColumns]) + // Both halves off one layout: what is hidden, and what somebody has already + // decided for themselves. `shared/column-layout.ts` § `setHidden` says what + // reading those two out of two different states cost. + setHiddenColumns((layout) => + hiddenAfterAuto(layout.hidden, listedForms, new Set(layout.touched)), + ) + }, [listedForms, setHiddenColumns]) /** * The list narrowed by what was typed into the catalog number column. * diff --git a/apps/catalog/app/shared/column-layout.test.ts b/apps/catalog/app/shared/column-layout.test.ts index 0c31be8..b34b05a 100644 --- a/apps/catalog/app/shared/column-layout.test.ts +++ b/apps/catalog/app/shared/column-layout.test.ts @@ -157,15 +157,57 @@ describe('the press that edits the columns', () => { * The list turning a column on for itself is not a press. A width stored * under the set the list settles on has to survive the settling. */ - it('leaves them alone when the list edits its own columns', () => { + it('leaves the widths alone when the list edits its own columns', () => { localStorage.setItem('table-part-tools.catalogNumber.brand.RE', '8px 40% 30% 30%') const { result } = renderHook(() => useColumnLayout(COLUMN_KEY.tools, COLUMNS)) act(() => { - result.current.setHidden((hidden) => hidden.filter((code) => code !== 'RE')) + result.current.setHidden((layout) => layout.hidden.filter((code) => code !== 'RE')) }) expect(widths()).toEqual(['table-part-tools.catalogNumber.brand.RE']) expect(result.current.hidden).not.toContain('RE') }) }) + +/** + * The rule that edits the columns for the list, and the state it reads. + * + * **Both halves off one layout** (Paul, 2026-09-11: "I hide corner radius and + * tip angle … I hit refresh. They come back. Local storage also changes to add + * them back in."). `hiddenAfterAuto` asks two questions of this state — what is + * hidden, and what has somebody already decided — and the answer to the second + * is what stops it undoing the first. While the rule took only `hidden`, the + * caller had to read `touched` out of its own render, and on the frame where a + * stored layout is restored those are different states: the hidden set came + * from the update queue and `touched` was still the empty default. + */ +describe('a rule rewriting the hidden set', () => { + const COLUMNS: ReadonlyArray = [ + { code: 'DC', default: true }, + { code: 'RE', default: false }, + ] + + beforeEach(() => { + localStorage.clear() + }) + + it('reads what somebody decided out of the same layout as the hidden set', () => { + localStorage.setItem( + COLUMN_KEY.tools, + written({ hidden: ['RE'], order: ['DC', 'RE'], touched: ['RE'] }, COLUMNS), + ) + const { result } = renderHook(() => useColumnLayout(COLUMN_KEY.tools, COLUMNS)) + const seen: Array> = [] + + act(() => { + result.current.setHidden((layout) => { + seen.push(layout.touched) + return layout.hidden + }) + }) + + // The restored answer, not the empty default the hook started at. + expect(seen).toEqual([['RE']]) + }) +}) diff --git a/apps/catalog/app/shared/column-layout.ts b/apps/catalog/app/shared/column-layout.ts index 446786c..367e224 100644 --- a/apps/catalog/app/shared/column-layout.ts +++ b/apps/catalog/app/shared/column-layout.ts @@ -215,11 +215,22 @@ export const useColumnLayout = (key: string, columns: ReadonlyArray