fix(nav): scroll the sub nav tab row, and keep the filters on its line (#4834)

The row is content-width and never collapses — `useResolvedNav` derives the
bar/More split from the user's saved config, not the viewport — so it measures a
fixed ~1334px signed in at every width from 1024 to 1400. `@md:overflow-visible`
overrode both overflow axes above the `md` container breakpoint (1024px),
removing the row's only escape: above 1024 it could not scroll, and an ancestor
`overflow-hidden` clipped whatever exceeded the viewport.

Measured at 1136px signed in, on `/`, `/models` and `/leaderboard/overall`: row
right edge 1334, scrollable overflow 0, and neither Shop nor More hit-testable.
`document.scrollWidth` equalled the viewport at every width, which is why a
page-level overflow check finds nothing here.

Dropping the override restores the scroll the row already relies on below 1024.
`overflow-y` cannot be `visible` beside `overflow-x: auto` — it computes to
`auto` — so the row clips on both axes, and an outline contributes no scrollable
overflow. The row pays the focus ring's 4px of ink on the axis that scrolls,
sized from the ink and carrying `var(--mantine-scale)` as Mantine's own rule
does, rather than from a rem scale that only matches at a 16px root.

The padding is horizontal ONLY, and that is a trade rather than an oversight.
Padding all four sides also unclipped the ring top and bottom, including below
1024 where it is clipped today, but it made the bar taller on every page at every
width. Justin saw the rendered result and declined it. Not "unchanged behaviour",
though: the 36px More button used to set the row's height and leave a 32px pill 2px
of slack, so half the ring showed. Shrinking More to pill height took that. The
mechanism is unchanged; the amount is not.

Second problem, same bar: on feed routes the filters and the settings gear
wrapped to a second line, doubling the bar's height from 44px to 88px. `SubNav2`
wraps, and a wrapping container places items at their flex BASIS before shrinking
any of them, so at `basis: auto` the row's content width does not fit and the
siblings wrap. `flex-1` gives it `basis: 0`: both share the line, and the row
absorbs the shortfall by scrolling. `shrink-0` on the More button is the other
half, because that shrink then lands on the children and More is the one that
collapses, to an empty 28px circle.

`min-w-0` is deliberately absent: `overflow-x: auto` already zeroes a flex item's
automatic minimum size, and removing it changed nothing at any of eight widths.
So is `lg:flex-nowrap`, measured the same way.

Measured on /images, sub nav height, against a control built by reverting only
these classes on the same dev server: 88px to 44px at 1440/1280/1184/1136/1024/900,
and byte-identical child geometry at 768/640/390. The band is route-dependent and
those figures are one route: 40px on /models where every control is `h-8`, 44px on
/images where one filter control is 36px, and 36px on /comics, where
`FilterButton`'s `compact-sm` takes the `h-9` branch.

The rest is one size for the whole row, all of it measured rather than eyeballed:
`SubNav2` top-aligns its children, because on platforms that draw classic
scrollbars the scroller is taller than its pills and centring put the filters half
a scrollbar low; the More button is 32px at 14px/600, matching the pills, where it
was 36px at 16px/500; and the settings gear is 32px, circular, with a 16px icon in
`--mantine-color-bright`. The gear's icon was never smaller than its neighbours —
every icon in the bar is 16x16 with a 2px stroke — it was rgb(222,226,230) against
their rgb(254,254,254), and at a matched box that reads as smaller rather than
dimmer. `bright` is the same #222/#fefefe pair the pills use, so the gear matches
the pills in both schemes; against the globe specifically it is exact in dark and
slightly darker in light, where the globe is `text-gray-8`.

The scrollbar is deliberately left visible: it is the only thing telling anyone the
row scrolls, and "it doesn't look like it scrolls" was the original report.

Guarded at two tiers, and every guard was mutated:

  Shell's provider deleted     RED  expected false to be true
  scale-95 planted (whitelist) RED  to deeply equal [...7 items]
  relative on the row          RED  expected false to be true
  flex-1 removed               RED  expected 32 to be +0
  shrink-0 removed from More   RED  expected 28 to be close to 81.796875
  padding to px-0              RED  expected -4 to be >= 0
  @md:overflow-visible back    RED  expected 'visible' to be 'auto'

The hit-test is the one that needed building twice. The trap it guards — a
`position: sticky` or `relative` ancestor becoming the unportalled dropdown's
containing block — produces a dropdown whose rect is IDENTICAL to a working one
while its items stop being hit-testable, so a rect assertion and a `textContent`
assertion both pass against the broken state. The first version of that test still
passed with `relative` planted, because the geometry harness mounts a bare
`MantineProvider` and never receives `ThemeProvider`'s `Popover.withinPortal:
false`: the menu portalled in the test while rendering inline in the app. It now
nests a provider carrying that default.

The source gate strips comments before matching, so deleting the live row and
leaving a commented-out copy fails loudly instead of passing silently, and it
rejects positioning tokens on the row so the prohibition above is checked rather
than merely written down.

`.moreButton`'s height moved from an explicit `32px` to `h-8`, beside the pills'
own. Measured either side on the same route and browser rather than argued from
the cascade, because that cascade misled two reviewers on this file today:
32px / 16px / 10px / 14px / 600 in both states, identical on every axis.

A second tidy-up — dropping `variant="subtle"`, which `LegacyActionIcon` and
`ThemeProvider` each already set — is deliberately NOT here. The gear does not
render for the probe session, so it could not be measured, and an unverifiable
change whose only benefit is tidiness is not worth carrying.

`.moreButton` no longer states a font either: measured with the declarations
removed, the button still renders 14px/600, because that is what a Mantine
`size="sm"` Button already produces. Two lines of framework default, deleted.

The More button's height is now pinned in the geometry tier against a PILL's
rather than a literal — deleting `h-8` reddens with `expected 36 to be 32`,
where before it passed both tiers green while growing the bar 4px site-wide.

The More button's height and typography are pinned in the geometry tier against
a PILL's and against the literal 32, because parity alone passes when both move:
`h-8` to `h-9` on both controls leaves them equal at Mantine's 36px and grows the
bar 4px on every page. Mutants: 36-vs-32 either side, and 20px-vs-14px on the font.

The pill's `text-base font-medium` does NOT render. `globals.css`'s unlayered
`.mantine-Button-label *` sets both to `inherit` and sits after
`@tailwind utilities`, so the span takes Mantine's `size="sm"` 14px/600. Three
reviewers read those classes as 16px/500 in one day, so it is written beside them.

The `geometry` project is `continue-on-error` on pull requests, so everything it
measures informs rather than gates. The height claim is therefore asserted in the
`unit` tier too — a 0.3s check that the More button's `clsx` still carries `h-8`.
Paired control: removing it gives `expected [ 'shrink-0' ] to include 'h-8'`,
reordering the class list stays green. The font and geometry claims stay advisory
and the PR body says so.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Justin Maier
2026-09-15 18:08:18 -06:00
committed by GitHub
parent 7def68db71
commit 233d0aa8be
8 changed files with 673 additions and 22 deletions
+13 -8
View File
@@ -791,7 +791,7 @@ jobs:
# (`unit*`, `@civitai/*`, `app:*`), and its only CI home is the preview
# pipeline's report-only `preview / component-tests`. Adding it here is a
# much larger change than this harness and belongs in its own PR, on measured
# cost: locally, on a 16-core box, `geometry` is 2 files / 10 tests in 9.07s
# cost: locally, on a 16-core box, `geometry` is 6 files / 73 tests in 44.93s (2026-09-15)
# while `component` is 212 files / 2,362 tests in 112.92s wall — and 334s of
# that is test time spread across workers, so a 2-core runner (browser pool
# `min(12, cpus - 1)` = 1 instance) does not divide it. That is a job an order
@@ -824,9 +824,10 @@ jobs:
# remove `continue-on-error` and this paragraph together. What would make that
# safe, concretely: no run red for a reason other than a real geometry
# regression — in particular no Chromium launch failures and no timeouts on a
# 2-core runner. The tier is 10 tests and ~19s on a runner, so unlike `unit`
# there is no import/transform phase to blame; if it goes red it is either the
# code or the browser, and both are worth someone's attention.
# 2-core runner. This tier has no import/transform phase to blame, unlike
# `unit`, so a red is either the code or the browser — both worth someone's
# attention. (Runner wall time has not been re-measured since the tier was
# 10 tests; it is 73 now.)
continue-on-error: ${{ github.event_name == 'pull_request' }}
timeout-minutes: 15
steps:
@@ -881,8 +882,8 @@ jobs:
# breaks this — only losing one does.
#
# 🔴 THE FILE COUNT COMES FROM `testResults.length`, NOT FROM
# `numTotalTestSuites`. Measured against a real report: this run is 2 files
# and `numTotalTestSuites` is 4, because that field counts `describe` blocks
# `numTotalTestSuites`. Measured against a real report when the tier was 2
# files, `numTotalTestSuites` read 4, because that field counts `describe`s
# rather than files. A floor written against it would have been off by a
# factor that changes whenever someone adds a `describe`.
#
@@ -898,8 +899,12 @@ jobs:
if: always()
run: |
node - <<'JS'
// Minimum ledger for the `geometry` tier.
const MIN_FILES = 2;
// Minimum ledger for the `geometry` tier. Measured 2026-09-15: 6 files / 73 tests.
// FILES ratchets at exactly current — files do not merge by accident, so a drop means a
// file stopped being collected, which is the thing this ledger exists to catch. TESTS
// stays conservative: a legitimate refactor merges cases, and a test-count ratchet at
// current would teach people to raise the floor rather than investigate the drop.
const MIN_FILES = 6;
const MIN_TESTS = 10;
const REPORT = 'geometry-report.json';
+10 -1
View File
@@ -38,8 +38,17 @@ export function SubNav2() {
const section = filterSections.find((x) => x.pathname === router.pathname);
return (
// `items-start`, not `items-center`: `HomeTabs` is a horizontal scroller, and on platforms
// that draw classic (space-consuming) scrollbars it is taller than its pills by the scrollbar's
// height. Centring put the filters and the gear half a scrollbar below the tabs. Justin asked
// for them level with the tabs (2026-09-15), which is the top edge.
//
// The scrollbar itself is deliberately NOT hidden. It is the only thing telling anyone the row
// scrolls, and "it doesn't look like it scrolls" was the original report. `scrollbar-none` here
// would take the 4px back and re-break that — the sibling row in `HomeStyleSegmentedControl`
// styles its scrollbar rather than removing it, if a thinner one is ever wanted.
<div
className={clsx('flex items-center justify-between gap-2 px-2 py-1', {
className={clsx('flex items-start justify-between gap-2 px-2 py-1', {
['flex-wrap']: router.pathname !== '/',
})}
>
@@ -65,9 +65,13 @@
}
.moreButton {
padding: 8px 10px 8px 16px;
font-size: 16px;
font-weight: 500;
/* Label leads and the caret trails, so this is not the pills' `pl-3 pr-4`: 16px in, 10px out.
Height is `h-8` on the element beside the pills' own, and has to stay there: measured
2026-09-15, a `height: 40px` added back here rendered 32px, because Turbopack wraps modules in
`@layer modules` and `@tailwind utilities` is deliberately unlayered. (Under
`next build --webpack` modules are unlayered too and the winner is emission order — no build
in use takes that path.) */
padding: 0 10px 0 16px;
&[data-active="true"] {
background: light-dark(var(--mantine-color-gray-4), var(--mantine-color-dark-4));
@@ -60,7 +60,23 @@ export function HomeTabs() {
};
return (
<div className="flex items-center gap-1 overflow-x-auto overflow-y-hidden text-black @md:overflow-visible dark:text-white">
// Horizontal padding only, and it is load-bearing: `overflow-x: auto` clips at the padding box,
// and an outline contributes no scrollable overflow, so without it the first pill's focus ring
// is cut on the left and the last control's on the right with no way to scroll them into view.
// No VERTICAL padding — it would make the bar taller on every page, which Justin declined on
// review, so the ring is clipped top and bottom.
//
// Not `px-1`: Mantine's ring is a `2px` outline at `calc(0.125rem * var(--mantine-scale))`
// offset, so a pure-rem class matches the ink only at a 16px root with scale 1. 4px at
// defaults.
//
// `flex-1` keeps `SubNav2`'s feed filters on this same line. That container wraps, and a
// wrapping container places items at their flex BASIS before shrinking any of them, so at the
// default `basis: auto` this row's ~1334px of content does not fit and the filters wrap to a
// second line. `basis: 0` lets both share the line and this row absorb the shortfall by
// scrolling. `shrink-0` on the More button below is the other half: the shrink then lands on
// the children, and More is the one that collapses — to an empty 28px circle.
<div className="flex flex-1 items-center gap-1 overflow-x-auto overflow-y-hidden px-[calc(0.125rem*var(--mantine-scale,1)+2px)] text-black dark:text-white">
{bar.map((entry) => {
const label = getDisplayName(entry.key);
// Mantine's `disabled` only gates the Transition, not the portal — a disabled Tooltip
@@ -84,6 +100,10 @@ export function HomeTabs() {
classNames={{ label: 'flex gap-2 items-center capitalize overflow-visible' }}
>
{navIcons[entry.key]({ size: 16 })}
{/* These two do NOT render: `globals.css`'s `.mantine-Button-label *` sets `font-size`
and `font-weight` to `inherit`, is unlayered, and sits after `@tailwind utilities`,
so the span inherits Mantine's `size="sm"` 14px/600. Three reviewers have read this
as 16px/500 in one day. Measured 2026-09-15. */}
{showLabels && <span className="text-base font-medium capitalize">{label}</span>}
{dot(entry, '-ml-1 -mr-2')}
</Button>
@@ -114,7 +134,7 @@ export function HomeTabs() {
color="gray"
variant="subtle"
data-active={moreOpened}
className={classes.moreButton}
className={clsx('h-8 shrink-0', classes.moreButton)}
>
<Group gap={4} wrap="nowrap">
More
@@ -0,0 +1,413 @@
/**
* THE SUB NAV TAB ROW IS REACHABLE AT EVERY WIDTH — MEASURED IN PIXELS.
*
* The defect: the row is content-width and never collapses (`useResolvedNav` derives the bar/More
* split from saved config, not the viewport), so it measured a fixed ~1334px signed in at every
* width from 1024 to 1400 (measured 2026-09-15). `@md:overflow-visible` overrode BOTH overflow axes above the `md`
* container breakpoint, so above 1024 the row could not scroll and an ancestor `overflow-hidden`
* clipped "Shop" and "More" out of reach. `document.scrollWidth` stayed equal to the viewport the
* whole time, which is why no page-level check found it.
*
* WHY THIS TIER. The sibling source gate pins the class list, and a class list is a spelling.
* Measured by mutation: `@md:overflow-visible` and `md:overflow-visible` switch at the identical
* 1024px — `screens` and `containers` both come from `breakpoints.json` — and produce the identical
* dead band, so any guard that enumerates variant prefixes catches one and misses the other. This
* file asserts the CONSEQUENCE instead: the row is a real scroller and the ring fits. That holds
* under every spelling, including the container-query one that shipped the defect — see `Shell`,
* without which this file is blind to exactly that class. It also catches causes no class list can
* express: a `style={{}}` override, a CSS-module rule, or an ancestor's clip.
*
* It cannot be written in the `component` tier: that tier loads 24 CSS rules and no Tailwind, so
* `overflow-x-auto` computes to the INITIAL value `visible` there and an assertion of "not visible"
* would pass against a completely broken row. See `test/geometry-setup.tsx`.
*/
import type { ReactNode } from 'react';
import { describe, expect, test, vi } from 'vitest';
import { cleanup } from 'vitest-browser-react';
import { MantineProvider } from '@mantine/core';
import { cascadeEvidence, nextLayout, renderAtViewport } from '../../../test/geometry-setup';
import type * as TrpcModule from '~/utils/trpc';
vi.mock('~/providers/FeatureFlagsProvider', () => ({
useFeatureFlags: () => ({
articles: true,
bounties: true,
comicCreator: true,
challengePlatform: true,
cosmeticShop: true,
model3dFeed: true,
userHubs: true,
auctions: true,
vault: true,
}),
}));
vi.mock('~/hooks/useCurrentUser', () => ({
useCurrentUser: () => ({ id: 1, username: 'tester' }),
}));
vi.mock('~/components/UserSettings/hooks', () => ({ useCurrentUserSettings: () => ({}) }));
vi.mock('next/router', () => ({
useRouter: () => ({ pathname: '/leaderboard/[id]', asPath: '/' }),
}));
// `...actual` is the real module, so a new export cannot silently become `undefined`. `trpc` itself
// is REPLACED rather than spread: `createTRPCNext` returns a flat Proxy over a function, which has
// no own enumerable keys, so `{ ...actual.trpc }` copies nothing and would only look like a merge.
// `HomeTabs` reaches for exactly one procedure; a second one fails loudly on `undefined.useQuery`.
vi.mock('~/utils/trpc', async (importOriginal) => {
const actual = await importOriginal<typeof TrpcModule>();
return { ...actual, trpc: { changelog: { getLatest: { useQuery: () => ({ data: 0 }) } } } };
});
const { HomeTabs } = await import('./HomeContentToggle');
/**
* 🔴 THE `@container` WRAPPER IS NOT DECORATION — WITHOUT IT THIS FILE CANNOT SEE THE BUG.
*
* `@md:overflow-visible`, the class that shipped the defect, is a CONTAINER query: Tailwind
* compiles it to `@container (min-width: …)`, which is FALSE when no ancestor establishes a query
* container. The harness providers are `QueryClientProvider` + a bare `MantineProvider`, so a row
* rendered directly under them never matches it. Measured: with `@md:overflow-visible` back on the
* row and no wrapper, this file reported every test passing, exit 0.
*
* In production the container is `ScrollArea` (`'scroll-area flex-1 @container'`) and the clip
* comes from an ancestor's `overflow-hidden` — which is also why the page never grew a scrollbar
* while the tabs were unreachable. Both are reproduced here.
*/
function Shell({ children }: { children: ReactNode }) {
return (
<div className="overflow-hidden">
<div className="@container">
{/* 🔴 The harness's own provider is a BARE `<MantineProvider>` (`test/geometry-setup.tsx`),
so none of `ThemeProvider`'s defaults reach it — including
`Popover: { defaultProps: { withinPortal: false } }`, which `Menu` inherits. Without
this nested provider the More menu PORTALS in here while it renders inline in the app,
and the reachability test below cannot see a clipped dropdown at all: measured
2026-09-15, it reported 8 passed with `relative` planted on the row. */}
<MantineProvider
theme={{ components: { Popover: { defaultProps: { withinPortal: false } } } }}
>
{children}
</MantineProvider>
</div>
</div>
);
}
/** The scrolling row itself — the element every assertion below is about. */
function tabRow(): HTMLElement {
const anchor = document.querySelector('a[href="/models"]');
if (!anchor?.parentElement)
throw new Error('tab row not found — HomeTabs rendered no /models pill');
return anchor.parentElement;
}
/** The More menu's trigger — by its label, since `data-testid` is stripped from production. */
function moreButton(): HTMLElement {
const found = [...tabRow().querySelectorAll('button')].find((b) =>
(b.textContent ?? '').trim().startsWith('More')
);
if (!found) throw new Error('More button not rendered — check the feature-flag mock');
return found as HTMLElement;
}
/**
* The ring Mantine draws OUTSIDE an element's border box: outline-width + outline-offset.
*
* The ring only exists while the element is focused, so this focuses before reading — an
* unfocused element reports the initial `outline-width: 0`. `focusVisible` asks for the visible
* ring explicitly rather than relying on the heuristic; measured, a plain `.focus()` also yields
* 4px here, so it is intent, not a workaround.
*
* 🔴 The caller asserts this is nonzero BEFORE the containment checks, and that ordering is the
* load-bearing part: at 0 the inflated rect IS the rect, so every containment assertion compares
* a box against itself and passes against a row with no padding at all. Do not remove the nonzero
* assertion as redundant — it is what makes an unexpected 0 loud instead of silent.
*/
function ringInk(el: HTMLElement): number {
// `preventScroll` because focusing scrolls the element into view by default, which would move
// the very rects the caller is about to compare.
el.focus({ focusVisible: true, preventScroll: true } as FocusOptions);
const s = getComputedStyle(el);
return (parseFloat(s.outlineWidth) || 0) + (parseFloat(s.outlineOffset) || 0);
}
describe('sub nav tab row geometry', () => {
test('the real cascade is loaded — without this every assertion below is vacuous', async () => {
const { observed } = await renderAtViewport(
<Shell>
<HomeTabs />
</Shell>,
{ width: 1136, height: 800 }
);
expect(observed).toEqual({ width: 1136, height: 800 });
// `overflow-x-auto` computing to `auto` is only meaningful if Tailwind is actually present:
// an unstyled element reports the INITIAL value for most of what this file reads. Assert a
// fact that is false without the stylesheet before asserting anything about the row.
const evidence = cascadeEvidence();
expect(evidence.tailwindFlexUtilityResolves).toBe(true);
expect(evidence.ruleCount).toBeGreaterThan(1000);
});
test('is a horizontal scroller whose content is reachable at 1136px', async () => {
await renderAtViewport(
<Shell>
<HomeTabs />
</Shell>,
{ width: 1136, height: 800 }
);
await nextLayout();
const row = tabRow();
// The bug: `overflow-x` computed `visible`, so the row rendered at full content width and was
// clipped by an ancestor with no way to scroll.
expect(getComputedStyle(row).overflowX).toBe('auto');
// That this OVERFLOWS at 1136 is a property of the fixture — nine flags on and no saved config
// give a ~1334px row (2026-09-15). If a future default nav ships fewer tabs this reddens here,
// which is the fixture going stale rather than the row breaking.
expect(row.scrollWidth).toBeGreaterThan(row.clientWidth);
// The scroll actually moves: a clamped assignment would leave this at 0, and then a later
// reachability claim would be about a row that never scrolled.
row.scrollLeft = row.scrollWidth;
await nextLayout();
// Not exact equality: `scrollWidth`/`clientWidth` are rounded integers while Chromium's scroll
// offset can be fractional, and a sub-pixel difference here would read as a code regression.
expect(row.scrollLeft).toBeCloseTo(row.scrollWidth - row.clientWidth, 0);
});
test.each([1200, 1500])('stays a scroller at %ipx', async (width) => {
// The sibling source gate is a class list, and a class list is a spelling; this file is the
// half that measures. But a single measurement point is its own blind spot — `lg:` variants
// (1184px) are not live at 1136, so a `lg:`-prefixed override would pass a 1136-only check.
await renderAtViewport(
<Shell>
<HomeTabs />
</Shell>,
{ width, height: 800 }
);
await nextLayout();
expect(getComputedStyle(tabRow()).overflowX).toBe('auto');
});
test('shrinks so the feed filters keep their place on the same line', async () => {
// NAMED FOR THE DECISION: Justin asked (2026-09-15) for the filters and the settings gear to
// stay on one line with the tabs, and for the tab row to scroll earlier instead of pushing
// them to a second row.
//
// `flex-1` is what delivers that, and the wrapper here must ALLOW WRAPPING or this test
// cannot see it. `SubNav2` wraps, and a wrapping container places items at their FLEX BASIS
// before it shrinks anything: at `basis: auto` the row's basis is its ~1334px content, which
// does not fit, so the sibling wraps. `flex-1` sets `basis: 0`, so both fit on the line and
// the row then shrinks into what is left. Under `flex-nowrap` nothing wraps whatever the
// basis is, and this test passed against a reverted `flex-1` — measured 2026-09-15.
//
// `min-w-0` is deliberately NOT on the row. A flex item's automatic minimum size would
// normally stop it shrinking below its content, but `overflow-x: auto` already zeroes that,
// so the class is redundant here — removing it changed nothing at any of eight widths.
const FILTERS_WIDTH = 402;
await renderAtViewport(
<Shell>
<div className="flex flex-wrap items-start" data-row-under-test>
<HomeTabs />
<div style={{ width: FILTERS_WIDTH, flexShrink: 0, height: 36 }} data-filters />
</div>
</Shell>,
{ width: 1136, height: 800 }
);
await nextLayout();
const row = tabRow();
const line = row.parentElement as HTMLElement;
const filters = line.querySelector('[data-filters]') as HTMLElement;
// The row gave way rather than the sibling: it is narrower than its own content, and narrow
// enough to leave the sibling its full width on the same line.
expect(row.clientWidth).toBeLessThan(row.scrollWidth);
expect(filters.getBoundingClientRect().width).toBe(FILTERS_WIDTH);
expect(filters.getBoundingClientRect().right).toBeLessThanOrEqual(
line.getBoundingClientRect().right + 1
);
// Same line, not merely both present — the whole point of the change.
expect(Math.round(filters.getBoundingClientRect().top)).toBe(
Math.round(row.getBoundingClientRect().top)
);
});
test('keeps the More button from being squashed by that shrink', async () => {
// The fix above passes shrink pressure to the row's children, and More is the one that gives:
// measured 2026-09-15 before `shrink-0`, it collapsed to an empty 28px circle with its label
// squeezed away, which is what Justin saw and reported. `textContent` reads "More" in both
// states — the text is still in the DOM — so the width is what separates them.
//
// Compared against its OWN unpressured width rather than a threshold. A constant would be
// tuned to this box's fonts while the fixed state (~87px) is font-dependent and the squashed
// one (~28px) is nearly not, so the margin differs per machine. This needs no number.
// Wide enough that the row does NOT overflow, so nothing is shrinking. At 1136 the row
// overflows on its own and More is already squashed without the sibling — measured, that made
// an earlier version of this comparison pass against a reverted `shrink-0`, both sides 28px.
await renderAtViewport(
<Shell>
<div className="flex flex-wrap items-start">
<HomeTabs />
</div>
</Shell>,
{ width: 1600, height: 800 }
);
await nextLayout();
const relaxedRow = tabRow();
expect(relaxedRow.scrollWidth).toBeLessThanOrEqual(relaxedRow.clientWidth);
const relaxed = moreButton().getBoundingClientRect().width;
// AWAITED because this one sits BETWEEN two renders: `cleanup()` returns a promise and unmounts
// via `act`, and `tabRow()` resolves by document selector, so an unawaited teardown leaves the
// first render in the document and both reads below land on it instead of the pressured row.
await cleanup();
await renderAtViewport(
<Shell>
<div className="flex flex-wrap items-start">
<HomeTabs />
<div style={{ width: 402, flexShrink: 0, height: 32 }} />
</div>
</Shell>,
{ width: 1136, height: 800 }
);
await nextLayout();
const row = tabRow();
// The premise. Without this the test can pass by the row never having been squeezed at all —
// if the pills ever start absorbing the shortfall first, it would keep passing while having
// stopped measuring `shrink-0`.
expect(row.clientWidth).toBeLessThan(row.scrollWidth);
// Against a PILL **and** against the number. Parity alone is not enough: deleting `h-8` from
// both the pills and More — or moving both to `h-9`, which is the likelier edit — leaves them
// equal at Mantine's `size="sm"` 36px and grows the bar 4px on every page. Parity catches the
// one-sided change, the literal catches the two-sided one, so keep both.
const firstPill = row.children[0] as HTMLElement;
// Or the assertion is `x === x`: with an empty `bar`, child zero IS the More button.
expect(firstPill).not.toBe(moreButton());
const pillHeight = firstPill.getBoundingClientRect().height;
expect(pillHeight).toBeCloseTo(32, 1);
expect(moreButton().getBoundingClientRect().height).toBeCloseTo(pillHeight, 1);
// The font is not pinned in CSS any more — `.moreButton` dropped its `font-size`/`font-weight`
// because a Mantine `size="sm"` Button already produces 14px/600. That makes this the only
// thing standing between the two controls and a silent typographic divergence.
const labelFont = (el: Element) => {
const s = getComputedStyle(el);
return `${s.fontSize}/${s.fontWeight}`;
};
expect(labelFont(moreButton())).toBe(labelFont(firstPill));
// Precision 1 (±0.05), not 2 (±0.005): Chromium quantises rects to 1/64px = 0.0156, so a
// single LayoutUnit of drift would fail the tighter one, and ±0.05 still separates the
// squashed ~28px from the fixed width by three orders of magnitude.
expect(moreButton().getBoundingClientRect().width).toBeCloseTo(relaxed, 1);
});
test('keeps the More menu reachable — a clipped dropdown has the same rect as a working one', async () => {
// NAMED FOR THE TRAP. The block comment in the sibling source gate forbids `relative`,
// `position: sticky`, `@container`, `transform`, a `filter` and `will-change` on this row,
// because the More dropdown is not portalled: its containing block is the sticky
// subnav ABOVE the scroller, and any of those properties moves it INSIDE, where
// `overflow-x: auto` clips it at every width.
//
// 🔴 This is the only assertion that can see that. Measured 2026-09-15 by wrapping the button
// in a sticky element: the dropdown's rect was byte-identical with and without the fault —
// same x, y, width, height — while its item went from hit-testable to not. So a rect
// assertion and a `textContent` assertion BOTH pass against the broken state. Until this test
// existed, adding `relative` to the row passed both tiers green.
await renderAtViewport(
<Shell>
<HomeTabs />
</Shell>,
{ width: 1136, height: 800 }
);
await nextLayout();
const row = tabRow();
const hit = (el: Element) => {
const r = el.getBoundingClientRect();
return el.contains(document.elementFromPoint(r.left + r.width / 2, r.top + r.height / 2));
};
// The original report was that MORE ITSELF could not be clicked, so start there. At 1136 it
// sits past the row's right edge and is NOT hit-testable — that is the design Justin chose
// (More scrolls off rather than pinning), so the invariant is "reachable BY SCROLLING", which
// is exactly what the defect denied. Assert both halves: unreachable before, reachable after.
// The premise, so a stale fixture reads as a stale fixture: More is off the end because the
// row overflows, not because it vanished.
expect(row.scrollWidth).toBeGreaterThan(row.clientWidth);
expect(moreButton().getBoundingClientRect().left).toBeGreaterThan(
row.getBoundingClientRect().right
);
expect(hit(moreButton())).toBe(false);
row.scrollLeft = row.scrollWidth;
await nextLayout();
expect(hit(moreButton())).toBe(true);
moreButton().click();
await nextLayout();
await nextLayout();
const dropdown = document.querySelector('.mantine-Menu-dropdown');
if (!dropdown) throw new Error('More menu did not open — nothing to hit-test');
const item = dropdown.querySelector('a, button');
if (!item) throw new Error('More menu opened with no items — check the feature-flag mock');
// 🔴 POSITIVE CONTROL for `Shell`'s nested provider. Everything below is about a dropdown
// rendered INSIDE the clipping box; if the harness portals it to `<body>` instead it escapes
// every clip and the hit-test passes against a planted `relative`. Not hypothetical — that is
// how the first version of this test failed. Delete the provider and this reddens first.
expect(tabRow().contains(dropdown)).toBe(true);
expect(item.getBoundingClientRect().width).toBeGreaterThan(0);
expect(hit(item)).toBe(true);
});
test('pays enough padding for the focus ring on the scroll axis', async () => {
await renderAtViewport(
<Shell>
<HomeTabs />
</Shell>,
{ width: 1136, height: 800 }
);
await nextLayout();
const row = tabRow();
const clip = row.getBoundingClientRect();
// `overflow-y` cannot be `visible` beside `overflow-x: auto`, so the row clips on both axes,
// and an outline contributes no scrollable overflow — a clipped ring cannot be scrolled into
// view. The end children are the ones at risk.
row.scrollLeft = 0;
await nextLayout();
const first = row.children[0] as HTMLElement;
const firstRect = first.getBoundingClientRect();
const firstInk = ringInk(first);
// 🔴 Before the containment checks, and load-bearing: at ink 0 the inflated rect IS the rect,
// so every containment assertion compares a box against itself. Do not delete as redundant.
expect(firstInk).toBeGreaterThan(0);
expect(firstRect.left - firstInk).toBeGreaterThanOrEqual(clip.left);
// HORIZONTAL ONLY, deliberately — the why is on the row itself in `HomeContentToggle.tsx`.
// This is the axis that scrolls, and an outline contributes no scrollable overflow, so a ring
// clipped here is unreachable outright rather than merely cut.
row.scrollLeft = row.scrollWidth;
await nextLayout();
const last = row.children[row.children.length - 1] as HTMLElement;
// By identity, not by index. Under this mock `more` resolves to exactly `['bounties']`; drop
// that flag later and the More button never renders, the last child silently becomes the Shop
// pill, and every assertion here still passes having stopped measuring the control the fix is
// for. `data-testid` would be stripped from production builds, so match the label instead.
expect(last.textContent).toContain('More');
const lastInk = ringInk(last);
// Same guard as `firstInk`, and for the same reason. `last` is the More menu's trigger, whose
// focus goes through Mantine's ref forwarding and is the likeliest of the two to be rewrapped
// later — at ink 0 the assertion below degenerates into one test 2 already makes.
expect(lastInk).toBeGreaterThan(0);
// The invariant stated directly: the padding is at least the ink, on each side. An earlier
// version compared the last child's right edge after `scrollLeft = scrollWidth`, which needed
// a 1px tolerance — `scrollLeft` clamps to an integer while the content width is fractional,
// leaving a sub-pixel remainder — and that tolerance silently accepted an underpay of up to
// 0.7px. It was also calibrated at a row width production never renders, since `flex-1` is
// inert under this file's block-level wrapper. Reading the padding has neither problem and
// reddens on `px-[3.5px]` as well as on `px-0`.
const padding = getComputedStyle(row);
expect(parseFloat(padding.paddingLeft)).toBeGreaterThanOrEqual(firstInk);
expect(parseFloat(padding.paddingRight)).toBeGreaterThanOrEqual(lastInk);
});
});
@@ -41,14 +41,19 @@ export function SubNavSettingsButton({
if (!withHomepageOption)
return (
<LegacyActionIcon
size="md"
size={32}
radius="xl"
variant="subtle"
color="gray"
// `bright`, not `LegacyActionIcon`'s grey. Measured 2026-09-15 in DARK: the grey put this
// at rgb(222,226,230) beside the nav's other icons at rgb(254,254,254), which at a matched
// 16px box reads as a smaller icon rather than a dimmer one. `bright` is the theme's
// black/white pair, the same one the pills use, so it tracks them in both schemes.
c="var(--mantine-color-bright)"
className={className}
aria-label="Customize navigation"
onClick={() => openSubNavSettings()}
>
<IconSettings />
<IconSettings size={16} />
</LegacyActionIcon>
);
@@ -56,13 +61,14 @@ export function SubNavSettingsButton({
<Menu position="bottom-end" withinPortal>
<Menu.Target>
<LegacyActionIcon
size="md"
size={32}
radius="xl"
variant="subtle"
color="gray"
c="var(--mantine-color-bright)"
className={className}
aria-label="Customize page and navigation"
>
<IconSettings />
<IconSettings size={16} />
</LegacyActionIcon>
</Menu.Target>
<Menu.Dropdown>
@@ -0,0 +1,191 @@
import { describe, expect, it } from 'vitest';
import { readFileSync } from 'fs';
import path from 'path';
import { stripComments } from '../../../../test/strip-comments';
/**
* SOURCE GATE the sub nav's tab row must stay horizontally scrollable at EVERY width.
*
* This is the cheap half of a pair. It fails in ~0.3s and names the CAUSE (a class), which the
* geometry test beside it cannot. `HomeTabs.geometry.test.tsx` asserts the CONSEQUENCE in a real
* browser with the real cascade and is immune to how the classes are spelled. Keep both; if only
* one can survive, keep the geometry one.
*
* WHY. The row is content-width and never collapses: `useResolvedNav` derives the bar/More split
* from the user's saved config, not the viewport, so the row is a fixed ~1334px signed in
* (measured 2026-09-15) at every width from 1024 to 1400. It used to carry
* `@md:overflow-visible`, which overrode
* BOTH axes above the `md` container breakpoint (1024px) and removed the row's only escape.
* Measured 2026-09-14 at 1136px signed in: right edge 1334, scrollable overflow 0, and neither
* "Shop" nor "More" hit-testable, on every route. `document.scrollWidth` stayed equal to the
* viewport throughout an ancestor `overflow-hidden` absorbs it so a page-level overflow check
* cannot see this bug.
*
* WHY THE PADDING IS PART OF THE SAME DECISION. `overflow-y` cannot be `visible` while
* `overflow-x` is `auto` (CSS Overflow 3: it computes to `auto`), so a horizontally scrollable row
* necessarily clips on both axes. Mantine's focus ring is `outline: 2px` at `outline-offset: 2px`
* 4px of ink OUTSIDE the border box on all four sides and an outline contributes no scrollable
* overflow, so a clipped ring cannot be scrolled into view. Measured at 1136: without horizontal
* padding the first pill's ring fell 4px outside the clip box on the left and the More button's
* 4px outside on the right. Hence horizontal padding, sized from the ink rather than from the 2px
* of slack a 32px pill happens to have in its row.
*
* NOT `TwScrollX`, the repo's shared scrollable strip that covers six other rows. Its outer
* wrapper is `relative overflow-hidden` (`TwScrollX.tsx:43`) exactly the pair the below
* forbids so it would not merely overlay this row's controls, it would DELETE the More
* dropdown at every width, and that is not fixable without changing `TwScrollX` for all six of
* its consumers. Secondarily it paints a scroll arrow at `absolute inset-y-0 right-0 z-10` with
* no prop to suppress it, over the More trigger this fix exists to make clickable, and hardcodes
* `scrollbar-none`. (An earlier version of this paragraph also said it "pays no padding" that
* is wrong, `className` reaches its inner scroller.)
*
* DO NOT GIVE THIS ROW `relative`, `position: sticky`, `@container`, a `transform`, a `filter`,
* or `will-change`. Only this row: `SubNav2` above it is unpositioned and unclipped, so neither a
* position nor an overflow there reaches the dropdown on its own it takes the PAIR on one
* element, and nothing checks for that on `SubNav2`.
* The More menu's dropdown is not portalled (the theme defaults `Popover.withinPortal` to false and
* Mantine's `Menu` does not override it), so it renders inside this row and escapes the clip only
* because it is absolutely positioned and its containing block is the sticky subnav ABOVE the
* scroller. Any of those makes the clipping box its containing block instead, and the menu
* disappears at every width.
*
* Measured 2026-09-15, wrapping the More button in a `position: sticky` element: the dropdown's
* rect was IDENTICAL with and without the fault same x, y, width, height while its item went
* from `hitTestable: true` to `false`. A rect assertion and a `textContent` assertion both pass
* against the broken state. Only hit-testing separates them, which is why the geometry file beside
* this one opens the menu and hit-tests an item see the test named for this trap. Measured:
* before that test existed, planting `relative` on the row passed both tiers green.
*
* TO WHOEVER IS ABOUT TO DELETE THIS: re-adding any width-conditional override of this row's
* overflow puts the tabs back out of reach between 1024px and ~1342px roughly a third of desktop
* widths, on every route. If the row should collapse by width instead, that reverses PR #4591's
* deliberate removal of width-driven placement and is a product decision, not a CSS one.
*/
const COMPONENT = path.resolve(__dirname, '../HomeContentToggle.tsx');
/** The row wrapping the nav pills — matched on the pair of classes only it carries. */
const ROW = /<div\s+className="([^"]*\bgap-1\b[^"]*\boverflow-x-auto\b[^"]*)"/g;
function tabRowClassName(source: string): string {
const matches = [...source.matchAll(ROW)];
if (matches.length !== 1) {
throw new Error(
`Expected exactly one tab row in HomeContentToggle.tsx, found ${matches.length}. If the row ` +
'was refactored or a second scroller was added above it, re-point this guard rather than ' +
'deleting it — see the block comment above.'
);
}
return matches[0][1];
}
describe('sub nav tab row', () => {
// Comments stripped first: without this, deleting the live element and leaving a commented-out
// copy leaves exactly one match, and every assertion below passes against a component that no
// longer renders the row. Strings are KEPT — `className` is one.
const source = stripComments(readFileSync(COMPONENT, 'utf8'));
it('scrolls horizontally with no width-conditional overflow override', () => {
const className = tabRowClassName(source);
expect(className).toContain('overflow-x-auto');
// Whitelist the tokens rather than enumerate variant prefixes. A pattern like
// /@(max-)?\w+:overflow-/ sees only CONTAINER variants, so the viewport spelling
// `md:overflow-visible` — which switches at the same 1024px, since `screens` and `containers`
// both come from breakpoints.json — restores the identical dead band and passes.
//
// Match `overflow` anywhere in the token, not the substring `overflow-`: the arbitrary-property
// spelling `lg:[overflow:visible]` contains no hyphen after the word and would otherwise be
// invisible here. Sorted, because the pair's order carries no meaning.
const overflow = className.split(/\s+/).filter((token) => /overflow/.test(token));
expect(overflow.sort()).toEqual(['overflow-x-auto', 'overflow-y-hidden']);
});
it('can shrink below its content width so the feed filters stay on one line', () => {
// `SubNav2` wraps, and a wrapping container places items at their flex BASIS before it shrinks
// anything — so at `basis: auto` this row's content width pushes the filters to a second line.
// Justin asked for one line (2026-09-15). The geometry file beside this one measures the
// consequence and reddens by a whole line height when this token goes; this tier names it.
expect(tabRowClassName(source).split(/\s+/)).toContain('flex-1');
});
it('carries nothing that would become the containing block of the More dropdown', () => {
// The ⚠️ above, as a check rather than as prose. The geometry file beside this one catches the
// same fault by hit-testing an open menu; this tier names the cause in 0.3s.
//
// A WHITELIST, for the same reason the overflow assertion above is one, and this was a denylist
// first: the set of ways to become a containing block is open-ended and a denylist leaked four
// ways at once — `[position:relative]` survives any variant-prefix strip, `scale-95` /
// `rotate-3` / `translate-x-1` emit a `transform` without the word in the token, `blur-sm` and
// `grayscale` emit a `filter`, and `contain-layout` / `content-visibility-*` apply the same
// layout containment that makes `@container` unsafe here. Enumerating what IS allowed closes
// all four and makes every future token a deliberate decision.
// The padding token is normalised: the padding test below pins only that it is unconditional
// and nonzero, and its magnitude is the geometry file's job. Pinning the literal here would
// redden a containing-block test for a ring-width change.
const tokens = tabRowClassName(source)
.split(/\s+/)
.map((t) => t.replace(/^px-\[.+\]$/, 'px-[…]'))
.sort();
expect(tokens).toEqual([
'dark:text-white',
'flex',
'flex-1',
'gap-1',
'items-center',
'overflow-x-auto',
'overflow-y-hidden',
'px-[…]',
'text-black',
]);
});
it('keeps the More button at pill height, in a tier that can fail a PR check', () => {
// The geometry tier measures this properly — More against a pill AND against 32 — but that
// project is `continue-on-error` on pull requests (`.github/workflows/lint.yml`), so it informs
// rather than gates. This is the enforced half: 0.3s, in a tier a red actually stops.
//
// Deleting `h-8` here sends the button back to Mantine's `size="sm"` 36px and grows the bar 4px
// on every page. Order-insensitive, so shuffling the class list is not a failure.
const more = /clsx\('([^']*)',\s*classes\.moreButton\)/.exec(source);
if (!more)
throw new Error(
'More button className not found in HomeContentToggle.tsx. If it was refactored, re-point ' +
'this guard rather than deleting it — the geometry tier cannot fail a PR check.'
);
expect(more[1].split(/\s+/)).toContain('h-8');
});
it('pays unconditional horizontal padding so the focus ring is not clipped on the scroll axis', () => {
const className = tabRowClassName(source);
const padding = className.split(/\s+/).filter((token) => /(^|:)-?p[xytrbl]?-/.test(token));
// Unconditional: a `md:`/`@md:`-prefixed token leaves the row unpadded below the breakpoint.
expect(padding.every((token) => !token.includes(':'))).toBe(true);
// Nonzero on BOTH axes. `py-0` and `p-0` are the exact states this forbids, and both satisfy
// a naive /\bp[xy]-\d/. Every side spelling counts, so `pt-1 pb-1 px-1` — behaviourally
// identical to `p-1` — is accepted rather than rejected for being written out.
const SIDES: Record<string, string[]> = {
'': ['x', 'y'],
x: ['x'],
y: ['y'],
t: ['y'],
b: ['y'],
l: ['x'],
r: ['x'],
};
const axes = padding.flatMap((token) => {
// An arbitrary value — the row ships a `px-[calc(…)]` — because the ring's ink
// is part-px and part-rem and a pure-rem scale only matches it at a 16px root. Its magnitude
// is the geometry test's job; all this tier can say is that it is not a zero literal.
const arbitrary = /^p([xytrbl]?)-\[(.+)\]$/.exec(token);
if (arbitrary) return /^0[a-z]*$/.test(arbitrary[2]) ? [] : SIDES[arbitrary[1]];
const [, side, value] = /^p([xytrbl]?)-([0-9.]+)$/.exec(token) ?? [];
return value && Number(value) > 0 ? SIDES[side] : [];
});
// X ONLY, on purpose — the why is on the row itself in `HomeContentToggle.tsx`. This is the
// axis that scrolls, and an outline contributes no scrollable overflow, so a ring clipped here
// cannot be reached at all.
expect([...new Set(axes)]).toContain('x');
});
});
+5 -2
View File
@@ -528,8 +528,11 @@ function AppPage(props: PageProps) {
* named two files. That was a SAMPLE generalised into an enumeration: it
* missed a third supporting case and all three counter-examples. Enumerate
* before claiming a convention.
* 3. It is ~52px (h-8 pills + `py-1` + `mb-3`) of the vertical budget this
* route is tightest on; a 375×667 phone leaves the page ~386px total.
* 3. It costs ~52px of vertical budget (measured 2026-09-15: a 32px row plus
* `SubNav2`'s `py-1` and `SubNav`'s `mb-3`; 4px more on routes whose feed filters are
* `h-9`, such as `/comics`), on the route the site is tightest on.
* `subNav: null` means the band never renders here, so that is a cost
* avoided rather than one already counted against this page's height.
* 4. Two chrome bars over a THIRD-PARTY app reads badly the route already
* renders `AppBlockChrome` (the "Apps / <name>" breadcrumb).
*