wip(#430): wrap the two admin tables in a padded sh-well — rescued from a local-only branch - #503
Closed
TortoiseWolfe wants to merge 1 commit into
Closed
wip(#430): wrap the two admin tables in a padded sh-well — rescued from a local-only branch#503TortoiseWolfe wants to merge 1 commit into
TortoiseWolfe wants to merge 1 commit into
Conversation
…RIFIED The deferred half of #430. Both scrollers were a bare `<div className="overflow-x-auto">` at AdminPaymentPanel.tsx:216 and AdminMessagingOverview.tsx:173 — the line numbers in the ticket were accurate, unlike #373's, which drifted four times. `sh-well` paints an inset shadow BELOW its children, so it needs a padded parent: putting it on the `overflow-x-auto` div itself would clip the shadow inside the scroller and hide it under the table. So the scroller is WRAPPED by `sh-well rounded-lg p-2` rather than annotated. WHAT IS NOT VERIFIED, AND WHY THIS IS NOT PUSHED. I could not confirm the wells actually PAINT. Both routes are auth-gated and the E2E storage state at tests/e2e/fixtures/storage-state-auth.json is stale — with it loaded, /admin/payments redirects to the home page, so the components never render and a `.sh-well` query returns 0. That matters more than usual here: these two items were DEFERRED from #430 precisely because a sh-well with the wrong parent is a visual no-op, and "a class being present proves nothing about whether it renders" has been true four times in this session. Type-check clean and 35 component tests passing do not answer the question that was actually asked. Hit my own documented trap on the way: a `{/* comment */}` placed between `{cond ? (` and its element is two root nodes and a syntax error. The comment now sits above the ternary. Refs #430 — needs an authenticated visual check before this ships.
This was referenced Aug 2, 2026
TortoiseWolfe
added a commit
that referenced
this pull request
Aug 3, 2026
… it found (#509) * wip(#430): wrap the two admin tables in a padded sh-well — NOT YET VERIFIED The deferred half of #430. Both scrollers were a bare `<div className="overflow-x-auto">` at AdminPaymentPanel.tsx:216 and AdminMessagingOverview.tsx:173 — the line numbers in the ticket were accurate, unlike #373's, which drifted four times. `sh-well` paints an inset shadow BELOW its children, so it needs a padded parent: putting it on the `overflow-x-auto` div itself would clip the shadow inside the scroller and hide it under the table. So the scroller is WRAPPED by `sh-well rounded-lg p-2` rather than annotated. WHAT IS NOT VERIFIED, AND WHY THIS IS NOT PUSHED. I could not confirm the wells actually PAINT. Both routes are auth-gated and the E2E storage state at tests/e2e/fixtures/storage-state-auth.json is stale — with it loaded, /admin/payments redirects to the home page, so the components never render and a `.sh-well` query returns 0. That matters more than usual here: these two items were DEFERRED from #430 precisely because a sh-well with the wrong parent is a visual no-op, and "a class being present proves nothing about whether it renders" has been true four times in this session. Type-check clean and 35 component tests passing do not answer the question that was actually asked. Hit my own documented trap on the way: a `{/* comment */}` placed between `{cond ? (` and its element is two root nodes and a syntax error. The comment now sits above the ternary. Refs #430 — needs an authenticated visual check before this ships. * feat(#454, #430): an is_admin E2E fixture, and the two AAA violations it found No E2E run had ever rendered an admin page. `AdminGate.tsx:81` bounces an authenticated non-admin to `/`, so three separate things were measuring the wrong page or nothing: the AAA sweep reported six `/admin*` routes as swept while measuring the HOME page, the admin table wells could not be verified (draft PR #503 sat 17/17 green and unmergeable), and the landmark gate had to exclude those six outright. `seedIsolatedAdmin()` promotes a throwaway user through `user_profiles.is_admin` — the single authority since #240, read live by the `is_admin()` SECURITY DEFINER RPC. One service-role UPDATE; no token refresh and no auth-hook registration, because no live RLS policy reads the JWT claim. It VERIFIES ITSELF through the user's own session and throws otherwise. A silently-unpromoted user still renders — it renders the redirect — so every assertion downstream would measure the home page and pass. That is the defect #454 is about, and without the check the fixture built to fix it would reproduce it. Not the shared storage-state user, deliberately: `is_admin()` gates 25 policies across 9 tables, so promoting it would give every existing spec cross-user reads and row-count assertions would keep passing while measuring something else. Unblinding the sweep immediately found two real AAA violations on /admin/email, both invisible until now: .stat-title #484f58 on #ebe5dd 6.62:1 (3 elements) text-base-content/70 #5f656d on #f5f0eb 5.19:1 (2 elements) The first is the sharper one. globals.css ALREADY corrected `.stat-title`, raising alpha to 80% and documenting "~7.1:1 on light". Measured: 6.62. 7.1 was computed against base-100; a `.stats` block sits on base-200 — exactly the trap #462 records, where /80 measures 7.08 on base-100 and 6.4-6.5 on base-200. The alpha was right for the surface it was checked on and wrong for the one it renders on. Now solid, like #411, #425 and #495 before it. Coverage: landmark sweep 36 -> 42 paths (exclusions 9 -> 3); contrast sweep 36 -> 42 paths x 2 themes, 85 passing. Mutation-tested against the original defect: with the sh-well wrapper reverted, admin-depth reports `0/1 scrollers welled` and fails by name. The spec asserts the INVARIANT (every scroller is welled) rather than the element, because /admin/messaging's well is behind `top_senders.length > 0` — a data-dependent element is how #495 looked like a flake. A floor stops it passing vacuously. Orphan sweep in auth.setup.ts pages through listUsers (#197 truncates at 50) and deletes throwaway admins that survived a previous run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(#430): the shared admin table was never welled, and the spec could not see it CI found both defects on the first run that could reach an admin page as an admin — which is the entire argument for #454's fixture. 1. #430's fix wrapped the two HAND-ROLLED scrollers in AdminPaymentPanel and AdminMessagingOverview and missed `AdminDataTable`, the shared component several admin surfaces render. It only appears when there is data, so a local project with no conversations never rendered it. CI's shared project has data, and the spec reported it by name: /admin/messaging: 1 scroller(s) are not wrapped in .sh-well unwelled parent: "flex flex-col gap-3 mt-8" Wrapping it in AdminDataTable fixes every consumer at once. 2. The spec itself was timing-dependent, and that is why CI called it FLAKY rather than failed. `page.evaluate` does not retry the way a web-first assertion does (#396, instance 5) — it snapshots the DOM once. The admin panels render a `loading-spinner` until their RPCs return, so counting too early sees zero scrollers and passes on nothing. Measured: a 3.5s settle read 0/0 on /admin/payments, which had read 1/1 moments earlier. It now waits for `.loading-spinner` to reach count 0 — a signal, not a sleep — and re-reads through `expect.poll`. /admin/messaging goes from 0/0 to 1/1 locally as a result: the scroller was always there, the spec just measured before it existed. The flake was the tell. A test that passes when the data is absent and fails when it is present is not flaky; it is a defect plus a vacuous pass, which is the shape #495 wore. No mutation test needed for the AdminDataTable fix: CI already ran it against the original defect and failed by name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: TurtleWolfe <TurtleWolfe@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
|
Superseded by #509, which merged this commit ( This PR was the worked example of the problem #454 was about: 17/17 green and unmergeable, because the E2E user was not an admin, #509's It also found what this PR had missed: |
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
2 participants
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.
Draft — pushed to preserve it, not to merge it. Refs #430.
This commit existed only on one local machine: not on
origin, not merged, on a branch 22 commits behindmain. #430's body already recorded that the fix lived only there. Rebased ontomain(clean, no conflicts) and pushed so it is not one disk failure away from gone.What it does: wraps — does not replace — the two admin table scrollers in a padded
sh-well.AdminPaymentPanel.tsx:216andAdminMessagingOverview.tsx:173, both still bareoverflow-x-autoonmain.Why it is a draft and not ready to merge — the commit says so itself (
NOT YET VERIFIED):AdminGate.tsx:81redirects an authenticated non-admin to/, and the E2E user is not an admin, so no gate has ever painted these wells. That is the same missingis_adminfixture that blocks Reskin the admin console: 6 routes via organisms/Admin* #430 and now The AAA contrast sweep reports six /admin routes as swept while measuring the home page — the gap is admin PRIVILEGE, not auth #454.sh-wellis box-shadow only (globals.css:347-349), so whetherbg-base-100belongs alongside it is a real behavioural question, not cosmetic.Do not merge on green CI alone — green here would mean the suite never reached the page.
🤖 Generated with Claude Code