fix(public): net the live site against WCAG 2.2 tap-target drift without blocking PRs - #107
fix(public): net the live site against WCAG 2.2 tap-target drift without blocking PRs#107nish3451 wants to merge 9 commits into
Conversation
…out blocking PRs PR #22 (footer) and #25 (in-content) brought every link in public/styles.css up to the WCAG 2.2 SC 2.5.8 24px minimum, but the backlog item keeps re-opening because the live site still serves the pre-fix stylesheet (June-20 bundle: 874 lines vs 901 in main, missing all five tap-target rules), so rendered in-content links are ~17px. PR #66 wired a live guard into the blocking npm test/ci chain, but PR #84 established that the blocking chains must stay green on repo state alone. Land the detector the same way #84 landed the soft-404 net: - scripts/check-public-live-tap-targets.mjs: fetches the deployed https://tinystudio.in/styles.css and re-asserts the five link rules the local suite requires (.top-nav a, .plain-list a, .product-links a, .rail-item strong a, .footer-links a: block-level box, min-height 24px, >= 4px vertical padding). A non-2xx response or a stylesheet that misses a rule fails loudly (the CodeRabbit P1 on #66); only a network-level failure skips, and SKIP_LIVE_CHECKS=1 is honored like the sibling live checks. - npm run site:check-live entry point now runs both live checks - .github/workflows/live-site-check.yml gains a nightly + manual-dispatch live-tap-targets job Verify: node --check clean; against the current live stylesheet the check reports 11 failures and exits 1 (the point of the net: public/ is compliant, the deployment is not - the red run is the signal to re-deploy from origin/main); npm test stays green (blocking chain untouched); git diff --check clean.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. 📝 WalkthroughWalkthroughThe pull request adds a deployed stylesheet checker for five WCAG 2.2 tap-target selectors. It adds retry and skip handling for network failures, validates checker wiring, and runs the check from the npm script and nightly workflow. ChangesLive tap-target validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change adds a non-blocking live-site accessibility drift check while keeping repository checks green; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Workflow as live-site-check.yml
participant Checker as check-public-live-tap-targets.mjs
participant Site as tinystudio.in
Workflow->>Checker: run live tap-target check
Checker->>Site: fetch styles.css
Site-->>Checker: return stylesheet
Checker-->>Workflow: report validation result
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 809042570d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const match = css.match(new RegExp(`${pattern}\\s*\\{([^}]*)\\}`)) | ||
| return match ? match[1] : null |
There was a problem hiding this comment.
When a later or more-specific rule overrides one of these selectors—for example, a mobile media query sets .top-nav a { min-height: 0; padding: 0; }—String.match still returns the first compliant block, so the nightly check reports success while the rendered tap target is below 24px. Since this detector is intended to catch deployed CSS drift, it should account for the cascade (ideally by checking computed styles at representative viewports) rather than accepting the first textual occurrence.
Useful? React with 👍 / 👎.
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4123145c29
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } catch (err) { | ||
| console.log(` ok skipped: ${LIVE_CSS_URL} unreachable (${err?.cause?.code ?? err?.name ?? "network error"}) - no tap-target assertions run`) | ||
| } |
There was a problem hiding this comment.
When /styles.css alone times out, resets, or returns a truncated body while the HTML endpoints remain reachable, this catch path reports an ok skipped result and the job exits successfully after the wiring checks. The separate live-soft-404 job in .github/workflows/live-site-check.yml never requests this asset, so the nightly workflow can stay green even though browsers cannot load the stylesheet and the tap-target guarantees are absent. Treat an exhausted stylesheet fetch as a monitoring failure; offline callers already have the explicit SKIP_LIVE_CHECKS=1 escape hatch.
Useful? React with 👍 / 👎.
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea5a636443
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (rule) { | ||
| ok(/display:\s*(inline-block|inline-flex|block)/.test(rule), `live ${selector} links are block-level boxes (hit area covers the line box)`) | ||
| ok(!/display:\s*inline\s*;/.test(rule), `live ${selector} links are not plain inline boxes`) | ||
| ok(/min-height:\s*24px/.test(rule), `live ${selector} links declare min-height: 24px`) |
There was a problem hiding this comment.
Add a width assertion as well as min-height: when a short label or icon link is added under any of these selectors, the existing HTML coverage test still accepts it and this live check remains green even though the rendered target can be narrower than 24px because every required rule has zero horizontal padding and no min-width. The guard therefore does not enforce the stated 24-by-24 target minimum for all content it permits.
Useful? React with 👍 / 👎.
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/live-site-check.yml:
- Line 43: Update the actions/checkout step to set persist-credentials to false,
ensuring the GITHUB_TOKEN is not retained in .git/config after checkout.
In `@scripts/check-public-live-tap-targets.mjs`:
- Around line 67-87: The targetRuleOf/assert24pxRule validation currently only
reads the first direct selector match, missing grouped selectors and later
source-order overrides. Replace targetRuleOf with stylesheet rule parsing that
collects declarations from every rule containing the requested selector, then
evaluate the final effective declarations after applying source-order overrides;
add coverage for grouped selectors and later overriding rules.
🪄 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: bc89e226-73b6-45ed-95f6-9883f5d7cbfd
📒 Files selected for processing (3)
.github/workflows/live-site-check.ymlpackage.jsonscripts/check-public-live-tap-targets.mjs
| live-tap-targets: | ||
| runs-on: [self-hosted, linux, x64, vps-verify] | ||
| steps: | ||
| - uses: actions/checkout@v7 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Confirm this job has no later Git operation that requires checkout credentials.
sed -n '40,60p' .github/workflows/live-site-check.yml
rg -n -C 2 '\bgit\b|actions/checkout' .github/workflows/live-site-check.ymlRepository: nish3451/tinystudio-in
Length of output: 796
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- workflow ---'
cat -n .github/workflows/live-site-check.yml
printf '%s\n' '--- checkout and Git references in workflows ---'
rg -n -C 3 'actions/checkout|persist-credentials|\bgit\b|GITHUB_TOKEN|github\.token' .github/workflowsRepository: nish3451/tinystudio-in
Length of output: 4485
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for file in scripts/check-public-live-soft-404.mjs scripts/check-public-live-tap-targets.mjs; do
if [ -f "$file" ]; then
printf '%s\n' "--- $file ---"
rg -n -C 2 'child_process|exec|spawn|git|GITHUB_TOKEN|github\.token' "$file" || true
fi
done
printf '%s\n' '--- package scripts and Git dependencies ---'
rg -n -C 2 '"(prepare|preinstall|install|postinstall|ci)"|git[[:space:]]' package.json package-lock.json 2>/dev/null || trueRepository: nish3451/tinystudio-in
Length of output: 3367
Disable checkout credential persistence.
This self-hosted job performs no Git operations after checkout. Set persist-credentials: false so the GITHUB_TOKEN is not retained in .git/config during later steps.
🧰 Tools
🪛 zizmor (1.29.0)
[warning] 43-43: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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 @.github/workflows/live-site-check.yml at line 43, Update the
actions/checkout step to set persist-credentials to false, ensuring the
GITHUB_TOKEN is not retained in .git/config after checkout.
Source: Linters/SAST tools
| const targetRuleOf = (css, selector) => { | ||
| const esc = (s) => s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&") | ||
| const pattern = selector | ||
| .split(",") | ||
| .map((s) => esc(s.trim())) | ||
| .join("\\s*,\\s*") | ||
| const match = css.match(new RegExp(`${pattern}\\s*\\{([^}]*)\\}`)) | ||
| return match ? match[1] : null | ||
| } | ||
|
|
||
| const assert24pxRule = (selector, rule) => { | ||
| ok(rule !== null, `live styles.css has a ${selector} rule`) | ||
| if (rule) { | ||
| ok(/display:\s*(inline-block|inline-flex|block)/.test(rule), `live ${selector} links are block-level boxes (hit area covers the line box)`) | ||
| ok(!/display:\s*inline\s*;/.test(rule), `live ${selector} links are not plain inline boxes`) | ||
| ok(/min-height:\s*24px/.test(rule), `live ${selector} links declare min-height: 24px`) | ||
| const paddingMatch = rule.match(/padding:\s*([^;]+)/) | ||
| ok(paddingMatch !== null, `live ${selector} links declare vertical padding`) | ||
| if (paddingMatch) { | ||
| const vertical = parseFloat(paddingMatch[1].trim().split(/\s+/)[0]) | ||
| ok(vertical >= 4, `live ${selector} vertical padding is at least 4px (${vertical}px), so 16px text + padding >= 24px`) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Evaluate CSS rules after selector grouping and source-order overrides.
Line 73 only matches a selector directly before { and returns its first match. A grouped rule such as .top-nav a, .plain-list a { ... } does not match. A later rule for the same selector can override a compliant earlier rule and still pass this check.
Parse stylesheet rules, collect every matching selector, and evaluate the final effective declarations. Add cases for grouped selectors and later overrides.
🤖 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 `@scripts/check-public-live-tap-targets.mjs` around lines 67 - 87, The
targetRuleOf/assert24pxRule validation currently only reads the first direct
selector match, missing grouped selectors and later source-order overrides.
Replace targetRuleOf with stylesheet rule parsing that collects declarations
from every rule containing the requested selector, then evaluate the final
effective declarations after applying source-order overrides; add coverage for
grouped selectors and later overriding rules.
…link-targets-live-net-20260812-rebase
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
1 participant
What
The backlog item "In-content links stay below the WCAG 2.2 24px tap-target minimum" is fixed in the repo (PR #22 raised
.footer-links a, PR #25 raised.top-nav a,.plain-list a,.product-links a,.rail-item strong a) but keeps re-opening because the live site still serves the pre-fix stylesheet (the June-20 bundle). Measured today:public/(main, incl. #22+#25)Change
This lands the drift detector without putting the live site in the blocking chain — PR #84 set the convention that
npm test/npm run cistay green on repo state alone:scripts/check-public-live-tap-targets.mjs— fetches the deployedhttps://tinystudio.in/styles.cssand re-asserts the same five link rules the local suite (test-public-link-targets.mjs) requires:.top-nav a,.plain-list a,.product-links a,.rail-item strong a,.footer-links amust each be a block-level box withmin-height: 24pxand >= 4px vertical padding.SKIP_LIVE_CHECKS=1honored like the sibling live checks.npm run site:check-livenow runs the soft-404 and tap-target live checks..github/workflows/live-site-check.ymlgains a nightly + manual-dispatchlive-tap-targetsjob.Supersedes PR #66
PR #66 wired this same guard into the blocking
npm test/npm run cichain (its repo-checks are red and it conflicts with main). Per the PR #84 convention the blocking chain must stay green on repo state alone, so #66 cannot merge as-is. This PR is the #84-shaped replacement (detector +site:check-live+ nightly net). Close #66 if this lands.Expected: nightly live-site-check is RED until the deployment is refreshed
Against today's live sheet this check reports 11 failures and exits 1 — that failure is the point:
public/is compliant, the deployment is not. The red nightly run is the loud signal to re-deploy fromorigin/main. The deploy lane (deploy-public-site.yml+publish-public-site.mjs) is in place but fails closed until a Cloudflare Pages-scopedCLOUDFLARE_API_TOKENis provisioned (one-time dashboard step documented in the script; tracked in PR #85) — the live refresh is the external dependency that unblocks the green net.Verify
node --check scripts/check-public-live-tap-targets.mjs— cleannode scripts/check-public-live-tap-targets.mjs— 19 checks, 11 failures vs the current live sheet, exit 1 (expected drift signal)npm test— exit 0, blocking chain untouched and greengit diff --check— cleanSummary by CodeRabbit
New Features
Chores