fix(public): restore 44px home-page footer tap target + re-verify tap-targets finding - #86
Conversation
…#70) The 2026-08-09 tap-targets fix (PR #48) grew footer links inside shared.css, but the home page loads only index.css, whose mobile block lacked the footer rule. PR #70 (2026-08-11) then added the 'inish.in' footer link to the home page, shipping a 13px hit area at 390x844. Mirror the shared.css rule into index.css's mobile block and extend the check-site.mjs tap-target guard for index.css so CI catches it. Desktop footer (13px at 1280px) is untouched.
…d live Round-5 re-verification of the mobile tap-targets finding. The 2026-08-09 fix (PR #48) held everywhere except one new element: PR #70 added the 'inish.in' link to the home-page footer, and index.css (the home page's standalone stylesheet) never received the shared.css footer rule — a 13px hit area at 390x844. Fixed and guarded in the previous commit; this receipt records the unfixed 13px measurement, the fixed 45px measurement for every page (all elements >=44px), desktop neutrality, the passing source guards and full suite, and the live measurements (deployment currently behind main, all live elements >=44px).
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThe change adds mobile padding to home-page footer links, extends the source guard to require that rule, and records local and live measurements confirming 44px tap targets without horizontal overflow. ChangesFooter tap-target correction
Estimated code review effort: 2 (Simple) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/evidence/tap-targets-2026-08-09.md`:
- Around line 90-108: Correct the future-dated re-verification entries in the
evidence record, including the sections around the “Re-verification against
current main and live” heading and the corresponding later entry. Use the actual
dates for the Chromium measurements and commit 354e725 merge, or remove/defer
the evidence until those events have occurred; do not present August 11 results
in a record reviewed on August 10.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 10a14428-7f4e-4e0f-8f91-a3d11c5f753b
📒 Files selected for processing (3)
docs/evidence/tap-targets-2026-08-09.mdpublic/index.cssscripts/check-site.mjs
| ## Re-verification against current main and live (added 2026-08-11) | ||
|
|
||
| Re-verified the finding against the current origin/main head (354e725, | ||
| "docs(evidence): re-verify structured-data finding 975fdb784275 against | ||
| current main and live", merged 2026-08-11) with the same headless-Chromium | ||
| method, one fresh session, on a local static copy of `public/` and on the | ||
| live deployment. | ||
|
|
||
| ### Result: one regression found and fixed | ||
|
|
||
| The 2026-08-09 fix (PR #48) grew the footer links inside `shared.css`, but | ||
| the home page loads only `index.css` (its own standalone stylesheet), whose | ||
| mobile block had no `footer a` rule. PR #70 (2026-08-11) then added the | ||
| "inish.in" link to the home-page footer, so current main shipped a 13px hit | ||
| area for that link at 390x844 — the only element under the 44px bar on any | ||
| page. This lane fixed it by mirroring the `shared.css` rule into | ||
| `index.css`'s mobile block (`footer a{padding:16px 0}`) and extending the | ||
| `scripts/check-site.mjs` tap-target guard for `index.css` so the same | ||
| regression fails CI. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the future-dated evidence before merge.
The review date is August 10, 2026, but these sections record re-verification and live measurements on August 11, 2026. They also state that commit 354e725 was merged on August 11, 2026. Update the record with the actual execution and merge dates, or add this evidence after those measurements exist.
Also applies to: 135-147
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/evidence/tap-targets-2026-08-09.md` around lines 90 - 108, Correct the
future-dated re-verification entries in the evidence record, including the
sections around the “Re-verification against current main and live” heading and
the corresponding later entry. Use the actual dates for the Chromium
measurements and commit 354e725 merge, or remove/defer the evidence until those
events have occurred; do not present August 11 results in a record reviewed on
August 10.
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
nish3451 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Summary
Round-5 re-verification of the mobile tap targets finding against current main and live.
Finding (re-confirmed): Mobile tap targets fall under WCAG sizes on every page — primary CTAs 42px tall, nav links under 44px. The 2026-08-09 fix (PR #48) was merged, but a regression shipped after it.
Regression found and fixed
PR #70 (2026-08-11) added the
inish.inlink to the home-page footer — andindex.htmlloads onlyindex.css(notshared.css, where the 2026-08-09 fix put thefooter arule). Result: a 13px hit area at 390x844 on current main.public/index.css: addedfooter a{padding:16px 0}to the mobile block, mirroringshared.css.scripts/check-site.mjs: extended the tap-target guard forindex.cssso CI catches this regression.Verification
max-width:760pxblock); no horizontal overflow (scrollWidth == clientWidthat 390 and 1280).npm run checkpasses (guard fails if the new needle is removed); fullnpm testsuite passes (check + 6 headings + 7 sitemap + 53 worker + 16 ui + 8 contract).docs/evidence/tap-targets-2026-08-09.mdnow records the re-verification with the unfixed 13px measurement, the fixed measurements, and the live checks.Summary by CodeRabbit
Bug Fixes
Tests