diff --git a/src/components/AppBlocks/PageBlockHost.tsx b/src/components/AppBlocks/PageBlockHost.tsx index 8391b7d97a..c2eebbe092 100644 --- a/src/components/AppBlocks/PageBlockHost.tsx +++ b/src/components/AppBlocks/PageBlockHost.tsx @@ -347,9 +347,17 @@ export const FILL_MIN_HEIGHT_PX = 300; * 1288 the widest ORDINARY civitai content measure — Mantine `xl` (1320 * border-box) is the widest container size in use across `src/pages`, * and `APPS_TWO_COLUMN_DETAIL_MEASURE` (the store-preview page an app is - * usually launched FROM) is exactly it. An app capped below this would + * usually launched FROM) starts there. An app capped below this would * render narrower than the page that linked to it, which reads as a * downgrade rather than a frame. + * ⚠️ THAT CONSTANT IS NO LONGER A SINGLE NUMBER, and the sentence above used + * to say "is exactly it". It is a BAND now — `{min: 1288, max: 1600}` — so on + * a wide screen the store-preview page reaches 1600, which is EXACTLY this + * cap rather than 312px below it. The conclusion survives (an app is never + * narrower than the page that launched it) but the MARGIN this paragraph + * implied is gone: at the top of that band the two are equal. If the + * store-preview band is ever raised again, this cap stops being a ceiling + * over it and the reasoning here has to be re-made rather than re-read. * 2560 `APPS_PAGE_CONTAINER_WIDTH` — the deliberate outlier, and it is an * outlier for a reason that does NOT transfer: it exists for card GRIDS * and wide TABLES (`appsPageWidths.ts` records the measurements), which @@ -359,8 +367,13 @@ export const FILL_MIN_HEIGHT_PX = 300; * to 2560. The gap between the cap and the outlier therefore WIDENED, which * does not by itself justify widening the cap — see below. * - * 1600 is above every ordinary content measure on the site and below the grid - * container, i.e. no app is ever narrower than a civitai page. It also clears the + * 1600 is at-or-above every ordinary content measure on the site and below the grid + * container, i.e. no app is ever narrower than a civitai page. ⚠️ "AT-OR-ABOVE" IS THE + * CORRECTION: this read "above every ordinary content measure" while + * `APPS_TWO_COLUMN_DETAIL_MEASURE` was the fixed 1288. It is a band now, topping out at + * exactly 1600, so on a wide screen the store-preview page and this cap are the SAME + * width. The claim that matters — no app renders narrower than the page that launched it + * — still holds at equality; the headroom it used to have does not. It also clears the * widest app-imposed well (1100) by ~45%, so the cap can never letterbox an app * that has already thought about its own width, while leaving a two-pane shell * like Notepad or Sensei a ~1350px content pane — the case the cap exists for. @@ -371,9 +384,11 @@ export const FILL_MIN_HEIGHT_PX = 300; * used to appear here and it is a moving target: the apps container has taken three * values over time — 1600 → 1920 → 2560 — without any of them being a statement about * how wide a THIRD-PARTY app should be. The cap's real justification is the two bounds above - * it does control — above every ordinary content measure, and comfortably clear of - * the widest app-imposed well — neither of which moves when the apps container - * does. Widening 1600 is a separate decision with its own evidence. + * it does control — at-or-above every ordinary content measure, and comfortably clear of + * the widest app-imposed well — neither of which moves when the apps CONTAINER does. + * (The store-preview band's ceiling does sit exactly on the first of those two, so it is + * a bound this cap now touches rather than clears; raising that band again would invert + * it, and this reasoning would have to be re-made.) Widening 1600 is a separate decision with its own evidence. * * 🔴 THE APP THIS IS PROBABLY WRONG FOR, and why the opt-out ships WITH the cap * rather than after it: Playable Collections. Re-read at its DEPLOYED ref diff --git a/src/components/AppBlocks/RevenuePanel.tsx b/src/components/AppBlocks/RevenuePanel.tsx index 9428af278a..1bcdfe74a2 100644 --- a/src/components/AppBlocks/RevenuePanel.tsx +++ b/src/components/AppBlocks/RevenuePanel.tsx @@ -14,6 +14,7 @@ import { } from '@mantine/core'; import { IconBolt, IconInfoCircle } from '@tabler/icons-react'; import Link from 'next/link'; +import { AppsTableColgroup, APPS_REVENUE_COLUMNS } from '~/components/Apps/appsWideLayout'; import { trpc } from '~/utils/trpc'; /** @@ -239,6 +240,18 @@ export function RevenuePanel({ appBlockId }: { appBlockId?: string }) { ) : ( + {/* + 🔴 FIRST CHILD, BEFORE the row groups — HTML requires it there; the + ordering is pinned by `~/components/Apps/__tests__/appsWideLayout.test.ts`. + The ledger is chosen by `scoped`, the SAME flag that + decides whether the App column renders, so the column count and the width + list cannot drift apart. Unscoped (`/apps/revenue`, full container) the + App link is primary; scoped there is no App column and Scope takes the + slack. + */} + Date diff --git a/src/components/AppBlocks/__tests__/pageBlockHostMaxWidth.test.ts b/src/components/AppBlocks/__tests__/pageBlockHostMaxWidth.test.ts index d85c5907ce..cc371031fb 100644 --- a/src/components/AppBlocks/__tests__/pageBlockHostMaxWidth.test.ts +++ b/src/components/AppBlocks/__tests__/pageBlockHostMaxWidth.test.ts @@ -261,8 +261,15 @@ describe('the full-page App Block host caps its width, and the cap is overridabl * * Lower 1280: below this the cap would be narrower than the widest ORDINARY * civitai content measure (Mantine `xl`, 1320 border-box / 1288 content — also - * `APPS_TWO_COLUMN_DETAIL_MEASURE`, the store-preview page an app is launched - * FROM), so an app would render narrower than the page that linked to it. + * `APPS_TWO_COLUMN_DETAIL_MEASURE`'s FLOOR, the store-preview page an app is + * launched FROM), so an app would render narrower than the page that linked to it. + * + * ⚠️ THAT MEASURE IS A BAND NOW, `{min: 1288, max: 1600}`, and this comment used to + * cite it as a single number. The lower bound is unaffected — it is keyed on the + * floor — but the sibling claim in `PageBlockHost.tsx` that 1600 sits "above every + * ordinary content measure on the site" is now PARITY rather than headroom: at the + * top of that band the store-preview page and the app cap are the same width. The + * conclusion holds, the margin does not, and both files say so. * * Upper 1920: the width `APPS_PAGE_CONTAINER_WIDTH` held when this band was * chosen. ⚠️ THAT CONSTANT IS NOW 2560 — the ultrawide pass moved it — and the diff --git a/src/components/Apps/ActivePreviewsPanel.tsx b/src/components/Apps/ActivePreviewsPanel.tsx index ff2bd189ea..e3d76c4327 100644 --- a/src/components/Apps/ActivePreviewsPanel.tsx +++ b/src/components/Apps/ActivePreviewsPanel.tsx @@ -4,6 +4,7 @@ import { ModQueryError, isModAuthzError } from '~/components/Apps/ModQuerySurfac import { useFeatureFlags } from '~/providers/FeatureFlagsProvider'; import { showErrorNotification, showSuccessNotification } from '~/utils/notifications'; import { trpc } from '~/utils/trpc'; +import { AppsTableColgroup, APPS_ACTIVE_PREVIEWS_COLUMNS } from '~/components/Apps/appsWideLayout'; /** * MOD REVIEW SANDBOX — global "Active previews (N / cap)" panel. Extracted from @@ -110,9 +111,14 @@ export function ActivePreviewsPanel() { )}
+ {/* 🔴 FIRST CHILD, BEFORE the row groups — see `appsWideLayout`. This panel is + the reason that module's guard ENUMERATES tables: `/apps/review` gave up its + 1368 body cap, and measured at 1440 → 2560 without a ledger the gap between a + row's slug and its "Tear down" button grew 817.36 → 1381.23. */} + - App + App Version State Age @@ -132,13 +138,21 @@ export function ActivePreviewsPanel() { {p.state.replace('preview-', '')} - + {/* Same reason as the state badge above: a relative-age label + ("3 minutes ago") is one phrase and wrapping it is never right. */} + {formatAge(p.updatedAt)} diff --git a/src/components/Apps/AppActivityPanel.tsx b/src/components/Apps/AppActivityPanel.tsx index 791d8cfb81..780653f9ee 100644 --- a/src/components/Apps/AppActivityPanel.tsx +++ b/src/components/Apps/AppActivityPanel.tsx @@ -358,6 +358,15 @@ export function AppActivityPanel({ return (
+ {/* 🔴 NO COLUMN LEDGER, DELIBERATELY — this table is EXEMPT in + `~/components/Apps/appsWideLayout`, and the exemption is the measured outcome + rather than an omission. Its natural layout already renders every cell on ONE + line at 768/1200/1440/2560 (row height 36.19 at all four), because its + max-content sum ≈ the container's content width at 768: there is no surplus to + place at the narrow end. Every ledger that meaningfully redistributes at 2560 + wraps a cell at 768, and the one that does NOT (`[16,12,27,25,null]`) lands + within ~15px per column of natural at 2560 — i.e. it buys nothing. See the + EXEMPT entry for the full table of measurements. */} When diff --git a/src/components/Apps/AppListingsModerationTable.tsx b/src/components/Apps/AppListingsModerationTable.tsx index 6081778d6c..856bb8105f 100644 --- a/src/components/Apps/AppListingsModerationTable.tsx +++ b/src/components/Apps/AppListingsModerationTable.tsx @@ -20,6 +20,7 @@ import { MessageAppOwnerModal } from '~/components/Apps/MessageAppOwnerModal'; import { ModQueryError, isModAuthzError } from '~/components/Apps/ModQuerySurface'; import { ReasonGatedActionModal } from '~/components/Apps/ReasonGatedActionModal'; import { listingStatusChip } from '~/components/Apps/appListingModerationView'; +import { AppsTableColgroup, APPS_MOD_LISTINGS_COLUMNS } from '~/components/Apps/appsWideLayout'; import { LISTING_KIND_LABELS } from '~/components/Apps/listingKindLabels'; import { actionOpensOwnerMessage, @@ -313,6 +314,10 @@ export function AppListingsModerationTable({ const renderTable = (groups: SubmissionGroup[]) => (
+ {/* 🔴 FIRST CHILD, BEFORE the row groups — see `appsWideLayout`. `/apps/review` + hosts this table and no longer caps its body, so the slack has to have + somewhere deliberate to go: the App cell (slug + kind + status chips). */} + diff --git a/src/components/Apps/AppsPageLayout.chromeAlignment.browser.test.tsx b/src/components/Apps/AppsPageLayout.chromeAlignment.browser.test.tsx index 45a4784006..465c787537 100644 --- a/src/components/Apps/AppsPageLayout.chromeAlignment.browser.test.tsx +++ b/src/components/Apps/AppsPageLayout.chromeAlignment.browser.test.tsx @@ -60,6 +60,7 @@ import { cleanup } from 'vitest-browser-react'; // `test/` lives outside `src`, so the `~` alias doesn't reach it — relative import. import { renderWithProviders } from '../../../test/component-setup'; import type * as TrpcMod from '~/utils/trpc'; +import type { AppsMeasure } from './appsPageWidths'; // 🔴 The viewer MUST be one the sub-nav renders for. `AppsSubNav` hides itself // entirely below two qualifying tabs, and the summary query is stubbed empty here, so @@ -81,7 +82,23 @@ vi.mock('~/utils/trpc', async (importOriginal) => ({ })); const { AppsPageLayout } = await import('./AppsPageLayout'); -const { APPS_PAGE_MEASURES, APPS_FULL_MEASURE_PAGES } = await import('./appsPageWidths'); +const { APPS_PAGE_MEASURES, APPS_FULL_MEASURE_PAGES, isAppsMeasureBand } = await import( + './appsPageWidths' +); + +/** + * What a measure RESOLVES TO against a given container content width. + * + * A number is itself; a BAND is its `clamp()`, evaluated in JS. Both are then capped by + * the content width, because a `max-width` cannot make a box wider than its container. + */ +function resolveMeasure(measure: AppsMeasure | undefined, contentWidth: number): number { + if (measure === undefined) return contentWidth; + const wanted = isAppsMeasureBand(measure) + ? Math.min(measure.max, Math.max(measure.min, (measure.grow / 100) * contentWidth)) + : measure; + return Math.min(wanted, contentWidth); +} /** * Every route that renders the shared chrome, with the measure it passes. @@ -91,7 +108,7 @@ const { APPS_PAGE_MEASURES, APPS_FULL_MEASURE_PAGES } = await import('./appsPage * measured; deriving it without pinning would let the set SHRINK to one element (or * empty) and the "they all agree" assertion pass vacuously. Both halves are needed. */ -const ROUTES: { route: string; measure?: number }[] = [ +const ROUTES: { route: string; measure?: AppsMeasure }[] = [ ...APPS_FULL_MEASURE_PAGES.map((route) => ({ route, measure: undefined })), ...Object.entries(APPS_PAGE_MEASURES).map(([route, measure]) => ({ route, measure })), ].sort((a, b) => (a.route < b.route ? -1 : a.route > b.route ? 1 : 0)); @@ -148,7 +165,7 @@ function measure() { }; } -async function renderAndMeasure(measurePx: number | undefined) { +async function renderAndMeasure(measurePx: AppsMeasure | undefined) { renderWithProviders(
@@ -183,9 +200,10 @@ describe('the /apps route set that renders the shared chrome', () => { '/apps/submit', ]); // Both classes are represented, so the loops below exercise the measured AND the - // measure-free branch of the layout rather than one of them 13 times. - expect(ROUTES.filter((r) => r.measure === undefined)).toHaveLength(5); - expect(ROUTES.filter((r) => typeof r.measure === 'number')).toHaveLength(8); + // measure-free branch of the layout rather than one of them 13 times. 6/7 since + // `/apps/review` gave up its 1368 cap and joined the full-container list. + expect(ROUTES.filter((r) => r.measure === undefined)).toHaveLength(6); + expect(ROUTES.filter((r) => r.measure !== undefined)).toHaveLength(7); }); }); @@ -236,17 +254,18 @@ describe.each(VIEWPORTS)( // route, measured or not. A centred measure box would put it at // `navLeft + (contentWidth - m) / 2` and fail here — which is the whole // reason the box carries no auto margins. - [navLeft, m === undefined ? contentWidth : Math.min(m, contentWidth)], + [navLeft, resolveMeasure(m, contentWidth)], ]) ); expect(seen).toEqual(expected); // And the measured routes are genuinely DISTINCT widths, so the fixture varies - // the dimension under test instead of feeding one value 13 times. + // the dimension under test instead of feeding one value 13 times. TWO classes + // since the narrow-table cap was deleted. const measuredWidths = new Set( - ROUTES.filter((r) => typeof r.measure === 'number').map((r) => seen[r.route][1]) + ROUTES.filter((r) => r.measure !== undefined).map((r) => seen[r.route][1]) ); - expect(measuredWidths.size).toBe(3); + expect(measuredWidths.size).toBe(2); }); test('🔴 a measured page HEADER is bounded too, and the band keeps its 16/32 grouping', async () => { diff --git a/src/components/Apps/AppsPageLayout.tsx b/src/components/Apps/AppsPageLayout.tsx index ffcb6f388b..037075f884 100644 --- a/src/components/Apps/AppsPageLayout.tsx +++ b/src/components/Apps/AppsPageLayout.tsx @@ -1,7 +1,11 @@ import { Box, Container, Group, Stack, Text, Title } from '@mantine/core'; import type { ReactNode } from 'react'; import { AppsSubNav } from '~/components/Apps/AppsSubNav'; -import { APPS_PAGE_CONTAINER_WIDTH } from '~/components/Apps/appsPageWidths'; +import { + APPS_PAGE_CONTAINER_WIDTH, + appsMeasureCss, + type AppsMeasure, +} from '~/components/Apps/appsPageWidths'; /** * Shared chrome for every `/apps/*` surface. @@ -85,8 +89,15 @@ export function AppsPageLayout({ * Values come from `APPS_PAGE_MEASURES` in `~/components/Apps/appsPageWidths` — * they are CONTENT widths (the old container widths minus the `Container`'s own * `2 × 16px` gutter), because this box sits INSIDE that gutter. + * + * 🔴 A MEASURE IS NOW EITHER A NUMBER OR A BAND, and this layout does NOT decide + * which: it hands whatever it is to `appsMeasureCss` and applies the result. A band + * renders as a `clamp()` whose middle term is a PERCENTAGE, which resolves against + * this box's containing block — the `Container`'s content box — so the ramp is + * bounded by the container's own cap. Nothing about the box changes: still one + * `maw`, still no margin, still a direct child of the root stack. */ - measure?: number; + measure?: AppsMeasure; children: ReactNode; }) { const hasHeader = Boolean(title || subtitle || actions); @@ -116,7 +127,8 @@ export function AppsPageLayout({ * RESOLVED geometry — body left edge == nav left edge on every route — which is * the check that catches a centring mechanism of any spelling. */ - const bounded = (node: ReactNode) => (measure != null ? {node} : node); + const bounded = (node: ReactNode) => + measure != null ? {node} : node; // `pb` only — NO `py`. The top pad is deliberately gone so `/apps/*` starts // directly under the global header instead of 16px below it; the BOTTOM pad // stays because this Container is the outermost element on every apps page, so diff --git a/src/components/Apps/AppsWideLayout.geometry.test.tsx b/src/components/Apps/AppsWideLayout.geometry.test.tsx new file mode 100644 index 0000000000..b4677c2949 --- /dev/null +++ b/src/components/Apps/AppsWideLayout.geometry.test.tsx @@ -0,0 +1,981 @@ +/** + * `/apps/*` SPENDS ITS WIDTH — the rendered proof, at two named container widths. + * + * ───────────────────────────────────────────────────────────────────────────── + * WHAT IS BEING GUARDED + * ───────────────────────────────────────────────────────────────────────────── + * The ultrawide pass raised the shared apps container 1920 → 2560, so a route with no + * body measure went from 1888 to 2528 of content. Nothing was clipped and nothing + * errored — the extra 640px simply became PADDING, which on a `space-between` row lands + * entirely between a row's content and the control that acts on it. + * + * Two mechanisms answer that: + * + * · `AppsTableColgroup` — percentage widths on every column except the primary, which + * is left `auto` so the surplus lands there. + * · `AppsCardGrid` — `/apps/installed`'s cards step to a second column exactly where the + * surplus appeared, so a card's own width stops tracking the container and the + * name→Manage gap stops growing. + * + * 🔴 THE SPLIT WITH THE UNIT TIER IS NOT WHAT THIS PARAGRAPH ORIGINALLY SAID. It claimed a + * misplaced `
` is "ignored SILENTLY" and that `__tests__/appsWideLayout.test.ts` + * "cannot see any of that". Both halves were refuted by mutation, in opposite directions: + * + * - a `` moved AFTER `` changed **no rendered width at all** (every + * assertion in this file stayed green), because React inserts nodes through the DOM API + * so the HTML parser's table foster-parenting never runs and Chromium honours the + * columns wherever the element sits — while the unit file's structural guard went red; + * - a `` DELETED entirely does turn these assertions red, which is the positive + * control proving that green was about placement rather than about this tier being + * blind. + * + * So: PLACEMENT and ledger↔table COLUMN COUNT are owned by `__tests__/appsWideLayout.test.ts` + * (AST, per-table, in the blocking tier). WIDTHS are owned here. Neither is a substitute + * for the other, and the sentence that said one of them saw everything was wrong. + * + * ───────────────────────────────────────────────────────────────────────────── + * 🔴 WHY THE `geometry` PROJECT AND NOT `component` + * ───────────────────────────────────────────────────────────────────────────── + * Every number here depends on the Mantine `Container`'s `max-width`/`padding-inline`, + * on `Table`'s `width: 100%` and its cell padding, and on the CASCADE LAYER ORDER that + * decides which of Tailwind's preflight, this repo's `globals.css` and Mantine's own + * sheets wins. `test/component-setup.tsx` injects the `:root` custom properties ONLY — + * 24 CSS rules, no preflight, no Mantine component rules — so in that tier a `
` is + * unstyled, every column is content-width, and the container has no cap at all. The + * measurements would be internally consistent and about a different page. + * + * The harness asserts the cascade actually arrived (`cascadeEvidence()`), so a stylesheet + * that fails to load fails the run rather than quietly reproducing the defect's numbers. + * + * ───────────────────────────────────────────────────────────────────────────── + * THE TWO WIDTHS ARE NAMED, AND NEITHER SITS ON A THRESHOLD + * ───────────────────────────────────────────────────────────────────────────── + * 1440 — the ordinary desktop, BELOW the old 1920 container, so it is the "nothing may + * move here" reference. Content: 1440 − 32 = 1408. + * 2560 — exactly the container's cap, the widest full-bleed case. Content: 2528. + * + * The card grid's second column arrives at 2416 of content — 1008 above the first fixture + * and 112 below the second, so neither measurement is on the rung. One measurement is not + * a general claim, which is why every assertion below is a comparison BETWEEN the two. + * + * ───────────────────────────────────────────────────────────────────────────── + * ⚠️ WHAT THIS FILE DOES NOT PROVE, STATED SO NOBODY READS IT AS PROVEN + * ───────────────────────────────────────────────────────────────────────────── + * The `/apps/installed` block below mounts the real `InstalledAppCard` inside + * `AppsCardGrid` — but the GRID IS SUPPLIED BY THE TEST, so these assertions say "the card + * behaves correctly when it is in the grid", not "the page puts it in one". Measured + * against `origin/main`'s components with only the new module scaffolded in, the two + * installed tests PASSED while the four table tests went red — i.e. this file alone cannot + * see the page reverting to a `Stack`. That claim is `__tests__/appsWideLayout.test.ts`'s + * "🔴 /apps/installed uses the card GRID, and no longer caps its body", which is red at + * `origin/main` for exactly that reason. Two guards, one for the mechanism and one for its + * adoption; neither is a substitute for the other. + */ +import { describe, expect, test, vi } from 'vitest'; +import { cleanup } from 'vitest-browser-react'; +// `test/` lives outside `src`, so the `~` alias doesn't reach it — relative import. +import { cascadeEvidence, nextLayout, renderAtViewport } from '../../../test/geometry-setup'; +import type * as TrpcMod from '~/utils/trpc'; +import type { GroupedApp } from '~/components/Apps/groupSubscriptionsByApp'; +import type { SubscriptionRecord } from '~/server/schema/blocks/subscription.schema'; +import type { MyAppRow } from '~/components/Apps/myAppsView'; +import type { OffsiteReviewRequest, OnsiteReviewRequest } from '~/components/Apps/unifiedReviewRow'; +import { capabilitiesForKind } from '~/shared/constants/app-capabilities.constants'; + +// The sub-nav needs a qualifying viewer or it renders no `` every column + // grows — automatic table layout distributes surplus across all of them in proportion + // to their content — so "the App column got wider" is satisfied by the DEFECT. What + // separates the two is HOW MUCH of the 1120px it took: the ledger gives it 54% of the + // table, i.e. more than the other four columns put together. + const { narrow, wide } = await atBothWidths(list, headerWidths); + expect(narrow, 'the queue renders five columns on the Pending tab').toHaveLength(5); + expect(wide).toHaveLength(5); + + const appDelta = wide[1] - narrow[1]; + const otherDelta = wide.reduce((s, w, i) => (i === 1 ? s : s + (w - narrow[i])), 0); + + expect(appDelta, 'the App column did not grow at all').toBeGreaterThan(0); + expect( + appDelta, + `the App column took ${px(appDelta)} of the container's ${CONTAINER_DELTA}px, and the ` + + `other four columns took ${px(otherDelta)} between them — the primary column is ` + + 'supposed to absorb the slack' + ).toBeGreaterThan(otherDelta); + // …and the two together account for the whole container delta, so nothing has been + // silently spent as table margin. + expect(px(appDelta + otherDelta)).toBeCloseTo(CONTAINER_DELTA, 0); + }); + + test('the non-primary columns hold their declared share at the wide width', async () => { + // The other half of "proportional": the fixed columns are a PERCENTAGE of the table, + // not a content width that happens to have grown. Asserted at the wide fixture only, + // because at 1408 a column can legitimately exceed its share (min-content wins). + await renderRoute(list(), WIDE); + const widths = headerWidths(); + const table = document.querySelector('table')!.getBoundingClientRect().width; + for (const [index, share] of [ + [0, 6], + [2, 6], + [3, 9], + [4, 6], + ] as const) { + expect(px(widths[index]), `column ${index} should be ${share}% of ${px(table)}`).toBeCloseTo( + (share / 100) * table, + 0 + ); + } + await cleanup(); + }); +}); + +// ── table route 2: /apps/mine ──────────────────────────────────────────────── + +describe('/apps/mine — the author table spends the width on its App column', () => { + const body = () => ; + + test('the App column grows with the container, and takes MOST of the surplus', async () => { + const { narrow, wide } = await atBothWidths(body, headerWidths); + expect(narrow, 'the author table renders four columns').toHaveLength(4); + expect(wide).toHaveLength(4); + + const appDelta = wide[0] - narrow[0]; + const otherDelta = wide.reduce((s, w, i) => (i === 0 ? s : s + (w - narrow[i])), 0); + + expect(appDelta).toBeGreaterThan(0); + expect( + appDelta, + `the App column took ${px(appDelta)} and Cover/Status/Updated took ${px(otherDelta)}` + ).toBeGreaterThan(otherDelta); + }); + + test('the App column is measurably the widest at BOTH widths', async () => { + // A second, independent reading of the same decision: the primary column is the one + // carrying the icon, the name and the slug, so it must never be out-grown by the + // date column at either measurement point. + const { narrow, wide } = await atBothWidths(body, headerWidths); + expect(narrow[0]).toBe(Math.max(...narrow)); + expect(wide[0]).toBe(Math.max(...wide)); + // …and the header cell the ledger's primary is documented against is the one measured. + await renderRoute(body(), WIDE); + expect(widthOf('apps-mine-col-app')).toBe(px(headerWidths()[0])); + await cleanup(); + }); +}); + +// ── table route 3: /apps/review's ACTIVE PREVIEWS — the payload of round 1 ─── + +describe('/apps/review — the active-previews panel keeps its buttons near its rows', () => { + /** + * 🔴 THE TABLE THE FIRST PASS EXPOSED. `/apps/review` renders four tables and the change + * that removed its 1368 body cap ledgered two of them. Measured on this one WITHOUT a + * ledger, 1440 → 2560: + * + * columns 228.02 | 165.17 | 146.45 | 152.05 | 682.31 + * 413.89 | 299.83 | 265.84 | 276.00 | 1238.44 + * slug → "Tear down" 817.36 → 1381.23 (+563.87) + * + * i.e. removing the cap re-opened, on this table, exactly the defect the cap had been + * suppressing. The ledger makes the ACTION column primary (case (b)), so the four short + * data columns stay at their own widths and the surplus lands past the buttons. + */ + function slugToTeardownGap(): number { + const slug = document.querySelector('table tbody code'); + const buttons = Array.from(document.querySelectorAll('table tbody button')); + const teardown = buttons.find((b) => (b.textContent ?? '').includes('Tear down')); + if (!slug || !teardown) { + throw new Error( + `the previews panel did not render its row (slug=${!!slug} teardown=${!!teardown})` + ); + } + // The glyphs' own box, not the cell's — a cell already spans its column at every width. + const range = document.createRange(); + range.selectNodeContents(slug); + return px(teardown.getBoundingClientRect().left - range.getBoundingClientRect().right); + } + + const panel = () => ; + + test('the panel renders its row at all (guards a vacuous measurement)', async () => { + // Every assertion below is a comparison of two numbers read off this row. If the trpc + // fixture stopped resolving, the panel returns `null` and the helpers throw — but the + // COUNT is what proves the fixture shape is still the two-button LIVE one. + await renderRoute(panel(), WIDE); + expect(document.querySelectorAll('table tbody tr')).toHaveLength(1); + // ONE `` REMOVED, not against a + // literal and not against the other widths. Rows legitimately get taller at 768 for + // ANY table — less width means more wrapping — so "not taller than at 2560" is a claim + // no correct table could satisfy. What a ledger must never do is make a row taller + // than the browser's own layout would at THAT width, and the only honest baseline for + // that is natural layout of the same content. Detaching the `` and + // re-measuring gives exactly that, in one render. + const offenders: string[] = []; + for (const vp of ALL_WIDTHS) { + const observed = await renderRoute(ui(), vp); + expect(observed).toEqual({ width: vp.width, height: vp.height }); + const withLedger = firstRowHeight(); + const colgroup = document.querySelector('table > colgroup'); + expect(colgroup, 'this case is supposed to be a LEDGERED table').not.toBeNull(); + colgroup!.remove(); + await nextLayout(); + const natural = firstRowHeight(); + if (withLedger > natural + 0.01) { + offenders.push( + `@${vp.width}: ${withLedger} with the ledger vs ${natural} without it ` + + `(+${px(withLedger - natural)})` + ); + } + await cleanup(); + } + expect( + offenders, + 'the column ledger made rows TALLER than the browser lays them out unaided — a share ' + + 'is below its cell content at that width, which a width assertion cannot see' + ).toEqual([]); + }); +}); + +// ── /apps/installed — the 640px dead gap ───────────────────────────────────── + +describe('/apps/installed — the space-between row keeps its control near its content', () => { + /** + * The gap between the app NAME's right edge and the Manage button's left edge. + * + * 🔴 MEASURED ON THE TEXT, NOT ON ITS CELL. The row's left child is `flex: 1`, so its + * BOX already spans the whole row at every width — reading the cell would report a + * constant zero gap and pass against the defect. What actually recedes is the button + * relative to the glyphs, which is what a moderator or an owner sees. + */ + function nameToButtonGap(): number { + const nameCell = Array.from(document.querySelectorAll('[data-apps-card-grid] .truncate')).at(0); + const button = Array.from(document.querySelectorAll('[data-apps-card-grid] button')).at(-1); + if (!nameCell || !button) throw new Error('the installed card did not render its row'); + // A `range` around the text node gives the glyphs' own box rather than the flex cell's. + const range = document.createRange(); + range.selectNodeContents(nameCell); + return px(button.getBoundingClientRect().left - range.getBoundingClientRect().right); + } + + const grid = () => ( + + + + ); + + test('🔴 the gap does NOT grow when the container does', async () => { + // The recorded defect, as a comparison: at 1920 → 2560 the audit measured this gap + // growing by exactly the container's own 640px, because a full-width card hands every + // extra pixel to the space between the name and the button. Two named widths, because + // one measurement is not a claim about a dimension. + const { narrow, wide } = await atBothWidths(grid, nameToButtonGap); + expect(narrow, 'the narrow fixture measured no gap at all').toBeGreaterThan(0); + expect( + wide, + `the name→Manage gap went ${narrow} → ${wide} across a ${CONTAINER_DELTA}px container ` + + 'increase; the card grid is supposed to spend that on a second column' + ).toBeLessThanOrEqual(narrow); + }); + + test('…because the CARD stops tracking the container (one column, then two)', async () => { + // The mechanism, stated separately from its consequence so a future change that keeps + // the gap constant some other way is still legible. The card is full-width at 1408 and + // roughly half-width at 2528. + const { narrow, wide } = await atBothWidths(grid, () => + px(document.querySelector('[data-apps-card-grid] > *')!.getBoundingClientRect().width) + ); + expect(narrow).toBe(NARROW.content); + // Two 1fr tracks with a 16px gap: (2528 − 16) / 2 = 1256. + expect(wide).toBe(1256); + expect(wide).toBeLessThan(narrow); + }); + + test("🔴 the Hidden tab's 12px gap gives the SAME rung, measured in the browser", async () => { + // F8's equivalence, in the engine rather than in arithmetic. `appsCardGridColumnsAt` + // MIRRORS the CSS; this reads the CSS. Two tracks either way, and the child is the + // gap's own width narrower — which is also the only consumer the `gap` prop has, so + // deleting the prop is visible here as well as at its call site. + const { observed } = await renderAtViewport( + + + + + + , + WIDE + ); + expect(observed).toEqual({ width: WIDE.width, height: WIDE.height }); + const gridEl = document.querySelector('[data-apps-card-grid]') as HTMLElement; + expect(getComputedStyle(gridEl).columnGap).toBe('12px'); + // Two 1fr tracks with a 12px gap: (2528 − 12) / 2 = 1258 — the same TWO columns the + // 16px default yields at this width, which is the whole claim. + expect(px((gridEl.firstElementChild as HTMLElement).getBoundingClientRect().width)).toBe(1258); + await cleanup(); + }); + + test('🔴 ON A PHONE the card fits the screen — the `min(100%, …)` is load-bearing', async () => { + // 🔴 THE ONE ASSERTION THAT MAKES THAT `min()` MORE THAN A COMMENT. Without it the + // track floor is a flat 1200px, and neither of this file's other fixtures is narrower + // than that — so dropping it passed the whole suite. Measured at 390×844 with the + // `min()` removed: gridBox 358, gridScroll 1200, child 1200, and + // `document.scrollWidth` UNCHANGED — the card is CLIPPED at the grid's edge with no + // scrollbar and no page overflow to notice it by, which is worse than the "overflows + // horizontally" the docstring used to claim. This route is phone-reachable, and this + // component converted three of its lists from `Stack` to grid. + const PHONE = { width: 390, height: 844 } as const; + const { observed } = await renderAtViewport( + {grid()}, + PHONE + ); + expect(observed).toEqual({ width: PHONE.width, height: PHONE.height }); + const gridEl = document.querySelector('[data-apps-card-grid]') as HTMLElement; + const child = gridEl.firstElementChild as HTMLElement; + const gridBox = px(gridEl.getBoundingClientRect().width); + const childBox = px(child.getBoundingClientRect().width); + // ONE column, and the card is inside the grid rather than hanging out of it. + expect(childBox).toBeLessThanOrEqual(gridBox); + // …and the grid is not itself a scroll container hiding the overflow. + expect(gridEl.scrollWidth).toBeLessThanOrEqual(Math.ceil(gridBox)); + // The positive control on the two assertions above: the grid really is narrower than + // the track floor here, so this fixture CAN see the defect. Without this, a viewport + // that quietly grew past 1200 would make both checks vacuous. + expect(gridBox).toBeLessThan(1200); + await cleanup(); + }); +}); diff --git a/src/components/Apps/MyAppsBody.tsx b/src/components/Apps/MyAppsBody.tsx index d929aa5343..c66833259b 100644 --- a/src/components/Apps/MyAppsBody.tsx +++ b/src/components/Apps/MyAppsBody.tsx @@ -46,6 +46,7 @@ import { sortByRecentlyUpdated, } from '~/components/Apps/myAppsView'; import { ownerListingState, ownerStateChip } from '~/components/Apps/offsiteOwnerControls'; +import { AppsTableColgroup, APPS_MINE_COLUMNS } from '~/components/Apps/appsWideLayout'; import { useFeatureFlags } from '~/providers/FeatureFlagsProvider'; import { canOpenListingAuthoringPage } from '~/shared/constants/app-capabilities.constants'; import { formatDate } from '~/utils/date-helpers'; @@ -513,9 +514,16 @@ function AppGroup({ return (
`s of the first table on the page. */ +function bodyCells(): Element[] { + return Array.from(document.querySelectorAll('table tbody tr:first-child > td')); +} + +describe('/apps/review reports — the other table a ledger cannot help', () => { + /** + * 🔴 UNLEDGERED AND DELIBERATELY SO — `__tests__/appsWideLayout.test.ts` requires this arm + * BY NAME for the `no-surplus` exemption, so deleting it turns the exemption red rather + * than leaving it an unmeasured claim. Three ledgers were tried and every one was either + * taller than natural at 1200 or clipped the `lineClamp={2}` details harder. + */ + const queue = () => ; + + test('the queue renders its row (guards a vacuous measurement)', async () => { + await renderRoute(queue(), WIDE); + expect(headerWidths()).toHaveLength(6); + expect(bodyCells()).toHaveLength(6); + expect(document.querySelectorAll('table tbody button').length).toBeGreaterThanOrEqual(1); + await cleanup(); + }); + + test('the reports table is no worse than natural at every width', async () => { + // It carries no colgroup, so "natural" is what it renders — the assertion is that the + // recorded band is still what the browser produces. A value pin with provenance: these + // are the four numbers the exemption was decided on, and a copy change that moves them + // should be a decision rather than a drift. + const heights = await atEachWidth(queue, firstRowHeight); + expect(document.querySelector('table > colgroup')).toBeNull(); + expect( + heights, + `row height at ${ALL_WIDTHS.map((v) => v.width).join('/')} — the band the no-surplus ` + + 'exemption was measured against' + ).toEqual([177.88, 88.69, 88.69, 82.89]); + }); + + test('🔴 the details box is CAPPED, which is why no column could absorb the slack', async () => { + // The measurement that rejected `Reason` as a primary: its text is capped at 260px, so + // a column given the surplus renders a wider cell around an identical sentence. + const detailsBox = () => { + const cell = bodyCells()[1]; + const texts = Array.from(cell.children); + return px(texts[texts.length - 1].getBoundingClientRect().width); + }; + const boxes = await atEachWidth(queue, detailsBox); + expect(Math.max(...boxes)).toBeLessThanOrEqual(260); + }); +}); + +describe('/apps/installed activity — the table that a ledger cannot help', () => { + /** + * 🔴 THIS TABLE IS DELIBERATELY UNLEDGERED, and this arm is what keeps that decision + * honest — `__tests__/appsWideLayout.test.ts` requires it BY NAME for the `no-surplus` + * exemption, so deleting it turns the exemption red rather than silently unmeasured. + * + * Its natural layout already renders every cell on one line at every width, because its + * max-content sum (~735px) is the container's content width at 768. Both ledgers that + * shipped made rows TALLER — 48.09 and 64.89 against 36.19 — and neither was visible to a + * width assertion at 1440/2560. + */ + const panel = () => ; + + test('the fixture is the RICH shape (guards a vacuous measurement)', async () => { + // On a passive row every cell is short and nothing can wrap, so the heights below + // would agree for a reason that has nothing to do with the layout. + await renderRoute(panel(), WIDE); + const cells = Array.from(document.querySelectorAll('table tbody tr:first-child > td')); + expect(cells).toHaveLength(5); + expect(cells[2].textContent).toContain('Tipped'); + expect(cells[3].textContent).toContain('/api/v1/buzz/tip'); + await cleanup(); + }); + + test('the activity table renders ONE LINE per cell at every width', async () => { + // 🔴 THE ARM THE EXEMPTION IS NAMED AGAINST. Four widths, two of them below 1440, + // asserting HEIGHT — the three things this tier lacked when the two bad ledgers passed. + const heights = await atEachWidth(panel, firstRowHeight); + expect( + heights, + `row height at ${ALL_WIDTHS.map((v) => v.width).join('/')} — every value must be the ` + + 'single-line height; a taller one means a column was squeezed below its content' + ).toEqual([36.19, 36.19, 36.19, 36.19]); + }); + + test('🔴 DETAIL is a fixed token — no layout can give it a usable pixel', async () => { + // Round 2 made this the PRIMARY column on the strength of its name. Its glyph box is + // identical at every width, which is half of why no ledger helps this table: one of + // the two cells that would have to absorb the surplus cannot. + const glyphs = await atEachWidth(panel, () => + glyphWidth( + (Array.from(document.querySelectorAll('table tbody tr:first-child > td'))[3] as Element) + .firstElementChild + ) + ); + expect(new Set(glyphs).size, `Detail glyph widths were ${glyphs.join(' / ')}`).toBe(1); + }); + + test('…and ACTION, the other candidate, is a BOUNDED sentence', async () => { + // The other half. It is genuinely variable — unlike `Detail` — but it stops growing, + // so handing it the surplus would park the remainder mid-row. Constant here because + // natural layout already gives it more than it needs at every width. + const read = () => { + const cell = Array.from(document.querySelectorAll('table tbody tr:first-child > td'))[2]; + return { + glyph: glyphWidth(cell.firstElementChild), + cell: px(cell.getBoundingClientRect().width), + }; + }; + const measured = await atEachWidth(panel, read); + expect(new Set(measured.map((m) => m.glyph)).size).toBe(1); + for (const m of measured) expect(m.cell).toBeGreaterThan(m.glyph); + // Guard-the-guard: an empty sentence would satisfy both trivially. + expect(measured[0].glyph).toBeGreaterThan(100); + }); + + test('…and the two cells a ledger squeezed are each ONE line box', async () => { + // The mechanism behind the height, so a future change that keeps the height constant + // some other way is still legible. `When` broke a `YYYY-MM-DD HH:mm` stamp across three + // lines under the shipped ledger; `Detail`'s monospace ref broke across two. + const linesPerWidth = await atEachWidth(panel, () => { + const cells = Array.from(document.querySelectorAll('table tbody tr:first-child > td')); + return [lineCount(cells[0].firstElementChild), lineCount(cells[3].firstElementChild)]; + }); + expect(linesPerWidth).toEqual([ + [1, 1], + [1, 1], + [1, 1], + [1, 1], + ]); + }); +}); + +describe('🔴 NO LEDGER MAKES ITS ROWS TALLER AT A NARROWER WIDTH', () => { + /** + * THE TIER-WIDE INVARIANT, and the one that would have caught both bad ledgers. + * + * A percentage share is smallest in absolute px at the NARROWEST container, so a share + * sized from a 1408 measurement can sit below its cell's content at 768 and 1200. The + * cell does not then get narrower than a width assertion expects — it gets TALLER. Every + * arm in this file read a width at 1440/2560 only, so two ledgers shipped green: + * `AppActivityPanel`'s rows were 48.09 and then 64.89 against a natural 36.19. + * + * The invariant is stated as SHAPE rather than as a number: a table's row height must not + * increase as the container gets narrower. It is deliberately not "equals N px" — these + * tables have different row contents and a literal per table would rot on any copy + * change — and it is not "equals the no-ledger height" either, because this tier cannot + * render a component with its own colgroup removed. + */ + const CASES = [ + { + name: '/apps/review queue', + ui: () => ( + + ), + }, + { name: '/apps/review previews', ui: () => }, + { name: '/apps/mine', ui: () => }, + ] as const; + + test('the case list covers every LEDGERED table this file can mount', () => { + // A loop over a list nobody pinned passes vacuously when the list shrinks. + expect(CASES.map((c) => c.name)).toEqual([ + '/apps/review queue', + '/apps/review previews', + '/apps/mine', + ]); + }); + + test.each(CASES)('$name — the ledger costs no vertical space at any width', async ({ ui }) => { + // 🔴 MEASURED AGAINST THE SAME TREE WITH ITS `
+ {/* + 🔴 FIRST CHILD, BEFORE the row groups — HTML requires it there; the ordering is + pinned by `__tests__/appsWideLayout.test.ts`. `App` carries the icon, the name and + the slug and is the ledger's primary column, so the container's surplus width + lands there instead of being distributed as padding across four columns. + */} + - App + App Cover Status Updated diff --git a/src/components/Apps/OffsiteReviewQueue.tsx b/src/components/Apps/OffsiteReviewQueue.tsx index 2b4d53f46b..e4f452a4ae 100644 --- a/src/components/Apps/OffsiteReviewQueue.tsx +++ b/src/components/Apps/OffsiteReviewQueue.tsx @@ -1349,6 +1349,12 @@ export function OffsiteReportsQueue() { ) : (
+ {/* 🔴 NO COLUMN LEDGER, DELIBERATELY — EXEMPT in `~/components/Apps/appsWideLayout` + as `no-surplus`. At 1200 this row's content wants App 240 + Reason 292 (to + reach its 260px details cap) + Reporter 94 + Reported 133 + Status 86 + + actions 414 = 1259px in 1168px of container, so SOMETHING is under-served + there whatever the split. Measured: every candidate ledger was either taller + than natural at 1200 or clipped the lineClamp-ed details harder. */} App @@ -1391,16 +1397,30 @@ export function OffsiteReportsQueue() { )} - {r.reporter?.username ?? `#${r.reporter?.id ?? '?'}`} + {/* 🔴 `nowrap` — these three columns carry shrink-to-content shares, and + a share below min-content only holds one line when min-content is the + whole label. A username, a date and a status badge are each one + token; wrapping them was what made these rows taller than the + browser's own layout. */} + + {r.reporter?.username ?? `#${r.reporter?.id ?? '?'}`} + - + - {formatDate(r.createdAt)} + + {formatDate(r.createdAt)} + - + {statusChip.label} diff --git a/src/components/Apps/ReportTabs.tsx b/src/components/Apps/ReportTabs.tsx index d518ed6616..f7c852b505 100644 --- a/src/components/Apps/ReportTabs.tsx +++ b/src/components/Apps/ReportTabs.tsx @@ -26,6 +26,10 @@ import { type ScopeVerdictsView, type SecurityAuditView, } from '~/components/Apps/agentReviewReport'; +import { + AppsTableColgroup, + APPS_AGENT_REPORT_SCOPE_COLUMNS, +} from '~/components/Apps/appsWideLayout'; /** * App Blocks — AGENTIC MOD CODE-REVIEW report renderer (P2, Phase-2 redesign). @@ -534,6 +538,10 @@ export function ScopesTab({ fz="xs" data-testid="scope-verdicts-table" > + {/* 🔴 FIRST CHILD, BEFORE the row groups — see `appsWideLayout`. Reachable on + `/apps/review/[publishRequestId]` (OnsiteReviewModalBody → AgentReviewPanel), + which takes the full container. Inert in the modal, load-bearing on the page. */} + Scope diff --git a/src/components/Apps/UnifiedReviewList.tsx b/src/components/Apps/UnifiedReviewList.tsx index 69ca4b2629..90485e48c3 100644 --- a/src/components/Apps/UnifiedReviewList.tsx +++ b/src/components/Apps/UnifiedReviewList.tsx @@ -9,6 +9,7 @@ import { import { useMemo, useState } from 'react'; import type { OffsitePendingRow } from '~/components/Apps/OffsiteReviewQueue'; import { canRetriggerBuild } from '~/components/Apps/deploy-status'; +import { AppsTableColgroup, APPS_REVIEW_QUEUE_COLUMNS } from '~/components/Apps/appsWideLayout'; import { mergeReviewRows, offsiteRequestToUnifiedRow, @@ -118,10 +119,29 @@ export function UnifiedReviewList({ {rows.length > 0 && (
+ {/* + 🔴 FIRST CHILD, BEFORE the row groups — HTML requires it there, and + `__tests__/appsWideLayout.test.ts` is what enforces it (see the note on that + guard for what the PIXELS can and cannot see about the ordering). Its ledger + is keyed on `showDeploy`, i.e. on the same DATA that decides whether the + Deploy column exists, so the two can never disagree about the column COUNT. + Why this table has one at all: `/apps/review` used to cap its whole page at + 1368 because these columns could not spend the container and the Review + button drifted away from its row. + */} + Kind - App + {/* The PRIMARY column — no width in the ledger, so it takes the slack. + The testid is what `AppsWideLayout.geometry.test.tsx` measures. */} + App Submitter {dateLabel} {showDeploy && Deploy} @@ -275,6 +295,11 @@ function UnifiedReviewRowView({ @@ -297,9 +322,12 @@ function UnifiedReviewRowView({ - + {/* Same reason as the kind badge: a timestamp is one token and must not wrap. */} + - {row.submittedAt.toLocaleString()} + + {row.submittedAt.toLocaleString()} + {showDeploy && ( diff --git a/src/components/Apps/__tests__/appsPageLayout.test.ts b/src/components/Apps/__tests__/appsPageLayout.test.ts index 5e77a3a131..8b2014fa6e 100644 --- a/src/components/Apps/__tests__/appsPageLayout.test.ts +++ b/src/components/Apps/__tests__/appsPageLayout.test.ts @@ -126,8 +126,14 @@ describe('AppsPageLayout takes NO per-page container width', () => { // The state, not a keyword: the Container's `size` is THE shared constant. expect(src).toMatch(/ { const src = layoutSrc(); expect(src.length).toBeGreaterThan(1000); expect(src).toMatch(/export function AppsPageLayout/); - expect(src).toMatch(/measure\?:\s*number/); + // `AppsMeasure` since the band pass — a number OR a `{min,max,grow}` band. The prop + // still exists and is still the only width the caller controls, which is what this + // guard-the-guard is for. + expect(src).toMatch(/measure\?:\s*AppsMeasure/); }); }); diff --git a/src/components/Apps/__tests__/appsPageLayoutRender.test.ts b/src/components/Apps/__tests__/appsPageLayoutRender.test.ts index 95ebe5a83c..a58ac03e05 100644 --- a/src/components/Apps/__tests__/appsPageLayoutRender.test.ts +++ b/src/components/Apps/__tests__/appsPageLayoutRender.test.ts @@ -3,6 +3,7 @@ import { renderToStaticMarkup } from 'react-dom/server'; import { MantineProvider } from '@mantine/core'; import { Window } from 'happy-dom'; import { describe, expect, it, vi } from 'vitest'; +import type { AppsMeasure } from '~/components/Apps/appsPageWidths'; /** * `/apps` chrome — the measure box, asserted on the RENDERED TREE, in the tier that @@ -49,13 +50,26 @@ vi.mock('~/components/Apps/AppsSubNav', () => ({ })); const { AppsPageLayout } = await import('~/components/Apps/AppsPageLayout'); -const { APPS_PAGE_MEASURES, APPS_PAGE_CONTAINER_WIDTH } = await import( +const { APPS_PAGE_MEASURES, APPS_PAGE_CONTAINER_WIDTH, appsMeasureCss } = await import( '~/components/Apps/appsPageWidths' ); /** Mantine emits `max-width:calc(rem * var(--mantine-scale))` for `maw={n}`. */ const remOf = (px: number) => `${px / 16}rem`; +/** + * The substring the rendered `max-width` must contain for a given measure. + * + * 🔴 TWO SHAPES, ONE ASSERTION. A numeric measure is converted to `rem` by Mantine; a + * BAND is a `clamp(…)` string, which Mantine's `rem()` early-returns untouched. Reading + * both through the module's own `appsMeasureCss` is deliberate — a second hand-written + * copy of the formatter here would agree with itself while disagreeing with the layout. + */ +const expectedMaxWidth = (measure: AppsMeasure): string => { + const css = appsMeasureCss(measure); + return typeof css === 'number' ? remOf(css) : css; +}; + type Tree = { /** * The measure box wrapping the page BODY, or `null` when there is no measure. @@ -79,7 +93,7 @@ type Tree = { band: Element; }; -function renderLayout(measure?: number, withHeader = false): Tree { +function renderLayout(measure?: AppsMeasure, withHeader = false): Tree { const html = renderToStaticMarkup( createElement( MantineProvider, @@ -236,19 +250,31 @@ describe('the measure box, on the rendered tree', () => { }); it('every route measure renders its own distinct max-width', () => { - const entries = Object.entries(APPS_PAGE_MEASURES); - // Guard-the-guard: an empty map would make the loop pass vacuously. - expect(entries.length).toBeGreaterThanOrEqual(8); + const entries = Object.entries(APPS_PAGE_MEASURES) as [string, AppsMeasure][]; + // Guard-the-guard: an empty map would make the loop pass vacuously. Seven since + // `/apps/review` gave up its cap and joined the full-container list. + expect(entries.length).toBeGreaterThanOrEqual(7); const rendered = new Set(); for (const [route, measure] of entries) { const t = renderLayout(measure); expect(t.measureBox, `${route} rendered no measure box`).not.toBeNull(); - expect(t.measureBox!.getAttribute('style'), route).toContain(remOf(measure)); + expect(t.measureBox!.getAttribute('style'), route).toContain(expectedMaxWidth(measure)); rendered.add(t.measureBox!.getAttribute('style') ?? ''); } - // Three measure CLASSES, so three distinct rendered widths — proof the fixture - // varies the dimension rather than feeding one value eight times. - expect(rendered.size).toBe(3); + // Two measure CLASSES, so two distinct rendered widths — proof the fixture varies + // the dimension rather than feeding one value seven times. + expect(rendered.size).toBe(2); + }); + + it('🔴 a BAND reaches the DOM as a clamp, not as a rem floor', () => { + // The failure this catches is a measure that renders its `min` and stops growing — + // which looks completely correct at 1440 and silently un-does the whole band on a + // wide screen. `remOf(min)` must NOT be what lands in the attribute. + const band = { min: 1068, max: 1368, grow: 55 } as const; + const t = renderLayout(band); + const style = t.measureBox!.getAttribute('style') ?? ''; + expect(style).toContain('clamp(1068px, 55%, 1368px)'); + expect(style).not.toContain(remOf(1068)); }); it('🔴 the HEADER is bounded by the same measure as the body', () => { diff --git a/src/components/Apps/__tests__/appsPageWidths.test.ts b/src/components/Apps/__tests__/appsPageWidths.test.ts index 35e8f4ffb4..05df1a1ee3 100644 --- a/src/components/Apps/__tests__/appsPageWidths.test.ts +++ b/src/components/Apps/__tests__/appsPageWidths.test.ts @@ -4,15 +4,21 @@ import path from 'path'; import ts from 'typescript'; import { describe, expect, test } from 'vitest'; import { + APPS_CARD_LIST_GAP, + APPS_CARD_LIST_MIN_COLUMN, APPS_CONTAINER_GUTTER, APPS_FULL_BLEED_PAGES, APPS_FULL_MEASURE_PAGES, - APPS_NARROW_TABLE_MEASURE, + APPS_LEGACY_CONTAINER_WIDTH, APPS_PAGE_CONTAINER_WIDTH, APPS_PAGE_MEASURES, APPS_READABLE_MEASURE, APPS_REDIRECT_ONLY_PAGES, APPS_TWO_COLUMN_DETAIL_MEASURE, + appsMeasureCss, + isAppsMeasureBand, + type AppsMeasure, + type AppsMeasureBand, } from '~/components/Apps/appsPageWidths'; import * as widthsModule from '~/components/Apps/appsPageWidths'; import { @@ -410,28 +416,61 @@ describe('the container is uniform, and it is the only container', () => { }); }); +/** The content width a body gets from the CURRENT shared container. */ +const USABLE = APPS_PAGE_CONTAINER_WIDTH - APPS_CONTAINER_GUTTER; +/** …and from the container this module shipped with before the ultrawide pass. */ +const LEGACY_USABLE = APPS_LEGACY_CONTAINER_WIDTH - APPS_CONTAINER_GUTTER; + +/** What a band resolves to at a given container CONTENT width — the `clamp()`, in JS. */ +function bandAt(band: AppsMeasureBand, contentWidth: number): number { + return Math.min(band.max, Math.max(band.min, (band.grow / 100) * contentWidth)); +} + describe('APPS_PAGE_MEASURES — the decided CONTENT measure per route', () => { - test('the narrow-table measure is 1368 and only /apps/review takes it', () => { - expect(APPS_NARROW_TABLE_MEASURE).toBe(1368); - expect(APPS_PAGE_MEASURES['/apps/review']).toBe(1368); - const takers = Object.entries(APPS_PAGE_MEASURES) - .filter(([, m]) => m === APPS_NARROW_TABLE_MEASURE) - .map(([r]) => r); - expect(takers).toEqual(['/apps/review']); + test('🔴 the NARROW-TABLE class is gone, and /apps/review takes no measure at all', () => { + // It existed for exactly one route and for exactly one reason: four short columns + // could not spend the container, so the surplus landed as padding between the last + // column and the Review button. The columns are proportional now + // (`APPS_REVIEW_QUEUE_COLUMNS`), so the cap would hide the fix rather than help it. + // + // 🔴 THE CONSTANT ITSELF IS ASSERTED ABSENT, not merely unused. A surviving export + // with no consumer is the shape that gets wired back in by the next reader who finds + // a table reading too wide — the module has already lost `APPS_PAGE_WIDTHS` and + // `MY_APPS_CONTAINER_SIZE` the same way. + expect(widthsModule).not.toHaveProperty('APPS_NARROW_TABLE_MEASURE'); + expect(APPS_PAGE_MEASURES).not.toHaveProperty('/apps/review'); + expect(APPS_FULL_MEASURE_PAGES).toContain('/apps/review'); + // Guard-the-guard: an empty namespace satisfies every `not.toHaveProperty`. + expect(widthsModule).toHaveProperty('APPS_READABLE_MEASURE'); }); - test('the two-column detail measure is 1288 and only the store preview takes it', () => { - expect(APPS_TWO_COLUMN_DETAIL_MEASURE).toBe(1288); - expect(APPS_PAGE_MEASURES['/apps/store-preview/[slug]']).toBe(1288); + test('the two-column detail band is 1288 → 1600 and only the store preview takes it', () => { + expect(APPS_TWO_COLUMN_DETAIL_MEASURE).toEqual({ min: 1288, max: 1600, grow: 65 }); + expect(APPS_PAGE_MEASURES['/apps/store-preview/[slug]']).toBe(APPS_TWO_COLUMN_DETAIL_MEASURE); // 🔴 Pinned in BOTH directions so a later "tidy-up" that folds the detail into // another class fails here rather than silently squeezing the right rail - // (readable) or putting the markdown description on a ~1250px measure (full). + // (readable) or putting the markdown description on a ~1685px measure (full). expect(APPS_PAGE_MEASURES['/apps/store-preview/[slug]']).not.toBe(APPS_READABLE_MEASURE); - expect(APPS_PAGE_MEASURES['/apps/store-preview/[slug]']).not.toBe(APPS_PAGE_CONTAINER_WIDTH); + const takers = Object.entries(APPS_PAGE_MEASURES) + .filter(([, m]) => m === APPS_TWO_COLUMN_DETAIL_MEASURE) + .map(([r]) => r); + expect(takers).toEqual(['/apps/store-preview/[slug]']); }); - test('the readable measure is 1068, and these six form/prose routes take it', () => { - expect(APPS_READABLE_MEASURE).toBe(1068); + test('🔴 the two-column CEILING keeps its markdown column inside the readable floor', () => { + // The ceiling is DERIVED rather than picked: the left column is prose at the `md` + // 8/12 span, so the page may only be as wide as leaves that column at or under the + // readable band's own floor. Stated as the relationship, so moving either number + // without the other fails here. + const mainColumn = (APPS_TWO_COLUMN_DETAIL_MEASURE.max * 8) / 12; + expect(mainColumn).toBeLessThanOrEqual(APPS_READABLE_MEASURE.min); + // …and it is not needlessly conservative — one more Mantine grid step (a 1700 cap) + // would break it, which is what makes the bound meaningful rather than arbitrary. + expect((1700 * 8) / 12).toBeGreaterThan(APPS_READABLE_MEASURE.min); + }); + + test('the readable band is 1068 → 1368, and these six form/prose routes take it', () => { + expect(APPS_READABLE_MEASURE).toEqual({ min: 1068, max: 1368, grow: 55 }); const takers = Object.entries(APPS_PAGE_MEASURES) .filter(([, m]) => m === APPS_READABLE_MEASURE) .map(([r]) => r) @@ -447,37 +486,125 @@ describe('APPS_PAGE_MEASURES — the decided CONTENT measure per route', () => { ]); }); - test('every measure is one of the THREE decided values — no fourth hand-picked number', () => { + test('every measure is one of the TWO decided classes — no third hand-picked number', () => { // The whole point of the module is that there are a few CLASSES of apps page, // not eleven bespoke numbers. A new page must join a class, or the class list // must grow deliberately — failing here first. for (const [route, measure] of Object.entries(APPS_PAGE_MEASURES)) { - expect( - [APPS_NARROW_TABLE_MEASURE, APPS_TWO_COLUMN_DETAIL_MEASURE, APPS_READABLE_MEASURE], - `${route}` - ).toContain(measure); + expect([APPS_TWO_COLUMN_DETAIL_MEASURE, APPS_READABLE_MEASURE], `${route}`).toContain( + measure + ); } // Pin the class list itself, as literals. Without this the check above is - // satisfied by ANY set of constants, including a fourth one added silently. - expect([ - APPS_NARROW_TABLE_MEASURE, - APPS_TWO_COLUMN_DETAIL_MEASURE, - APPS_READABLE_MEASURE, - ]).toEqual([1368, 1288, 1068]); + // satisfied by ANY set of constants, including a third one added silently. + expect([APPS_TWO_COLUMN_DETAIL_MEASURE, APPS_READABLE_MEASURE]).toEqual([ + { min: 1288, max: 1600, grow: 65 }, + { min: 1068, max: 1368, grow: 55 }, + ]); }); - test('🔴 a measure is always strictly inside the container', () => { - // A measure ≥ the container is a no-op that reads as a decision. A measure - // larger than the container's usable width is worse: it silently does nothing - // while claiming a class. - const usable = APPS_PAGE_CONTAINER_WIDTH - APPS_CONTAINER_GUTTER; - for (const [route, measure] of Object.entries(APPS_PAGE_MEASURES)) { - expect(measure, `${route} must actually narrow the body`).toBeLessThan(usable); - expect(measure, `${route} must be a positive px value`).toBeGreaterThan(0); + test('🔴 a measure is always strictly inside the container, at BOTH ends of its band', () => { + // A measure ≥ the container is a no-op that reads as a decision. For a band it is + // the CEILING that has to clear it — checking the floor alone would pass a band that + // silently stops narrowing anything on a wide screen. + for (const [route, measure] of Object.entries(APPS_PAGE_MEASURES) as [string, AppsMeasure][]) { + const min = isAppsMeasureBand(measure) ? measure.min : measure; + const max = isAppsMeasureBand(measure) ? measure.max : measure; + expect(max, `${route} must actually narrow the body at its widest`).toBeLessThan(USABLE); + expect(min, `${route} must be a positive px value`).toBeGreaterThan(0); + expect(max, `${route}'s band must not be inverted`).toBeGreaterThanOrEqual(min); } }); }); +describe('🔴 a BAND grows only where the container grew', () => { + const bands = Object.entries(APPS_PAGE_MEASURES).filter( + (entry): entry is [string, AppsMeasureBand] => isAppsMeasureBand(entry[1]) + ); + + test('the sweep found bands to check (guards a vacuous loop)', () => { + // Every measure is a band today, and if that ever stops being true this loop would + // pass by checking nothing. + expect(bands.length).toBeGreaterThanOrEqual(7); + expect(new Set(bands.map(([, b]) => b)).size).toBe(2); + }); + + test('every band is still at its FLOOR at the old 1920 container', () => { + // The provenance rule this module has held since the container/measure split: a + // width pass may add width on screens that got wider, never re-decide what a 1440 or + // 1920 monitor already showed. `grow` is what makes that true, so it is asserted as + // arithmetic rather than described in the docstring. + expect(LEGACY_USABLE).toBe(1888); + for (const [route, band] of bands) { + expect(bandAt(band, LEGACY_USABLE), `${route} moved at the old container width`).toBe( + band.min + ); + } + }); + + test('…and every band reaches its CEILING inside the current one', () => { + // The other end: a band whose `grow` is too small never reaches its own max, so the + // ceiling would be a number that reads as a decision and does nothing. + expect(USABLE).toBe(2528); + for (const [route, band] of bands) { + expect(bandAt(band, USABLE), `${route} never reaches its ceiling`).toBe(band.max); + } + }); + + test('🔴 the ramp is a RAMP — each band is strictly between its ends somewhere', () => { + // Without this, `grow: 100` would satisfy both tests above while jumping every + // measured page straight to its ceiling the moment the container passed 1888. + for (const [route, band] of bands) { + const midpoint = bandAt(band, (LEGACY_USABLE + USABLE) / 2); + expect(midpoint, `${route} jumps rather than ramps`).toBeGreaterThan(band.min); + expect(midpoint, `${route} jumps rather than ramps`).toBeLessThan(band.max); + } + }); + + test('appsMeasureCss renders a band as a clamp and a number untouched', () => { + expect(appsMeasureCss(APPS_READABLE_MEASURE)).toBe('clamp(1068px, 55%, 1368px)'); + expect(appsMeasureCss(APPS_TWO_COLUMN_DETAIL_MEASURE)).toBe('clamp(1288px, 65%, 1600px)'); + // A number is passed through so Mantine can convert it to rem as it always has. + expect(appsMeasureCss(777)).toBe(777); + // POSITIVE CONTROL on the formatter: feed a band no real measure equals and watch + // every term move, so the two assertions above cannot be satisfied by a hardcode. + expect(appsMeasureCss({ min: 111, max: 222, grow: 33 })).toBe('clamp(111px, 33%, 222px)'); + }); + + test('isAppsMeasureBand separates the two shapes', () => { + expect(isAppsMeasureBand(APPS_READABLE_MEASURE)).toBe(true); + expect(isAppsMeasureBand(1068)).toBe(false); + }); +}); + +describe('🔴 the card-list column ladder steps exactly where the surplus appeared', () => { + /** The CSS `repeat(auto-fill, minmax(min, 1fr))` count, mirrored in JS. */ + const columnsAt = (w: number) => + Math.max( + 1, + Math.floor((w + APPS_CARD_LIST_GAP) / (APPS_CARD_LIST_MIN_COLUMN + APPS_CARD_LIST_GAP)) + ); + + test('one column through the OLD container, two in the current one', () => { + // The whole justification for `/apps/installed` becoming a grid: it must change + // nothing a 1440 or 1920 monitor showed, and spend the 640px the ultrawide pass + // added. A min column of 1200 is what puts the step between the two. + expect(columnsAt(1408)).toBe(1); // 1440 viewport + expect(columnsAt(LEGACY_USABLE)).toBe(1); // the old 1920 container + expect(columnsAt(USABLE)).toBe(2); // the current 2560 container + }); + + test('the step is not sitting on either fixture width', () => { + // A ladder whose rung lands ON a width the tests measure cannot detect an + // off-by-one. The second column arrives at 2416, which is 528 above the old + // container's content width and 112 below the current one. + expect(columnsAt(2415)).toBe(1); + expect(columnsAt(2416)).toBe(2); + expect(USABLE - 2416).toBeGreaterThan(64); + expect(2416 - LEGACY_USABLE).toBeGreaterThan(64); + }); +}); + describe('🔴 the measures preserve the OLD rendered content widths exactly', () => { /** * THE +32px TRAP, PINNED. Mantine's `Container` is border-box: `size={N}` renders @@ -493,20 +620,23 @@ describe('🔴 the measures preserve the OLD rendered content widths exactly', ( expect(APPS_CONTAINER_GUTTER).toBe(32); }); + // 🔴 THE PROVENANCE IS NOW THE BAND'S **FLOOR**, and that is the point of a floor: a + // band whose min drifted off the old container width would silently re-decide what a + // 1440 monitor shows, which is the confounded change the gutter arithmetic above + // exists to keep out of a width pass. test.each([ - ['narrow table', APPS_NARROW_TABLE_MEASURE, 1400], ['two-column detail', APPS_TWO_COLUMN_DETAIL_MEASURE, 1320], ['readable', APPS_READABLE_MEASURE, 1100], - ])('%s: measure = old container width − gutter', (_label, measure, oldContainerWidth) => { - expect(measure).toBe(oldContainerWidth - APPS_CONTAINER_GUTTER); + ])('%s: band floor = old container width − gutter', (_label, band, oldContainerWidth) => { + expect(band.min).toBe(oldContainerWidth - APPS_CONTAINER_GUTTER); }); - test('the two-column measure equals the MODEL DETAIL page content width', () => { + test('the two-column FLOOR equals the MODEL DETAIL page content width', () => { // Its documented justification is "the same width as the model detail page", // which renders `` — Mantine's `xl` is 1320 border-box, so // its CONTENT is 1288. Stating the claim in content terms is what makes it true. const MANTINE_XL_CONTAINER = 1320; - expect(APPS_TWO_COLUMN_DETAIL_MEASURE).toBe(MANTINE_XL_CONTAINER - APPS_CONTAINER_GUTTER); + expect(APPS_TWO_COLUMN_DETAIL_MEASURE.min).toBe(MANTINE_XL_CONTAINER - APPS_CONTAINER_GUTTER); }); }); @@ -585,7 +715,10 @@ describe('🔴 /apps/mine is wide enough for its table, as a RELATIONSHIP', () = // counterfactual so the previous test cannot pass for the wrong reason. expect(APPS_PAGE_MEASURES).not.toHaveProperty('/apps/mine'); expect(APPS_FULL_MEASURE_PAGES).toContain('/apps/mine'); - expect(APPS_READABLE_MEASURE - SUBMISSIONS_CONTAINER_CHROME).toBeLessThan( + // The readable band's CEILING is used, not its floor: the counterfactual has to be + // "even at its widest, that class would still clip this table", or a band that grew + // past the floor would make this pass for a reason that is no longer true. + expect(APPS_READABLE_MEASURE.max - SUBMISSIONS_CONTAINER_CHROME).toBeLessThan( SUBMISSIONS_TABLE_MIN_WIDTH ); }); diff --git a/src/components/Apps/__tests__/appsWideLayout.test.ts b/src/components/Apps/__tests__/appsWideLayout.test.ts new file mode 100644 index 0000000000..0e70c82acb --- /dev/null +++ b/src/components/Apps/__tests__/appsWideLayout.test.ts @@ -0,0 +1,725 @@ +import fs from 'fs'; +import path from 'path'; +import ts from 'typescript'; +import { describe, expect, test } from 'vitest'; +import { + APPS_ACTIVE_PREVIEWS_COLUMNS, + APPS_CARD_LIST_GAP, + APPS_CARD_LIST_MIN_COLUMN, + APPS_AGENT_REPORT_SCOPE_COLUMNS, + APPS_FULL_MEASURE_CONTENT_WIDTH, + APPS_LEGACY_CONTENT_WIDTH, + APPS_MINE_COLUMNS, + APPS_MOD_LISTINGS_COLUMNS, + APPS_REVENUE_COLUMNS, + APPS_REVIEW_QUEUE_COLUMNS, + APPS_TABLE_COLUMN_LEDGERS, + appsCardGridColumnsAt, + appsTableColumnProblems, + type AppsTableColumns, +} from '~/components/Apps/appsWideLayout'; +import * as LEDGER_EXPORTS from '~/components/Apps/appsWideLayout'; + +/** + * `appsWideLayout` — the RULE and the LEDGERS, in the blocking `unit` project. + * + * WHAT THIS FILE OWNS: the DATA (one primary per ledger, the percentages leave room for + * it, the card ladder steps where it should), the COVERAGE (every headed table under the + * apps components has a ledger or a verified exemption), the PLACEMENT (the colgroup is + * its table's first child) and the ledger↔table COLUMN COUNT. + * + * WHAT `AppsWideLayout.geometry.test.tsx` OWNS: the resolved WIDTHS. + * + * ⚠️ THAT SPLIT IS NOT THE ONE IT LOOKS LIKE, AND IT WAS MEASURED RATHER THAN ASSUMED. + * The ordering guard here was written believing a misplaced `` is ignored by the + * browser — i.e. that geometry would be the real catch. Two mutations settled it on + * 2026-09-04, and re-confirmed after the file grew. Moving the element after + * `` left EVERY geometry assertion + * GREEN and turned only this file red: React inserts nodes through the DOM API, so the HTML + * parser's table foster-parenting never runs and Chromium honours the columns wherever the + * element sits. DELETING the colgroup outright turns the geometry assertions red — the + * positive control proving that green was about placement, not about that tier being blind. + * So placement and column count are enforced HERE and only here. + */ + +const repoFile = (rel: string) => path.resolve(__dirname, '../../../..', rel); +const srcOf = (rel: string) => fs.readFileSync(repoFile(rel), 'utf8'); + +/** + * Every file under `src` that RENDERS ``, excluding tests. + * + * Used to verify the exemption allowlist below, so a "nothing renders this" claim is + * re-derived rather than believed. Text-scanned rather than parsed on purpose: this is a + * search for a live call site, and being over-inclusive (a commented-out render would + * count) fails CLOSED — it would refuse an exemption, never grant one. + */ +function renderSitesOf(component: string): string[] { + const hits: string[] = []; + const needle = new RegExp(`<${component}[\\s/>]`); + const walk = (dir: string) => { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + if (entry.name !== '__tests__' && entry.name !== 'node_modules') walk(full); + continue; + } + if (!entry.name.endsWith('.tsx') || entry.name.includes('.test.')) continue; + if (needle.test(fs.readFileSync(full, 'utf8'))) { + hits.push(path.relative(repoFile('.'), full).replace(/\\/g, '/')); + } + } + }; + walk(repoFile('src')); + return hits.sort(); +} + +/** + * The file with every BLOCK COMMENT removed. + * + * 🔴 A COMMENT IS NOT A RENDER, AND THIS FILE LEARNED IT THE EXPENSIVE WAY ROUND. An + * earlier ordering guard read `indexOf(' text.replace(/\/\*[\s\S]*?\*\//g, ''); +const codeOf = (rel: string) => stripBlockComments(srcOf(rel)); + +describe('appsTableColumnProblems — the rule, on inputs it must REJECT', () => { + // 🔴 THE VALIDATOR'S ONLY REAL INPUTS ALL PASS, so without these it could be gutted + // and the suite would stay green. Each row below is a ledger that must be reported. + const cases: { name: string; columns: AppsTableColumns; offends: boolean }[] = [ + { name: 'a valid ledger', columns: [10, null, 14, 16, 12], offends: false }, + { name: 'a two-column ledger', columns: [null, 30], offends: false }, + { name: 'NO primary — nothing absorbs the slack', columns: [10, 20, 30], offends: true }, + { name: 'TWO primaries — the slack splits', columns: [null, 20, null], offends: true }, + { name: 'a zero-width column', columns: [null, 0, 20], offends: true }, + { name: 'a negative width', columns: [null, -5, 20], offends: true }, + { name: 'a non-finite width', columns: [null, Number.NaN], offends: true }, + { name: 'the percentages claim exactly 100', columns: [null, 50, 50], offends: true }, + { name: 'the percentages claim more than 100', columns: [null, 60, 60], offends: true }, + { name: 'an empty ledger', columns: [], offends: true }, + ]; + + test.each(cases)('$name', ({ columns, offends }) => { + expect(appsTableColumnProblems('fixture', columns).length > 0).toBe(offends); + }); + + test('the table exercises both verdicts and no row is a duplicate', () => { + // Guard-the-guard: duplicated rows inflate the table without adding coverage, and a + // table of only-bad rows is satisfied by a validator that rejects everything. + const keys = cases.map((c) => JSON.stringify(c.columns)); + expect(new Set(keys).size).toBe(keys.length); + expect(cases.some((c) => c.offends)).toBe(true); + expect(cases.some((c) => !c.offends)).toBe(true); + }); + + test('the message names WHICH ledger and WHAT is wrong', () => { + // A guard whose message does not say what is wrong sends the next reader hunting. + const [msg] = appsTableColumnProblems('my apps', [10, 20, 30]); + expect(msg).toContain('my apps'); + expect(msg).toContain('exactly ONE'); + const [over] = appsTableColumnProblems('my apps', [null, 60, 60]); + expect(over).toContain('120%'); + // …and the two defects do not print the same sentence. + expect(over).not.toContain('exactly ONE'); + }); +}); + +describe('every shipped ledger is valid', () => { + test('the sweep found the ledgers (guards a vacuous loop)', () => { + // A loop over an empty record passes having checked nothing. + expect(Object.keys(APPS_TABLE_COLUMN_LEDGERS)).toHaveLength(8); + }); + + test('no ledger has a problem', () => { + const problems = Object.entries(APPS_TABLE_COLUMN_LEDGERS).flatMap(([label, columns]) => + appsTableColumnProblems(label, columns) + ); + expect(problems).toEqual([]); + }); + + test('each ledger has the length we DECIDED (a value pin, not a relationship)', () => { + // 🔴 THE TITLE USED TO SAY "the column COUNT ITS TABLE RENDERS" — a relationship the + // body never checked, because it compares `.length` against a literal and never opens + // a component. Measured: adding a `` without touching the ledger left this + // GREEN for every table, and green at BOTH tiers for two of them. The relationship is + // now checked against the parsed tables, further down; this stays as what it always + // was — a value pin, honestly labelled, so a silent re-tune is still visible. + expect(APPS_REVIEW_QUEUE_COLUMNS.withoutDeploy).toHaveLength(5); // Kind App Submitter date action + expect(APPS_REVIEW_QUEUE_COLUMNS.withDeploy).toHaveLength(6); // …plus Deploy + expect(APPS_MINE_COLUMNS).toHaveLength(4); // App Cover Status Updated + expect(APPS_MOD_LISTINGS_COLUMNS).toHaveLength(5); // App Owner Category Reviews actions + expect(APPS_REVENUE_COLUMNS.withApp).toHaveLength(7); // Date App Scope Buzz Gross Share Status + expect(APPS_REVENUE_COLUMNS.scoped).toHaveLength(6); // …minus App + expect(APPS_ACTIVE_PREVIEWS_COLUMNS).toHaveLength(5); // App Version State Age actions + expect(APPS_AGENT_REPORT_SCOPE_COLUMNS).toHaveLength(6); // Scope Used Justified Sensitive Evidence Notes + }); + + test('🔴 the two-shape ledgers differ by exactly one column', () => { + // Both pairs exist because ONE optional column exists. A pair that differed by two + // would mean a width-conditional column set had crept in, which is the thing the + // module's docstring forbids. + expect( + APPS_REVIEW_QUEUE_COLUMNS.withDeploy.length - APPS_REVIEW_QUEUE_COLUMNS.withoutDeploy.length + ).toBe(1); + expect(APPS_REVENUE_COLUMNS.withApp.length - APPS_REVENUE_COLUMNS.scoped.length).toBe(1); + }); + + test('🔴 the PRIMARY column is where the ledger says it is — for EVERY ledger', () => { + // 🔴 THIS PIN WENT FROM 6/6 TO 6/10 WHEN FOUR LEDGERS WERE ADDED, AND THE GAP IS WHAT + // LET TWO WRONG DECISIONS SHIP. Measured at that revision: moving the primary in ALL + // FOUR new ledgers left the unit tier 31/31 green, and geometry caught only + // `ActivePreviewsPanel`. So the choice this module calls "a decision with two cases, + // and getting it wrong makes the defect WORSE" was unguarded at both tiers for + // `OffsiteReportsQueue`, `AppActivityPanel` and `ReportTabs` — and it WAS wrong for the + // first two. Keyed off the ledger record so a new entry cannot be added without one. + const PRIMARY_AT: Record = { + 'review queue (pending/rejected)': 1, // App — slug + optional title + 'review queue (approved)': 1, // App + 'my apps': 0, // App — icon + name + slug + 'moderation listings': 0, // App — slug + kind/status chips + 'revenue (unscoped)': 1, // App — link to the per-app page + 'revenue (scoped)': 1, // Scope — there is no App column + 'active previews': 4, // actions — case (b), no cell can use the room + 'agent report scopes': 5, // Notes — uncapped reviewer prose + }; + // The map and the ledger record must name the SAME set, or a ledger added without a + // decision would simply be skipped. + expect(Object.keys(PRIMARY_AT).sort()).toEqual(Object.keys(APPS_TABLE_COLUMN_LEDGERS).sort()); + for (const [label, index] of Object.entries(PRIMARY_AT)) { + expect(APPS_TABLE_COLUMN_LEDGERS[label].indexOf(null), `${label}'s primary column`).toBe( + index + ); + } + }); + + test('🔴 the primary column is left a MEANINGFUL share, not a sliver', () => { + // `< 100%` alone is satisfied by a ledger claiming 99%. The point of the primary is + // that it takes the surplus, so it has to be the biggest single share at the current + // container width — asserted against the real content width rather than a ratio. + for (const [label, columns] of Object.entries(APPS_TABLE_COLUMN_LEDGERS)) { + const claimed = columns.reduce((sum, c) => sum + (c ?? 0), 0); + const primaryPct = 100 - claimed; + const widest = Math.max(...columns.map((c) => c ?? 0)); + expect(primaryPct, `${label}: the primary column's share`).toBeGreaterThan(widest); + } + }); +}); + +describe('🔴 every HEADED table under /apps is enumerated, not remembered', () => { + /** + * 🔴 THIS REPLACED A HAND-WRITTEN LIST OF FOUR FILES, AND THE REPLACEMENT IS THE FIX + * FOR A REAL DEFECT THIS PR SHIPPED. The first pass removed `/apps/review`'s 1368 body + * cap and gave ledgers to the two tables somebody had in mind. That route renders FOUR + * tables, and the cap was the only thing holding the other two down: measured on + * `ActivePreviewsPanel` at 1440 → 2560, the gap between a row's slug and its "Tear + * down" button grew 817.36 → 1381.23 — half the container delta, i.e. exactly the + * defect this module exists to remove, newly introduced by removing its workaround. + * + * A list of files nobody derives cannot notice a table it never mentioned. So the set + * is DERIVED by parsing every component under the two directories, and anything without + * a ledger has to be EXEMPTED BY NAME with a reason. + * + * ⚠️ WHAT THIS GUARD DOES NOT COVER. This list was written as though it were closed + * ("stated rather than implied") and it was not — an audit found two more escapes, which + * is exactly the shape of over-claim the guard itself exists to stop. It is a list of the + * ones known TODAY, not a proof of completeness: + * · a table with NO header row is out of scope (there is no column ledger to attach + * and no header cell to count against one) — `AppAnalyticsPanel`'s two key/value + * tables are the live examples; + * · it reads `src/components/Apps` and `src/components/AppBlocks` only, so a table + * defined outside those directories and rendered on an apps route is invisible; + * · the walk matches the literal tag `Table`, so a NEW file that imports it under + * another name (`import { Table as DataTable }`) is not seen; + * · `findThead` walks JSX DESCENDANTS, so a NEW file whose header row is extracted + * into a sibling component renders a `
` this walk does not classify as headed. + * + * The last two are escapes for a NEW file only: an existing table cannot use them, + * because the SITES ledger below is asserted as an exact list and reds on a SHRINK as + * well as a growth. Both are low-realism and neither is fixed here — widening the walk + * to resolve aliases and cross-component structure is a parser, and this repo's + * `--header-height` guard records five rounds in which each parser added to close a hole + * shipped a new false PASS. + */ + const SCAN_DIRS = ['src/components/Apps', 'src/components/AppBlocks']; + + /** Tags that are a header CELL. `SortableTh` is a real one, not a naming convention. */ + const HEADER_TAGS = new Set(['Table.Th', 'SortableTh']); + + /** + * Headed tables that legitimately carry no ledger, each with the reason. + * + * 🔴 VERIFIED, NOT TRUSTED — the same rule the chrome allowlist in + * `appsPageWidths.test.ts` holds itself to. Every entry here claims the component is + * not rendered on any route, and the test below re-derives that by searching the whole + * of `src` for a JSX render of it. Adding a name cannot silence the guard. + */ + /** + * Headed tables that legitimately carry no ledger, each with the reason AND the way that + * reason is re-derived. Two kinds, because two different things can make a ledger wrong: + * + * `unrendered` — nothing renders the component, so there is no surface to lay out. + * Verified by searching the whole of `src` for a JSX render of it. + * `no-surplus` — the table is rendered, but its natural layout already fills the + * container at the NARROW end, so every ledger either wraps a cell + * there or reproduces natural layout at the wide end. Verified by a + * named geometry arm that keeps measuring it. + * + * 🔴 VERIFIED, NOT TRUSTED — the same rule the chrome allowlist in + * `appsPageWidths.test.ts` holds itself to. An exemption keyed on a name is exactly where + * a live table would hide, so adding a name here cannot silence the guard: each kind has + * a check below that has to pass. + */ + const EXEMPT: Record< + string, + | { kind: 'unrendered'; component: string; why: string } + | { kind: 'no-surplus'; component: string; why: string; geometryArm: string } + > = { + 'src/components/Apps/OffsiteReviewQueue.tsx#0': { + kind: 'unrendered', + component: 'OffsiteReviewQueue', + why: 'dead — superseded by the unified queue; nothing renders it (the LIVE table in this file is OffsiteReportsQueue, which does carry a ledger)', + }, + 'src/components/Apps/MySubmissionsList.tsx#0': { + kind: 'unrendered', + component: 'MySubmissionsList', + why: 'dead — /apps/my-submissions merged into /apps/mine and 301s there', + }, + 'src/components/Apps/OffsiteSubmissionsList.tsx#0': { + kind: 'unrendered', + component: 'OffsiteSubmissionsList', + why: 'dead — same merge as MySubmissionsList', + }, + 'src/components/Apps/OffsiteReviewQueue.tsx#1': { + kind: 'no-surplus', + component: 'OffsiteReportsQueue', + why: + 'at 1200 its row wants App 240 + Reason 292 + Reporter 94 + Reported 133 + Status ' + + '86 + actions 414 = 1259px in 1168px of container, so something is under-served ' + + 'there whatever the split. Every candidate ledger was taller than natural at 1200 ' + + 'or clipped the lineClamp-ed details harder. See the note in appsWideLayout.tsx.', + geometryArm: 'the reports table is no worse than natural at every width', + }, + 'src/components/Apps/AppActivityPanel.tsx#0': { + kind: 'no-surplus', + component: 'AppActivityPanel', + why: + 'its max-content sum (~735px) is the container content width AT 768, so there is no ' + + 'surplus to place at the narrow end. Two ledgers shipped and both regressed row ' + + 'height (48.09 and 64.89 against a natural 36.19); the only configuration that ' + + 'holds one line everywhere reproduces natural layout at 2560 to within ~15px on ' + + 'three of five columns. See the note in appsWideLayout.tsx.', + geometryArm: 'the activity table renders ONE LINE per cell at every width', + }, + }; + + type TableSite = { + id: string; + file: string; + /** Is the FIRST renderable child an `AppsTableColgroup`? */ + colgroupIsFirstChild: boolean; + /** Is there an `AppsTableColgroup` among the table's children at all? */ + hasColgroup: boolean; + /** Ledger expressions named on the colgroup's `columns` prop. */ + ledgerRefs: string[]; + /** Header cells that always render. */ + headerCellsAlways: number; + /** …plus the ones inside a `{cond && …}` / ternary. */ + headerCellsTotal: number; + }; + + const isElement = (n: ts.Node): n is ts.JsxElement | ts.JsxSelfClosingElement => + ts.isJsxElement(n) || ts.isJsxSelfClosingElement(n); + const tagOf = (n: ts.JsxElement | ts.JsxSelfClosingElement, src: ts.SourceFile) => + (ts.isJsxElement(n) ? n.openingElement.tagName : n.tagName).getText(src); + + /** Every `
` element in `file` that has a header row, with what it carries. */ + function tableSites(file: string): TableSite[] { + const abs = repoFile(file); + const source = ts.createSourceFile( + abs, + fs.readFileSync(abs, 'utf8'), + ts.ScriptTarget.Latest, + true, + ts.ScriptKind.TSX + ); + const sites: TableSite[] = []; + let seen = 0; + + /** Count header cells under `node`, split by whether they are inside an expression. */ + const countHeaders = (node: ts.Node): { always: number; total: number } => { + let always = 0; + let total = 0; + const walkRow = (row: ts.JsxElement) => { + for (const child of row.children) { + if (isElement(child) && HEADER_TAGS.has(tagOf(child, source))) { + always += 1; + total += 1; + } else if (ts.isJsxExpression(child) && child.expression) { + // A conditional header cell — counted toward the TOTAL only. + const inner = (n: ts.Node): void => { + if (isElement(n) && HEADER_TAGS.has(tagOf(n, source))) total += 1; + ts.forEachChild(n, inner); + }; + inner(child.expression); + } + } + }; + const findRows = (n: ts.Node): void => { + if (ts.isJsxElement(n) && tagOf(n, source) === 'Table.Tr') walkRow(n); + ts.forEachChild(n, findRows); + }; + findRows(node); + return { always, total }; + }; + + const visit = (node: ts.Node): void => { + if (ts.isJsxElement(node) && tagOf(node, source) === 'Table') { + // A header row group is what makes a table in scope. + let thead: ts.Node | null = null; + const findThead = (n: ts.Node): void => { + if (thead) return; + if (ts.isJsxElement(n) && tagOf(n, source) === 'Table.Thead') thead = n; + else ts.forEachChild(n, findThead); + }; + node.children.forEach(findThead); + if (thead) { + const id = `${file}#${seen}`; + seen += 1; + // 🔴 THE FIRST *RENDERABLE* CHILD. JSX text is whitespace and a `{/* … */}` + // comment is a JsxExpression with no expression — neither renders, and treating + // either as "the first child" would fail every correctly-written call site, + // all of which explain themselves in a comment above the colgroup. + const renderable = node.children.filter( + (c) => isElement(c) || (ts.isJsxExpression(c) && !!c.expression) + ); + const first = renderable[0]; + const colgroups = node.children.filter( + (c): c is ts.JsxSelfClosingElement | ts.JsxElement => + isElement(c) && tagOf(c, source) === 'AppsTableColgroup' + ); + const attrs = colgroups[0] + ? ts.isJsxElement(colgroups[0]) + ? colgroups[0].openingElement.attributes + : colgroups[0].attributes + : null; + const columnsText = + attrs?.properties + .filter(ts.isJsxAttribute) + .find((a) => a.name.getText(source) === 'columns') + ?.initializer?.getText(source) ?? ''; + const { always, total } = countHeaders(thead); + sites.push({ + id, + file, + colgroupIsFirstChild: + !!first && isElement(first) && tagOf(first, source) === 'AppsTableColgroup', + hasColgroup: colgroups.length > 0, + ledgerRefs: [...new Set(columnsText.match(/APPS_[A-Z0-9_]+(?:\.\w+)?/g) ?? [])], + headerCellsAlways: always, + headerCellsTotal: total, + }); + } + } + ts.forEachChild(node, visit); + }; + visit(source); + return sites; + } + + /** Every `.tsx` under the scan dirs that is not itself a test. */ + function scanFiles(): string[] { + const out: string[] = []; + for (const dir of SCAN_DIRS) { + const abs = repoFile(dir); + const walk = (d: string) => { + for (const entry of fs.readdirSync(d, { withFileTypes: true })) { + const full = path.join(d, entry.name); + if (entry.isDirectory()) { + if (entry.name !== '__tests__') walk(full); + continue; + } + if (!entry.name.endsWith('.tsx') || entry.name.includes('.test.')) continue; + out.push(path.relative(repoFile('.'), full).replace(/\\/g, '/')); + } + }; + walk(abs); + } + return out.sort(); + } + + const SITES = scanFiles().flatMap((f) => tableSites(f)); + + test('🔴 the headed-table set is exactly this (fails when it GROWS or SHRINKS)', () => { + // A LEDGER, not a floor. A walk that found nothing makes every loop below pass having + // checked nothing — the failure the hand-written file list had, arrived at by a + // different route — and a floor cannot see a NEW table either, which is the whole + // reason this describe block exists. Adding a headed table to these directories is + // meant to fail here and be classified deliberately. + expect(scanFiles().length).toBeGreaterThan(40); + expect(SITES.map((s) => s.id)).toEqual([ + 'src/components/AppBlocks/RevenuePanel.tsx#0', + 'src/components/Apps/ActivePreviewsPanel.tsx#0', + 'src/components/Apps/AppActivityPanel.tsx#0', + 'src/components/Apps/AppListingsModerationTable.tsx#0', + 'src/components/Apps/MyAppsBody.tsx#0', + 'src/components/Apps/MySubmissionsList.tsx#0', + // 🔴 TWO ENTRIES FOR ONE FILE, and this is the row that proves the walk is + // per-TABLE rather than per-file: `#0` is the dead `OffsiteReviewQueue` and `#1` is + // the live `OffsiteReportsQueue`. A per-file guard cannot express "one of these + // needs a ledger and the other does not", and an `indexOf`-based one would have + // graded the second table against the first one's colgroup. + 'src/components/Apps/OffsiteReviewQueue.tsx#0', + 'src/components/Apps/OffsiteReviewQueue.tsx#1', + 'src/components/Apps/OffsiteSubmissionsList.tsx#0', + 'src/components/Apps/ReportTabs.tsx#0', + 'src/components/Apps/UnifiedReviewList.tsx#0', + ]); + }); + + test('🔴 every headed table carries a ledger, or is exempted by name', () => { + const offenders = SITES.filter((s) => !s.hasColgroup && !EXEMPT[s.id]).map( + (s) => `${s.id} (${s.headerCellsTotal} header cells, no )` + ); + expect( + offenders, + 'A table with a header row on an /apps surface has no column ledger. Give it one in ' + + '~/components/Apps/appsWideLayout, or add it to EXEMPT with the reason it needs none.' + ).toEqual([]); + }); + + test("🔴 the colgroup is the table's FIRST CHILD — not merely somewhere in the file", () => { + // 🔴 THIS REPLACED AN `indexOf` COMPARISON, WHICH WAS SPELLED RATHER THAN STRUCTURAL, + // in two ways that both pass it: hoisting the colgroup OUTSIDE the `
` leaves it + // earlier in the file than the head (green, and it bounds nothing), and `indexOf` + // reads the FIRST occurrence in the file, so a second table in the same file was + // graded against the first one's colgroup. Asking the AST which element is the table's + // own first child cannot be satisfied by either. + const offenders = SITES.filter((s) => s.hasColgroup && !s.colgroupIsFirstChild).map( + (s) => `${s.id} (renders a colgroup, but it is not the table's first child)` + ); + expect(offenders).toEqual([]); + }); + + test('🔴 every EXEMPT table exists, and its REASON is re-derived (not believed)', () => { + const names = Object.keys(EXEMPT); + expect(names.length, 'the exempt list is empty — nothing to verify').toBeGreaterThan(0); + const offenders: string[] = []; + let unrenderedChecked = 0; + let noSurplusChecked = 0; + for (const id of names) { + const entry = EXEMPT[id]; + expect( + SITES.some((s) => s.id === id), + `${id} is exempted but no such table exists` + ).toBe(true); + if (entry.kind === 'unrendered') { + unrenderedChecked += 1; + const hits = renderSitesOf(entry.component); + if (hits.length > 0) { + offenders.push( + `${entry.component} is exempted as UNRENDERED but is rendered by ${hits.join(', ')}` + ); + } + } else { + noSurplusChecked += 1; + // 🔴 A `no-surplus` TABLE **IS** RENDERED — that is the whole difference — so the + // render search would reject it. What has to hold instead is that the measurement + // the exemption rests on is still being TAKEN: a named geometry arm covering it. + // Without this the reason decays into prose the moment the table's content changes. + const geometry = srcOf('src/components/Apps/AppsWideLayout.geometry.test.tsx'); + if (!geometry.includes(entry.geometryArm)) { + offenders.push( + `${entry.component} is exempted as NO-SURPLUS but its geometry arm ` + + `"${entry.geometryArm}" is not in AppsWideLayout.geometry.test.tsx — the ` + + 'exemption would be an unmeasured claim' + ); + } + if (renderSitesOf(entry.component).length === 0) { + offenders.push( + `${entry.component} is exempted as NO-SURPLUS but nothing renders it — it is ` + + 'UNRENDERED, and the two kinds must not be conflated' + ); + } + } + } + expect(offenders).toEqual([]); + // Guard-the-guard: both branches must actually be exercised, or one of them is dead + // code that would wave through the case it exists for. + expect(unrenderedChecked, 'no `unrendered` exemption was checked').toBeGreaterThan(0); + expect(noSurplusChecked, 'no `no-surplus` exemption was checked').toBeGreaterThan(0); + }); + + test('🔴 POSITIVE CONTROL — the render search can find a component that IS rendered', () => { + // Without this, `renderSitesOf` returning `[]` for every exempt entry is + // indistinguishable from a search wired to nothing, and the allowlist would be + // verified by a function that always says yes. + expect(renderSitesOf('ActivePreviewsPanel').length).toBeGreaterThan(0); + expect(renderSitesOf('AppActivityPanel').length).toBeGreaterThan(0); + expect(renderSitesOf('NoSuchComponentAnywhere')).toEqual([]); + }); + + test('🔴 the ledger LENGTH matches the header cells the table actually renders', () => { + // 🔴 THE TITLE USED TO CLAIM THIS AND THE BODY ASSERTED `.length` AGAINST A LITERAL — + // a docstring naming a RELATIONSHIP over a body inspecting ONE SIDE. Measured then: + // adding a `` without touching the ledger left `AppListingsModerationTable` + // and `RevenuePanel` green at BOTH tiers, and `[null,5,5,5,5]` satisfied every test. + // Now the table is read. + // + // A table with a CONDITIONAL header cell has two shapes, so it names two ledgers: the + // shortest must equal the always-rendered count and the longest the total. + const offenders: string[] = []; + let checked = 0; + for (const site of SITES) { + if (!site.hasColgroup) continue; + const lengths = site.ledgerRefs.map((ref) => { + const [name, key] = ref.split('.'); + const root = (LEDGER_EXPORTS as Record)[name]; + const value = key ? (root as Record)?.[key] : root; + return Array.isArray(value) ? value.length : NaN; + }); + if (lengths.length === 0 || lengths.some((n) => Number.isNaN(n))) { + offenders.push( + `${site.id}: could not resolve its ledger(s) [${site.ledgerRefs.join(', ')}]` + ); + continue; + } + checked += 1; + const min = Math.min(...lengths); + const max = Math.max(...lengths); + if (min !== site.headerCellsAlways) { + offenders.push( + `${site.id}: shortest ledger is ${min} but the table always renders ` + + `${site.headerCellsAlways} header cell(s) [${site.ledgerRefs.join(', ')}]` + ); + } + if (max !== site.headerCellsTotal) { + offenders.push( + `${site.id}: longest ledger is ${max} but the table renders at most ` + + `${site.headerCellsTotal} header cell(s) [${site.ledgerRefs.join(', ')}]` + ); + } + } + expect(offenders).toEqual([]); + // Guard-the-guard: an empty offender list is indistinguishable from a loop that + // resolved no ledgers at all. + // Six ledgered tables today — `AppActivityPanel` and `OffsiteReportsQueue` gave theirs + // up as `no-surplus` exemptions, so this floor moved DOWN deliberately rather than + // drifting. It exists so an empty offender list cannot mean "resolved nothing". + expect(checked, 'no table had its ledger resolved').toBeGreaterThanOrEqual(6); + }); + + test('🔴 the header-cell counter can SEE a conditional column (negative control)', () => { + // The two-shape tables are the only ones where `always` and `total` differ, so if the + // conditional branch of the counter were dead every assertion above would still pass. + const twoShaped = SITES.filter((s) => s.headerCellsTotal > s.headerCellsAlways); + expect(twoShaped.map((s) => s.id).sort()).toEqual([ + 'src/components/AppBlocks/RevenuePanel.tsx#0', + 'src/components/Apps/UnifiedReviewList.tsx#0', + ]); + for (const s of twoShaped) expect(s.ledgerRefs.length).toBe(2); + }); + + test('🔴 /apps/installed uses the card GRID *on the installed-apps list*, and no longer caps its body', () => { + // 🔴 THE testId IS LOAD-BEARING, NOT DECORATION. This asserted `/` whose PRIMARY column takes the slack, and `/apps/installed`'s card list + * steps to a second grid column. Both mechanisms live in + * `~/components/Apps/appsWideLayout` — read {@link APPS_CARD_LIST_MIN_COLUMN} for the + * measured 640px dead-gap defect the card half fixes. + * + * 🔴 `/apps/review` IS IN THAT LIST NOW, AND ITS 1368 CAP IS DELETED. It was the one + * route whose measure existed to work around this: four short columns could not spend + * the container, so the table distributed the surplus as padding and the Review button + * receded from the row it acts on. Capping the page refused the width to avoid + * mis-spending it; the columns are proportional now, so the workaround would only be + * hiding the fix. There is no `APPS_NARROW_TABLE_MEASURE` any more — do not + * reintroduce one for a table that reads too wide; give it a ``. * * 🔴 IT IS ALSO THE CHROME'S WIDTH, on every route, which is the point of this * module. Do not reintroduce a per-page `Container size=` — `AppsPageLayout` no @@ -100,44 +229,30 @@ export const APPS_CONTAINER_GUTTER = 32; export const APPS_PAGE_CONTAINER_WIDTH = 2560; /** - * The READABLE measure — single-column form/detail surfaces where line length, not - * available space, is the constraint. A submit wizard or a listing editor stretched - * to the full container (2528px of content today, less any reserved scrollbar) puts - * prose and form rows on an + * The READABLE measure — single-column form/prose surfaces where line length, not + * available space, is the constraint. A submit wizard or a listing editor stretched to + * the full container (2528px of content today) puts prose and form rows on an * unreadable measure. * - * `1068 = 1100 − 32`: the content width these pages rendered when they passed + * A BAND since the ultrawide pass, not the fixed 1068 it was: + * + * `min: 1068 = 1100 − 32` — the content width these pages rendered when they passed * `size={1100}`. 1100 was chosen as wider than the `sm` (620) / `md` (800) / `lg` * (990) tokens these pages used before it — the forms have two-column rows and media - * grids that were cramped at 620 — while staying inside a comfortable measure. + * grids that were cramped at 620 — while staying inside a comfortable measure. It is + * the FLOOR rather than the width, so nothing a 1440 or 1920 monitor shows moves. + * + * `max: 1368` — the number the deleted narrow-table class carried, and it is reused + * rather than invented for the reason that class recorded: 1400 (1368 of content) was + * "wider than the readable/form width while stopping short of the width where the + * row's two ends stop reading as one row". That judgement is about a two-ended ROW, + * which is exactly what these pages' form rows are. + * + * `grow: 55` — see {@link AppsMeasureBand}. 55% of 1888 (the old container's content) + * is 1038 ≤ 1068, so the floor still wins there; 55% of 2528 is 1390 ≥ 1368, so the + * ceiling is reached inside the current container. Both are asserted arithmetically. */ -export const APPS_READABLE_MEASURE = 1068; - -/** - * The NARROW-TABLE measure — a table surface with few, short columns, where the full - * container is not "full width" but "stretched". - * - * `/apps/review` is the case this exists for. It renders FOUR narrow columns (Kind / - * App / Submitter / Submitted) plus a Review button. At the full container width the - * columns cannot spend the space, so the table distributes it as padding: measured at - * the then-1888px content width, Submitter grew to ~380px to hold a short username and - * a large dead gap opened between the last column and the Review button, which is the - * action the moderator is actually aiming at. Raising the container to 2560 (2528 of - * content, less any reserved scrollbar) makes that worse, not better — which is why this class exists rather than - * tracking the container. - * - * `1368 = 1400 − 32`: the content width the page rendered at `size={1400}`. 1400 was - * picked over 1200 to keep the page wider than the readable/form width while stopping - * short of the width where the row's two ends stop reading as one row. - * - * 🔴 A DISTINCT CLASS, deliberately — not a one-off number. The module's rule is "a - * page joins a class, or the class list grows on purpose"; the guard in - * `__tests__/appsPageWidths.test.ts` enumerates the classes as literals, so adding a - * fourth bespoke measure still fails there first. `/apps/mine` is NOT moved here: its - * table has a measured 1424px scroll floor, so the full container is load-bearing for - * it in a way it is not for `/apps/review`. - */ -export const APPS_NARROW_TABLE_MEASURE = 1368; +export const APPS_READABLE_MEASURE: AppsMeasureBand = { min: 1068, max: 1368, grow: 55 }; /** * The TWO-COLUMN DETAIL measure — a detail page laid out as a main column plus a @@ -146,23 +261,33 @@ export const APPS_NARROW_TABLE_MEASURE = 1368; * `/apps/store-preview/[slug]` is the case this exists for. It is a deliberate port * of the MODEL DETAIL page's layout (`` in * `src/pages/models/[id]/[[...slug]].tsx`) — the same `ContainerGrid2` with the same - * `{ base: 12, sm: 7, md: 8 }` / `{ base: 12, sm: 5, md: 4 }` spans — so it takes - * that page's content measure rather than a number of its own. + * `{ base: 12, sm: 7, md: 8 }` / `{ base: 12, sm: 5, md: 4 }` spans. * - * `1288 = 1320 − 32`, and Mantine's `xl` container is 1320 border-box, so 1288 is + * `min: 1288 = 1320 − 32`, and Mantine's `xl` container is 1320 border-box, so 1288 is * EXACTLY what the model detail page renders its content at. Stating the equivalence * in content terms is what makes it true: `maw={1320}` here would have been 32px - * wider than the page it claims to match. + * wider than the page it claims to match. Why not the READABLE floor it used to be: at + * 1068 the `md` split gives a ~340px right rail, narrower than the creator card + + * action card want, and the page reads as a squeezed single column with a sliver + * beside it. * - * Why not the READABLE measure it used to be: at 1068 the `md` split gives a ~340px - * right rail, narrower than the creator card + action card want, and the page reads as - * a squeezed single column with a sliver beside it. Why not the full container: the - * left column is prose (a `CustomMarkdown` description), and 8/12 of the container's - * content width is a ~1685px measure at today's 2560 (it was ~1250px at 1920) — the - * exact thing {@link APPS_READABLE_MEASURE} exists to avoid, and the container getting - * wider only widens the gap. At 1288 the left column is ~825px and the rail ~410px. + * `max: 1600` — DERIVED, not chosen. The ceiling on this page is the LEFT column: it + * is prose (a `CustomMarkdown` description) at the `md` 8/12 span, and the readable + * band's own floor is 1068. `8/12 × 1600 = 1066.67 ≤ 1068`, so 1600 is the widest this + * page can be while its markdown column stays inside the measure the readable class + * exists to hold. The rail grows from ~410px to ~533px across the band, which is the + * half the ultrawide pass was asked for. Letting it track the full container instead + * would put the description on a ~1685px measure — the exact thing + * {@link APPS_READABLE_MEASURE} exists to avoid. + * + * `grow: 65` — 65% of 1888 is 1227 ≤ 1288 (floor holds at the old container) and 65% + * of 2528 is 1643 ≥ 1600 (ceiling reached inside the current one). */ -export const APPS_TWO_COLUMN_DETAIL_MEASURE = 1288; +export const APPS_TWO_COLUMN_DETAIL_MEASURE: AppsMeasureBand = { + min: 1288, + max: 1600, + grow: 65, +}; /** * CONTENT MEASURE per `/apps/*` route, keyed by the NEXT ROUTE PATHNAME (the @@ -197,13 +322,6 @@ export const APPS_TWO_COLUMN_DETAIL_MEASURE = 1288; * correctly. */ export const APPS_PAGE_MEASURES = { - /** - * NARROW TABLE, not full width. Four short columns (Kind / App / Submitter / - * Submitted) cannot spend the container — see {@link APPS_NARROW_TABLE_MEASURE}. - * The DETAIL route takes no measure at all: it renders side-by-side diff panels + - * a live preview, which do use the space. - */ - '/apps/review': APPS_NARROW_TABLE_MEASURE, /** * TWO-COLUMN DETAIL, not readable-single-column — the model-detail-page layout * (main column + right rail), so it takes that page's content measure. @@ -234,7 +352,7 @@ export const APPS_PAGE_MEASURES = { '/apps/[appBlockId]/revenue': APPS_READABLE_MEASURE, /** The developer get-started explainer — prose. */ '/apps/get-started': APPS_READABLE_MEASURE, -} as const satisfies Record; +} as const satisfies Record; export type AppsMeasuredRoute = keyof typeof APPS_PAGE_MEASURES; @@ -250,12 +368,18 @@ export type AppsMeasuredRoute = keyof typeof APPS_PAGE_MEASURES; * 1424px. Giving it the readable measure would re-create the exact clip the wide * width was introduced to fix; `__tests__/appsPageWidths.test.ts` pins the container * against that floor. + * + * 🔴 `/apps/review` JOINED THIS LIST when its 1368 cap was deleted — see the note on + * {@link APPS_PAGE_CONTAINER_WIDTH}. Taking the full container is only correct for it + * because its queue table now carries `APPS_REVIEW_QUEUE_COLUMNS`; the two changes are + * one decision and reverting either alone re-opens the dead-gap defect. */ export const APPS_FULL_MEASURE_PAGES = [ '/apps', '/apps/installed', '/apps/mine', '/apps/revenue', + '/apps/review', '/apps/review/[publishRequestId]', ] as const; diff --git a/src/components/Apps/appsWideLayout.tsx b/src/components/Apps/appsWideLayout.tsx new file mode 100644 index 0000000000..1822eae691 --- /dev/null +++ b/src/components/Apps/appsWideLayout.tsx @@ -0,0 +1,450 @@ +import type { ReactNode } from 'react'; +import { + APPS_CARD_LIST_GAP, + APPS_CARD_LIST_MIN_COLUMN, + APPS_CONTAINER_GUTTER, + APPS_LEGACY_CONTAINER_WIDTH, + APPS_PAGE_CONTAINER_WIDTH, +} from '~/components/Apps/appsPageWidths'; + +/** + * HOW A `/apps/*` SURFACE SPENDS SURPLUS CONTAINER WIDTH. + * + * `~/components/Apps/appsPageWidths` decides how wide a route's body is allowed to be. + * This module decides what the body DOES with it, and it exists because the two are + * different questions that were answered as one: the ultrawide pass raised the shared + * container 1920 → 2560, and the routes that take no measure went from 1888 to 2528 of + * content without a single column getting wider. + * + * 🔴 THE DEFECT IS A GAP, NOT A CLIP. Nothing was cut off — a table's columns simply + * stayed at their content width and the table distributed the extra 640px as PADDING, + * which on a `space-between` row lands entirely between a row's content and the control + * that acts on it. Measured on `/apps/installed`, where THREE + * `Group justify="space-between" wrap="nowrap"` rows (in `PinnedInstallRow`, + * `InstalledAppCard` and `HiddenBlocksPanel`) each moved their button 640px further from + * the name it belongs to. `/apps/review` had the same shape and had been "fixed" by + * CAPPING THE PAGE at 1368 — a workaround this module replaces, so that cap is deleted. + * + * TWO MECHANISMS, one per surface shape: + * + * · A TABLE takes {@link AppsTableColgroup}. Every column except the PRIMARY one gets + * a percentage width; the primary gets none, so CSS's automatic table layout hands + * it everything left over. That is why there is exactly one `null` in each ledger + * below — and WHICH column gets it is a decision, not a default; see the two cases. + * · A CARD LIST takes {@link AppsCardGrid}. Cards go SIDE BY SIDE once the container + * is wide enough for two of {@link APPS_CARD_LIST_MIN_COLUMN}, so the surplus buys + * a column instead of stretching one card across the screen. + * + * 🔴 NEITHER MECHANISM MAY BE REPLACED BY A BODY CAP. Refusing the width is what the + * container pass exists to stop doing, and a cap is invisible to every guard here — + * see `__tests__/appsPageWidths.test.ts` for the taxonomy pins that keep these routes + * in `APPS_FULL_MEASURE_PAGES`. + * + * 🔴 WHICH COLUMN IS PRIMARY IS A DECISION WITH TWO CASES, AND GETTING IT WRONG MAKES THE + * DEFECT WORSE RATHER THAN BETTER. The slack has to land somewhere; "the primary column + * absorbs it" only helps when that column can USE it. + * + * (a) A column with genuinely VARIABLE, long content — an app name, a free-text reason, + * an event detail — is primary. The slack becomes headroom for real content that + * would otherwise truncate. + * (b) A table where NO column has that — every cell a slug, a badge, a date, or a + * hard-CAPPED text — takes its LAST column as primary instead, so the slack TRAILS + * the row rather than splitting it. That column is usually the action column, and + * the reason it works is that a cell's content is left-aligned: the control sits at + * its LEFT edge, so the slack lands to the RIGHT of the button. ⚠️ THAT ONLY HOLDS + * IF THE CELL REALLY IS LEFT-ALIGNED — `OffsiteReportsQueue`'s action `Group` was + * `justify="flex-end"`, which pins the buttons to the table's right edge and turns + * case (b) into the defect. It is `flex-start` now; that is a no-op at every width + * where the column sits at min-content, i.e. everywhere it is not the primary. + * + * 🔴 "VARIABLE" MEANS THE CELL CAN ACTUALLY USE THE PIXELS — measure it, do not read the + * field name. Two ledgers were wrong on exactly this and both were caught by rendering: + * · `OffsiteReportsQueue`'s `Reason` looked like free text and is + * `lineClamp={2} style={{ maxWidth: 260 }}`. Measured 1440 → 2560 with it primary: + * the column went 587.73 → 1404.64 (+816.91) while the details box stayed **260 at + * both** — ~1145px of dead space inside the cell, i.e. padding relabelled. + * · `AppActivityPanel`'s `Detail` looked like the payload and is a fixed monospace ref. + * With it primary the column went 971.56 → 1744.34 (+772.78) while its glyph box + * stayed **151.72 at both**. The component's own comment said so; the ledger did not. + * + * `ActivePreviewsPanel` is why this is written down. Its first ledger made `App` primary + * on the general rule — and `App` is a short `{slug}`, so 49% of the table + * became dead space sitting *between* the slug and the "Tear down" button. Modelled + * against the measured no-ledger baseline that is a WIDER gap than doing nothing at all. + * Case (b) puts the same slack past the button, where nothing has to be scanned across it. + * + * 🔴 AND THE FIXED SHARES ARE SIZED TO CONTENT, NOT SPREAD TO FILL. A non-primary + * percentage that exceeds what its cell needs at the wide width is padding again, just + * relabelled — it re-creates a slice of the defect between every pair of columns. Each + * number below is roughly the cell's own width at the CURRENT container, which is small; + * at narrow widths min-content wins anyway (see the next paragraph), so the visible effect + * on a 1440 monitor is nil. + * + * ⚠️ A PERCENTAGE IS A PREFERENCE, NOT A FLOOR-AND-CEILING. Under automatic table layout + * a column is never squeezed below its MIN-CONTENT width, so on a narrow viewport a + * column whose percentage is smaller than its content simply takes what it needs and the + * primary gives way. That is the correct behaviour — these ledgers exist to place the + * SURPLUS on a wide screen, not to compress anything on a narrow one — but it does mean + * the percentages are only observably in force once the table is wider than the sum of + * its columns' min-content widths. + * + * 🔴 NO WIDTH-CONDITIONAL COLUMN SET. A ledger is a list of WIDTHS for a fixed set of + * columns; a column that exists only on wide screens would be a discoverability and an + * a11y problem (screen readers and narrow viewports would see a different table) and it + * would double the test surface. Where a table legitimately has two SHAPES — the review + * queue's Deploy column, the revenue table's App column — both shapes are enumerated + * here as their own ledger and the choice is made by DATA, never by width. + */ + +/** + * One table's column widths, in document order. + * + * A `number` is a percentage of the table's width. `null` marks the PRIMARY column, + * which is deliberately given no width at all so it absorbs whatever the percentages + * leave — the whole point of the ledger. + */ +export type AppsTableColumns = readonly (number | null)[]; + +/** + * Everything wrong with a ledger, as messages. Empty means valid. + * + * 🔴 A FUNCTION RATHER THAN AN INLINE ASSERTION, so the RULE has a test of its own on + * inputs that must be rejected. A validator whose only inputs are the four real ledgers + * — all of which pass — is a validator nobody has watched work. + */ +export function appsTableColumnProblems(label: string, columns: AppsTableColumns): string[] { + const problems: string[] = []; + if (columns.length === 0) { + return [`${label}: an empty ledger describes no table`]; + } + const primaries = columns.filter((c) => c === null).length; + if (primaries !== 1) { + problems.push( + `${label}: exactly ONE column must be the primary (null) so it can absorb the ` + + `slack — found ${primaries}` + ); + } + for (const [i, c] of columns.entries()) { + if (c === null) continue; + if (!Number.isFinite(c) || c <= 0) { + problems.push(`${label}: column ${i} must be a positive percentage, got ${c}`); + } + } + const total = columns.reduce((sum, c) => sum + (c ?? 0), 0); + // Strictly less than 100: at exactly 100 the primary column would be handed nothing, + // which is a ledger that has stopped doing the one thing it exists for. + if (total >= 100) { + problems.push( + `${label}: the non-primary columns claim ${total}% — they must leave the primary ` + + `column a share of its own (< 100%)` + ); + } + return problems; +} + +/** + * The `/apps/review` QUEUE table (`UnifiedReviewList`) — Kind · **App** · Submitter · + * date · [Deploy] · action. + * + * The App column is primary: it is the only cell carrying a variable-length identity + * (a slug plus an optional title), and it is what a moderator scans down. Everything + * else is a badge, a username, a formatted date or a button — all of which have a + * natural width that more space does not improve. + * + * The Deploy column exists on the Approved tab only, so BOTH shapes are enumerated + * rather than one being patched at the call site. Its presence is decided by whether a + * retrigger handler was supplied — i.e. by data, never by width. + */ +export const APPS_REVIEW_QUEUE_COLUMNS = { + /** Pending / Rejected: Kind · App · Submitter · date · action. */ + withoutDeploy: [6, null, 6, 9, 6] as AppsTableColumns, + /** Approved: Kind · App · Submitter · date · Deploy · action. */ + withDeploy: [5, null, 5, 8, 8, 5] as AppsTableColumns, +} as const; + +/** + * The `/apps/mine` table (`MyAppsBody`) — **App** · Cover · Status · Updated. + * + * App is primary for the same reason as above, and here it also carries the icon and + * the slug, so it is the cell that most wants the room. Cover is a fixed 96px image, so + * its 12% is a floor rather than an aspiration; Status holds up to three badges plus + * the completeness advisory, which is why it is the widest fixed share. + */ +export const APPS_MINE_COLUMNS: AppsTableColumns = [null, 5, 10, 5]; + +/** + * The `/apps/review` MANAGE-LISTINGS table (`AppListingsModerationTable`) — + * **App** · Owner · Category · Reviews · actions. + * + * The action cell is a `Group` of buttons plus a menu, so it gets the second-largest + * share; it still cannot be primary, because its natural width is set by its buttons + * and handing it the slack would push the buttons away from the row again. + */ +export const APPS_MOD_LISTINGS_COLUMNS: AppsTableColumns = [null, 5, 5, 4, 13]; + +/** + * The revenue attributions table (`RevenuePanel`) — Date · [App] · **Scope** · Buzz · + * Gross · Your share · Status. + * + * TWO SHAPES, and the primary column MOVES between them, which is why they are two + * ledgers rather than one with a hole: + * · unscoped (`/apps/revenue`) — App is a link to a per-app page and is primary; + * · scoped (`/apps//revenue`) — there IS no App column, because every row is the + * same app, so Scope takes the slack. + * Both are numbers-only decisions about a fixed column set; nothing here is width-aware. + */ +export const APPS_REVENUE_COLUMNS = { + /** `/apps/revenue`: Date · App · Scope · Buzz · Gross · Your share · Status. */ + withApp: [5, null, 7, 5, 5, 6, 7] as AppsTableColumns, + /** `/apps//revenue`: Date · Scope · Buzz · Gross · Your share · Status. */ + scoped: [6, null, 6, 6, 7, 8] as AppsTableColumns, +} as const; + +/** + * The `/apps/review` ACTIVE-PREVIEWS panel (`ActivePreviewsPanel`) — + * **App** · Version · State · Age · actions. + * + * 🔴 THIS LEDGER IS THE ONE THAT PROVES THE POINT OF ENUMERATING THEM. `/apps/review` + * renders FOUR tables, and the first pass gave ledgers to two of them while removing the + * 1368 cap that had been holding the other two down. Measured on the real panel in the + * real layout, 1440 → 2560, with no ledger: + * + * columns 228.02 | 165.17 | 146.45 | 152.05 | 682.31 (1440) + * 413.89 | 299.83 | 265.84 | 276.00 | 1238.44 (2560) + * slug → "Tear down" gap 817.36 → 1381.23 (+563.87) + * + * ⚠️ THAT PAIR WAS FIRST RECORDED AS `609.67 → 1173.55`, WHICH IS A DIFFERENT QUANTITY. + * The two differ by which endpoint the gap is measured to: 609.67/1173.55 is the `` + * BORDER BOX to the row's FIRST control (the "Open full-page preview" anchor); the shipped + * helper measures the slug's GLYPH RANGE to the "Tear down" BUTTON. The DELTA is +563.87 + * either way, so nothing about the argument moved — but a live assertion message that + * quoted one pair while printing the other reads as a broken harness, so the numbers here + * are the ones the guard actually produces. + * + * i.e. half the container's 1120px delta landed between a row's identity and the control + * acting on it — the exact phenomenon the header of this file calls THE DEFECT, on the + * route whose cap this change removes. Uncapping a page is a claim about EVERY table on + * it, which is why `__tests__/appsWideLayout.test.ts` now enumerates them instead of + * naming the ones somebody remembered. + * + * 🔴 THE PRIMARY IS THE **ACTION** COLUMN — case (b) at the top of this file, and this + * table is the worked example. Every data cell here is short and fixed in kind: a slug, a + * version, a state badge, a relative age. Making `App` primary on the general rule put the + * slack *inside* the column the reader has to scan across to reach the button, which is + * the defect with extra steps. With the action column primary, the surplus lands past the + * buttons. + * + * 🔴 AND THE FOUR SHARES ARE DELIBERATELY TOO SMALL TO BIND — this is the shrink-to-content + * idiom, not a proportion. A percentage is a preference floored at min-content, so a share + * smaller than the cell needs resolves to the cell's own width at EVERY container width, + * which is what makes those four columns CONSTANT. It has to be spelled this way here: + * ordinary content-sized shares (9/7/6/6) still grow with the table, and measured on this + * fixture that alone moved the slug → "Tear down" gap 510.38 → 823.95. The same numbers + * with these shares hold it flat. + */ +export const APPS_ACTIVE_PREVIEWS_COLUMNS: AppsTableColumns = [3, 2, 2, 2, null]; + +/** + * 🔴 THERE IS NO `APPS_OFFSITE_REPORTS_COLUMNS` EITHER, for the same measured reason as the + * activity table above — and this one took three wrong ledgers to establish. + * + * Round 2 made `App` primary, round 3 made `Reason` primary (it is + * `lineClamp={2} maxWidth: 260`, so it absorbed +816.91 for nothing), and round 4 made the + * actions column primary under case (b). The last one is right in KIND and still + * unshippable, because at 1200 this row's content wants + * + * App 240 + Reason 292 + Reporter 94 + Reported 133 + Status 86 + actions 414 = 1259px + * + * in 1168px of container. Something is under-served at 1200 whatever the split, and the + * browser's own layout picks the least-bad one. Measured: every candidate was either + * TALLER than natural at 1200 (105.48 / 177.88 against 88.69) or clipped the lineClamp-ed + * details harder than natural (a 150.77px details box against 260) — and the second is + * invisible to a row-height check, which is why the tier now A/Bs WIDTH as well as height. + * + * 🔴 THE `justify="flex-end"` ON ITS ACTION GROUP IS THEREFORE BACK. That flip was correct + * ONLY as part of case (b): with no ledger the column sits at its min-content and the two + * alignments render identically, so the flip would have been an unjustified change. + */ + +/** + * 🔴 THERE IS NO `APPS_ACTIVITY_COLUMNS`, AND THAT IS A MEASURED DECISION. + * + * `/apps/installed`'s activity tab (`AppActivityPanel`) carried one for two rounds and it + * was wrong both times — first with `Detail` primary (a fixed monospace ref), then with + * shares set from a 1408 measurement, which squeezed three columns below their content at + * every width a real desktop uses. Measured on a rich `tip` row, ROW HEIGHT: + * + * 768 1200 1440 2560 + * no ledger (natural) 36.19 36.19 36.19 36.19 + * [7, 8, 10, null, 6] (round 2) 48.09 48.09 48.09 36.19 + * [3, 4, 20, 13, null] (round 3) 64.89 64.89 64.89 48.09 + * + * Round 3 was 79% taller than natural at 1440 — `When` broke a `YYYY-MM-DD HH:mm` stamp + * across THREE lines, and `App` sat pinned at its 108.52 min-content from 768 through 2560 + * so a long name was ellipsised identically on both. + * + * 🔴 AND NO LEDGER FIXES IT, WHICH IS THE POINT. This table's max-content sum (~735px) is + * the container's content width AT 768, so there is no surplus to place at the narrow end. + * Three candidates sized from 1200 all still wrapped at 768 (48.09). The one configuration + * that holds a single line everywhere — `[16, 12, 27, 25, null]` — reproduces natural + * layout at the wide end to within ~15px on three of five columns: + * + * ledger @2560 404.47 | 303.36 | 682.55 | 632.00 | 505.63 + * natural @2560 411.09 | 609.14 | 667.58 | 615.19 | 225.00 + * + * So the choice is between a ledger that wraps text on a laptop and a ledger that is + * natural layout with extra steps. Both are worse than none, and this module's own rule — + * a share larger than its cell needs is padding relabelled — rules out the second. The + * table is EXEMPT; the guard in `__tests__/appsWideLayout.test.ts` records that as a + * `no-surplus` exemption and requires a geometry arm to keep proving it. + */ + +/** + * The agent code-review report's scope table (`ReportTabs`) — + * Scope · Used · Justified · Sensitive · Evidence · **Notes**. + * + * 🔴 NOTES IS PRIMARY. `Scope` is a fixed-shape identifier (`models:read:self`); `Used`, + * `Justified` and `Sensitive` are booleans. The two free-text columns are `Evidence` and + * `Notes`, and only one can be primary — `Evidence` therefore takes the largest FIXED + * share and `Notes`, which is the reviewer's own prose and the least bounded of the two, + * takes the slack. + * + * Reachable on `/apps/review/[publishRequestId]` (via `OnsiteReviewModalBody` → + * `AgentReviewPanel`), which takes the full container — so it is in scope even though it + * is flag-gated (`appBlocksAgenticReview`) and also renders inside a modal elsewhere. A + * ledger is inert in the modal (the table is narrower than the sum of its min-contents + * there) and load-bearing on the page, which is the right way round. + */ +export const APPS_AGENT_REPORT_SCOPE_COLUMNS: AppsTableColumns = [7, 5, 6, 6, 22, null]; + +/** Every ledger in this module, so a guard can sweep them all rather than a sample. */ +export const APPS_TABLE_COLUMN_LEDGERS: Readonly> = { + 'review queue (pending/rejected)': APPS_REVIEW_QUEUE_COLUMNS.withoutDeploy, + 'review queue (approved)': APPS_REVIEW_QUEUE_COLUMNS.withDeploy, + 'my apps': APPS_MINE_COLUMNS, + 'moderation listings': APPS_MOD_LISTINGS_COLUMNS, + 'revenue (unscoped)': APPS_REVENUE_COLUMNS.withApp, + 'revenue (scoped)': APPS_REVENUE_COLUMNS.scoped, + 'active previews': APPS_ACTIVE_PREVIEWS_COLUMNS, + 'agent report scopes': APPS_AGENT_REPORT_SCOPE_COLUMNS, +}; + +/** + * The proportional `` for a `/apps/*` table. + * + * 🔴 IT MUST BE THE TABLE'S FIRST CHILD, before `` — HTML puts `` + * ahead of any row group, and only a document that says so is valid. + * + * ⚠️ AND THE OBVIOUS JUSTIFICATION FOR THAT RULE IS WRONG HERE, SO DO NOT REPEAT IT. + * "A browser ignores a misplaced ``" is what this comment used to say, and it + * was refuted by mutating it: measured 2026-09-04 in `AppsWideLayout.geometry.test.tsx`, + * moving this element AFTER `` changed no rendered width at all — every + * geometry assertion in that file still passed. React inserts nodes through the DOM API rather than + * the HTML parser, so the parser's table foster-parenting never runs and Chromium applies + * the columns from wherever the element sits. The ordering is therefore a VALIDITY rule + * enforced structurally in `__tests__/appsWideLayout.test.ts`, not something the pixels + * can see — and a guard whose stated reason is false is the kind that gets deleted. + * + * 🔴 THE PRIMARY COLUMN GETS NO `width` AT ALL, and that absence is the mechanism. + * Under CSS automatic table layout a column with a specified width is given it; what is + * left over goes to the columns WITHOUT one. With exactly one such column, "what is left + * over" is the whole surplus, which is precisely "the primary column absorbs the slack". + * Giving it a percentage instead would make it merely another proportional column and + * the extra width would go back to being distributed as padding. + */ +export function AppsTableColgroup({ columns }: { columns: AppsTableColumns }) { + return ( + + {columns.map((pct, i) => ( + + ))} + + ); +} + +/** + * How many tracks {@link AppsCardGrid} resolves to at a given CONTENT width — the + * `repeat(auto-fill, minmax(min, 1fr))` arithmetic, as a pure function so the ladder + * can be pinned without a browser. + * + * Mirrors the CSS: `auto-fill` fits `floor((available + gap) / (min + gap))` tracks, and + * never fewer than one. + */ +export function appsCardGridColumnsAt( + contentWidth: number, + minColumn: number = APPS_CARD_LIST_MIN_COLUMN, + gap: number = APPS_CARD_LIST_GAP +): number { + return Math.max(1, Math.floor((contentWidth + gap) / (minColumn + gap))); +} + +/** The content width a route with no measure gets from the shared container. */ +export const APPS_FULL_MEASURE_CONTENT_WIDTH = APPS_PAGE_CONTAINER_WIDTH - APPS_CONTAINER_GUTTER; + +/** The content width the SAME route got from the container before the ultrawide pass. */ +export const APPS_LEGACY_CONTENT_WIDTH = APPS_LEGACY_CONTAINER_WIDTH - APPS_CONTAINER_GUTTER; + +/** + * A card list that spends surplus width on COLUMNS rather than on stretching one card. + * + * Replaces a `` of full-width cards. + * + * 🔴 THE INNER `min(100%, N)` IS LOAD-BEARING, AND ITS FAILURE MODE IS WORSE THAN THIS + * COMMENT USED TO SAY. It claimed a bare `minmax(1200px, 1fr)` "overflows horizontally". + * Measured at 390×844 with the `min()` removed: `gridBox=358`, `gridScroll=1200`, + * `child=1200`, and `document.scrollWidth` **unchanged** — so the card is 1200px wide + * inside a 358px grid, CLIPPED, with no scrollbar and no page-level overflow to notice + * it by. Nothing on screen says the content is cut off. That matters because this + * component converted three phone-reachable `Stack`s on `/apps/installed` into grids, so + * the narrow case is a real users' case rather than a theoretical one, and it is pinned + * at a phone viewport in `AppsWideLayout.geometry.test.tsx`. + * + * `alignItems: start` is deliberate and is NOT the store grid's bug: these cards are + * independent blocks with nothing bottom-pinned inside them (compare + * `AppListingsMarketplaceBody.stretch.geometry.test.tsx`, where stretching is what makes + * `h-full` resolve), so stretching them to the tallest row member would only inflate the + * short ones. + */ +export function AppsCardGrid({ + children, + testId, + gap = APPS_CARD_LIST_GAP, +}: { + children: ReactNode; + /** Optional `data-testid`, so a page's existing list id survives the swap. */ + testId?: string; + /** + * Track gap in px. Defaults to {@link APPS_CARD_LIST_GAP} (Mantine `md`). + * + * 🔴 IT IS A PROP BECAUSE THE PROVENANCE RULE APPLIES TO SPACING TOO. The lists this + * replaced did not all use the same gap — `/apps/installed`'s Hidden tab was + * `` (12px) — and defaulting every one of them to 16 would have moved + * something a 1440 monitor shows, which is precisely what + * `APPS_CARD_LIST_MIN_COLUMN`'s "nothing a 1440 or 1920 monitor shows changes" claim + * forbids. Both gaps yield the SAME column ladder at both container widths (pinned in + * `__tests__/appsWideLayout.test.ts`), so carrying the original number costs nothing. + */ + gap?: number; +}) { + return ( +
+ {children} +
+ ); +} diff --git a/src/pages/apps/installed.tsx b/src/pages/apps/installed.tsx index ff8cfde8c9..0b75c79e30 100644 --- a/src/pages/apps/installed.tsx +++ b/src/pages/apps/installed.tsx @@ -30,6 +30,7 @@ import { NotFound } from '~/components/AppLayout/NotFound'; import { openAppSettingsModal } from '~/components/Apps/AppSettingsModal'; import { Meta } from '~/components/Meta/Meta'; import { AppsPageLayout } from '~/components/Apps/AppsPageLayout'; +import { AppsCardGrid } from '~/components/Apps/appsWideLayout'; import { groupSubscriptionsByApp } from '~/components/Apps/groupSubscriptionsByApp'; import type { GroupedApp } from '~/components/Apps/groupSubscriptionsByApp'; import { useHiddenBlockList, unhideBlock } from '~/components/AppBlocks/hiddenBlocks'; @@ -217,7 +218,9 @@ interface InstalledAppCardProps { * surfaces are active happens through the existing AppSettingsModal (the * Manage button), which already supports both scopes. */ -function InstalledAppCard({ app, onManage }: InstalledAppCardProps) { +/** 🔴 EXPORTED SO ITS GEOMETRY CAN BE MEASURED — `AppsWideLayout.geometry.test.tsx` + * mounts THIS card rather than a fixture copy of its markup. */ +export function InstalledAppCard({ app, onManage }: InstalledAppCardProps) { const { blanketPublisher, blanketViewer, pinned } = app; const name = app.manifest.name ?? app.blockId; // Any blanket sub on the app is enough to seed the Manage modal — it @@ -355,7 +358,7 @@ function ScopeGrantsPanel() { return ; } return ( - + {grants.map((grant) => ( @@ -379,7 +382,7 @@ function ScopeGrantsPanel() { ))} - + ); } @@ -409,7 +412,10 @@ function HiddenBlocksPanel() { } return ( - + /* `gap={12}` — this list was ``, not `md`. Carrying its own number + keeps `APPS_CARD_LIST_MIN_COLUMN`'s "nothing a 1440 or 1920 monitor shows changes" + literally true on this tab; the column ladder is identical at both gaps. */ + {hidden.map((block) => ( @@ -446,7 +452,7 @@ function HiddenBlocksPanel() { ))} - + ); } @@ -530,11 +536,13 @@ export default function InstalledAppsPage() { ) : groupedApps.length === 0 ? ( ) : ( - + /* A GRID, NOT A `Stack` — the 640px dead-gap fix. Rationale + the measured + ladder: `APPS_CARD_LIST_MIN_COLUMN` in `~/components/Apps/appsPageWidths`. */ + {groupedApps.map((app) => ( ))} - + )} diff --git a/src/pages/apps/review.tsx b/src/pages/apps/review.tsx index 2c068d6ba2..ec2fcee26b 100644 --- a/src/pages/apps/review.tsx +++ b/src/pages/apps/review.tsx @@ -33,7 +33,6 @@ import type { } from '~/components/Apps/unifiedReviewRow'; import { Meta } from '~/components/Meta/Meta'; import { AppsPageLayout } from '~/components/Apps/AppsPageLayout'; -import { APPS_PAGE_MEASURES } from '~/components/Apps/appsPageWidths'; import { EMBEDDED_KIND_LABEL, STANDALONE_KIND_LABEL } from '~/components/Apps/listingKindLabels'; import { useFeatureFlags } from '~/providers/FeatureFlagsProvider'; import { isAppReviewer } from '~/shared/utils/app-blocks-access'; @@ -183,8 +182,15 @@ export default function ReviewQueuePage() { return ( <> + {/* + 🔴 NO `measure` — this page took a 1368 cap until the wide-tables pass, and + removing it is the point rather than a side effect. The cap existed because four + short columns could not spend the container, so the surplus landed as padding + between the last column and the Review button. `UnifiedReviewList` now carries a + proportional `` (`APPS_REVIEW_QUEUE_COLUMNS`), so the width goes into + the App column instead. Re-adding a cap here would hide that rather than help it. + */} diff --git a/test/geometry-setup.tsx b/test/geometry-setup.tsx index 51043c01b1..ad00fdbb0c 100644 --- a/test/geometry-setup.tsx +++ b/test/geometry-setup.tsx @@ -96,25 +96,27 @@ * the vacuous pass this harness exists to remove. * * ───────────────────────────────────────────────────────────────────────────── - * 🔴 WHAT RUNS THIS, TODAY: NOTHING. SAY IT OUT LOUD. + * 🔴 WHAT RUNS THIS: THE `Geometry tests` JOB — AND IT IS REPORT-ONLY ON A PR. * ───────────────────────────────────────────────────────────────────────────── - * `.github/workflows/lint.yml` selects projects by name — `--project 'unit*'`, - * `--project '@civitai/*'`, `--project 'app:*'`. None of those patterns matches - * `component` and none matches `geometry`, because the Actions runners install no - * Chromium (the `unit` job's own comment says so). The `component` tier's only CI - * home is the preview pipeline's `preview / component-tests` status, which is - * report-only. This project has no CI home at all yet. + * ⚠️ THIS PARAGRAPH SAID "NOTHING RUNS THIS, SAY IT OUT LOUD" AND HAD STOPPED BEING + * TRUE. `.github/workflows/lint.yml` now carries a `geometry:` job (`Geometry tests`) + * that installs Chromium and runs `vitest run --project geometry`, plus a collected- + * count ledger so a selector that matches nothing fails instead of exiting 0. It is + * observable on any PR's check list. Left uncorrected, the sentence would have talked + * the next reader out of relying on a gate that exists — the mirror image of the rot it + * was written to prevent. * - * That is a deliberate scope boundary, not an oversight: wiring a new job is a - * change to a pipeline, not to a test harness, and it deserves its own review. But - * it has to be stated, because a harness nothing runs rots — the assertions here - * would drift from the components they measure and nobody would learn of it until - * someone ran `pnpm test:geometry` by hand. Until this project is wired into a - * job, treat it as a tool you run deliberately, NOT as a gate you are behind. + * What is still true, and is the part that matters when you read a green check: + * the job carries `continue-on-error: ${{ github.event_name == 'pull_request' }}`, so + * on a PR it REPORTS and on a push to `main` it BLOCKS. Do not write "the blocking + * tier" about this project without that qualifier. * - * Two things are already true that make wiring it cheap when someone does: - * `pnpm run test:geometry` is the whole command, and the glob is disjoint from - * every other project's, so adding it cannot change what any existing job runs. + * `component` is still ungated: no project selector in that workflow matches it, and + * its only CI home is the preview pipeline's report-only `preview / component-tests`. + * + * `pnpm run test:geometry` is still the whole command locally, and the glob is + * disjoint from every other project's, so adding a file here cannot change what any + * other job runs. */ // ── THE CASCADE, IN PRODUCTION ORDER ─────────────────────────────────────────