test(admin): assert an open drawer never widens the shared first column - #66
Conversation
PR #59 moved the accounts row drawer out of the pinned name cell into its own full-width <tr>, and tested that the pin survives an open drawer and that the drawer's own cell is not itself pinned. Neither of those states the property the move existed for. Table columns are shared, so the drawer's `flex: 1 1 100%` crew group set a min-content width for column 1 on every row while it lived in the name cell: 97px closed against 281.5px open at 320px, which is how the pinned cell came to cover 98% of the scroll region. "An open row drawer keeps the pin" cannot catch a regression here — a pinned cell is wholly on screen at either width, so it passes just as happily at 281.5px. clearOfPin notices only indirectly, once a control at the far right happens to fall under the widened pin. The row measured is deliberately not the row being opened: "opening one row widens the column for every row" is the actual failure, and a neighbour's name cell is where it shows. Falsified by re-rendering the drawer inside the name cell, which fails it at 435.45px against 97px.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Comment |
Review caught the doc comment asserting "97px closed against 281.5px open". Neither number appears anywhere in the repo. The figure actually on record — kept by #59 as a tombstone at globals.css:1913 for the rule that used to unpin column 1 — is 279.5px of a 286px region. Cite that instead; the repo convention is that a comment's measured figures are reproducible. Also softens the vacuity guard's comment. It rules out a drawer that renders nothing, which is worth keeping, but it is not evidence of how much width the drawer's content demands: a cell spanning every column is wider than any single column by table structure alone.
1 participant
What
Adds one Playwright assertion to
e2e/admin.spec.ts. Test-only — no source file is touched.Why
PR #59 moved the accounts row drawer out of the pinned name cell into its own full-width
<tr>, and covered it with two tests: the pin survives an open drawer, and the drawer's own cell is not itself pinned. Neither states the property the move existed for.Table columns are shared, so while the drawer lived in the name cell its
flex: 1 1 100%crew group set a min-content width for column 1 on every row — until the pinned cell took 279.5px of a 286px region, 98%, the figure #59 kept on record as a tombstone atglobals.css:1913.The existing coverage cannot catch a regression here:
an open row drawer keeps the pin, and is not itself pinnedusespinGeometry, which compares the cell against its own current width. It is width-agnostic by construction: a wider pinned cell is still wholly on screen and still passes.clearOfPinnotices only indirectly, once a control at the far right happens to fall under the widened pin — a threshold effect, not the claim.So this states it directly: opening a row must not move column 1 at all.
The mechanism it protects is real:
.log--densehas notable-layout: fixed(unlike.log--audit), so under auto layout acolSpancell's content genuinely can pull column 1's width up. That is the premise the whole fix depends on.Notes on the assertion
The row measured is deliberately not the row being opened. "Opening one row widens the column for every row" is the actual failure mode, and a neighbour's name cell is where that shows.
It carries a guard that the drawer's cell is wider than the closed column. That rules out the one genuinely vacuous case — a drawer rendering nothing — but is not evidence of how much width the drawer's content demands, since a cell spanning every column is wider than any single column by table structure alone. The comment says so explicitly.
Tolerance is
toBeCloseTo(closed, 0), matching how every other width claim in this file is stated (lines 202, 211, 376, 383).Falsification
Re-rendering the drawer inside the name cell — the exact regression this guards — makes it fail. Verbatim output:
Green again on restore, and 6/6 green under
--repeat-each=6. (Those pixel values are this falsification's own output, not a claim about the historical bug — the recorded figures for that are the 279.5/286 above.)Verification
npm run typechecknpm run lint@next/next/no-img-element)npm run format:checknpm testnpm run test:e2eReviewed by
code-reviewer: no blocking findings. Its two comment criticisms — an unreproduced pixel figure and an overclaiming vacuity-guard comment — are fixed in the second commit.🤖 Generated with Claude Code