fix(#502): the nav rows never missed on width — one row missed a height floor entirely - #804
Merged
Merged
Conversation
…ht floor entirely #502 asked for a decision between three options for widening nav rows to 44px. None of them applies: the premise is false, and re-measuring found a different real defect. THE ROWS ARE 144px WIDE. `mobile-touch-targets.spec.ts:209` has opened the menu and measured `.dropdown-content a, .dropdown-content button` on BOTH axes since #378, with a coverage floor of 13. Its own comment records the pre-fix measurement as `26x144` -- 26 tall, 144 wide. Height was the defect and #378 fixed it; width never was one. A `min-w-11` would be a no-op: the rows sit in `ul.menu.dropdown-content` at `w-40 sm:w-44` with `p-2`, so the content box is ~144px and the fixed parent caps growth anyway. The inherited numbers could not be reproduced and could not even be attributed: #457 described "nav `<a class="link">` rows", and no `class="link"` exists in GlobalNav.tsx at all. The file has changed three times since that measurement (08-06 pricing, 08-07 merge, 08-10 the v3 brand marks). WHAT RE-MEASURING ACTUALLY FOUND, and what this fixes: 1. ONE ROW HAD NO HEIGHT FLOOR AT ALL. The signed-in Messages link in the avatar menu carried a bare `flex items-center justify-between`, while its own twin in the mobile menu carried `${MENU_ITEM} justify-between`. DaisyUI renders `menu li > a` at 26px, so it shipped a 26px touch target against a 44px standard. It survived because NO GATE CAN SEE IT: the touch-target sweep measures `.dropdown-content a` with the menu open, but runs signed OUT, and that row only renders signed IN. 2. THE DESKTOP `Demos` MENU WAS MEASURED ON ONE AXIS. `mobile-touch-targets.spec.ts:185` checked height only -- the last nav surface in the file that did, while the Display popover three tests below already records why that is not enough ("'L' measured 44x42 when only the height had a floor"). GUARDED, because the defect was "a row forgot what all its siblings have" and no test file existed for this component at all: `scripts/__tests__/nav-rows-have-a-height-floor.test.js` reads every `<li>` row in GlobalNav.tsx and asserts a floor class, with a coverage floor of its own (17 rows found today) so it cannot pass vacuously if the markup moves. Both changes mutation-verified with the mutant confirmed in the file first: - restoring the bare className fails the guard at `GlobalNav.tsx:564` - constraining a Demos menuitem to `!w-4` fails the spec with `"Atlas": 20x44px (min 44x44)` -- height untouched, so only the new width assertion could have produced that NOT FIXED HERE, filed as #803: no gate measures the OPEN mobile menu against the viewport edge. #502's central worry -- that widening rows reintroduces 320px overflow -- could not be caught today, because the overflow sweep never opens the menu and a closed DaisyUI dropdown is `display:none`, so all 13 rows report 0x0 and are skipped. The only thing protecting 320px is a CSS cap that nothing asserts. Since nothing is being widened here, no protection is added -- but the gap is real and belongs in its own ticket. Verified: touch-target spec 13/13 against a real root build; type-check and lint clean; `pnpm test:scripts` 367 passed. Closes #502 Refs #378, #457, #803
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#502 asked for a decision between three ways to widen the nav rows to 44px. None of them
applies — the premise is false, and re-measuring (which the issue itself demanded) turned up a
different, real defect one row over.
Full write-up in #502, rewritten around what is actually true.
The rows are 144px wide
mobile-touch-targets.spec.ts:209has opened the menu and measured.dropdown-content a, .dropdown-content buttonon both axes since #378, with a coverage floorof 13. Its own comment records the pre-fix measurement:
26 tall, 144 wide. Height was the defect; #378 fixed it. Width never was one, and
min-w-11would be a no-op — the rows sit inul.menu.dropdown-contentatw-40 sm:w-44withp-2, so the content box is ~144px and the fixed parent caps growth regardless.The inherited numbers could not be reproduced or attributed: #457 described "nav
<a class="link">rows" and noclass="link"exists inGlobalNav.tsxat all. The file haschanged three times since that measurement (08-06 pricing, 08-07 merge, 08-10 the v3 brand marks).
What re-measuring did find
1. One nav row had no height floor at all. The signed-in Messages link in the avatar menu
carried a bare
flex items-center justify-between, while its own twin in the mobile menu carried${MENU_ITEM} justify-between. DaisyUI rendersmenu li > aat 26px, so it shipped a 26pxtouch target against a 44px standard.
It survived because no gate can see it. The touch-target sweep measures
.dropdown-content awith the menu open — but runs signed out, and that row only renders signed in.
2. The desktop
Demos ▾menu was measured on one axis.:185checkedbox.heightonly —the last nav surface in the file that did, while the Display popover three tests below already
records why that is not enough: "'L' measured 44x42 when only the height had a floor."
Guard
GlobalNav.tsxhad no test file of any kind, and the defect's shape was "one row forgot whatall its siblings have" — so the guard reads every
<li>row in the file and asserts a floorclass, with a coverage floor of its own (17 rows found today) so it cannot pass vacuously if the
markup moves.
Both changes mutation-verified, mutant confirmed in the file first:
GlobalNav.tsx:564!w-4"Atlas": 20x44px (min 44x44)The second is the one that matters:
min-h-11was untouched, so only the new width assertioncould have produced that failure.
Not fixed here — filed as #803
No gate measures the open mobile menu against the viewport edge. #502's central worry — that
widening rows reintroduces 320px overflow — could not be caught today:
mobile-horizontal-scroll.spec.tsnever opens the menu (goto→ resize → measure), anddisplay: none, so the<ul>and all 13 rows report0x0and aredropped by that sweep's
if (r.width === 0 && r.height === 0) continueguard.The only thing protecting 320px is the CSS cap
max-w-[calc(100vw-4rem)], and nothing asserts it.Nothing is being widened here, so no protection is added — but the gap is real and belongs in its
own ticket rather than a comment on a closed one.
Verified
touch-target spec 13/13 against a real root build · type-check clean · lint clean ·
pnpm test:scripts367 passedCloses #502
Refs #378, #457, #803