Skip to content

fix: align camera reset accessible names with visible labels - #87

Merged
HemSoft merged 2 commits into
mainfrom
fix/reset-view-accessible-names
Oct 29, 2026
Merged

HemSoft merged 2 commits into
mainfrom
fix/reset-view-accessible-names

Conversation

@HemSoft

@HemSoft HemSoft commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

The camera reset controls display “Reset View”, but their accessible names omit “View”. Axe-core 4.14.0 flags both controls for label-content-name-mismatch. Start each accessible name with the full visible label and retain its chart context. Add a regression check and update the camera-control locator.

Validation

  • Full local validation passes: unit tests, coverage/risk, mutation qualification, Python/Markdown lint, actionlint, source/secret checks, security tests, pip-audit, npm audit, all 64 browser tests, performance budget tests, controlled failure probe, and full benchmark.
  • Updated both the browser-control test and the benchmark selector for the corrected accessible name.
  • With @axe-core/playwright 4.14.0 installed temporarily, all four accessibility and camera-control tests pass: npm run fixture && npx playwright test tests/browser/accessibility.spec.js tests/browser/controls.spec.js.
  • The accessibility scan reports zero violations. Accessible names are “Reset View for weekly velocity camera” and “Reset View for day and hour cadence camera”.
  • Screenshots show both rendered controls at 1440×1000. A recording is omitted because only accessible names changed; the camera reset behavior is covered by the browser test.

Weekly velocity reset control

Cadence reset control

@HemSoft HemSoft added the model:gpt-6.1-sol PR authored by gpt-6.1-sol label Oct 29, 2026
@HemSoft

HemSoft commented Oct 29, 2026

Copy link
Copy Markdown
Collaborator Author

@sfl review

@hemsoft-sfl-reviewer hemsoft-sfl-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Verdict: PASS • Round 1

Review done by claude-opus-5-5 (adversarial to codex)

This single commit renames the aria-label of the two 3D camera reset buttons so each starts with the visible text "Reset View". It also adds a test that the accessible name contains the visible text and updates the camera-control test to use the new name. No defects were found: the label change is markup only, it does not touch resetCamera behavior, and no file at the head SHA still uses the old names.

🔨 Full-Spectrum Review Summary

Effort: high
Review time: 0m 55s
Token count: Unavailable

Pass 🔴 Critical 🟠 High 🟡 Medium ⚪ Low
🔒 Security 0 0 0 0
✅ Accuracy 0 0 0 0
🧰 Quality & Maintainability 0 0 0 0
Total 0 0 0 0

Key Findings

No material defects found.

Coverage

  • Files reviewed:
    • src/template.py
    • tests/browser/accessibility.spec.js
    • tests/browser/controls.spec.js
  • Context read:
    • tests/browser/fixture.js
    • tests/browser/layout.spec.js
    • tests/browser/descriptions-walkthrough.spec.js
    • tests/browser/walkthrough.spec.js
    • tests/browser/mobile-walkthrough.spec.js
    • tests/browser/report.spec.js
    • tests/browser/static-report.spec.js
    • tests/browser/fallbacks.spec.js
    • tests/browser/integrity.spec.js
    • tests/browser/landing.spec.js
    • tests/browser/dune-build-editor.spec.js
    • tests/browser/dune-build-files.spec.js
    • tests/performance/budget.test.js
    • package.json
    • .github/workflows/validate.yml
    • README.md
  • Verified:
    • The compare response's base_commit.sha equals 2153f1b7acbd8db2951552c103f7c8c7cc23c332 and its last commit equals 5f89f2109809ffd37071478b65a33865839e9996. The compare status is ahead, ahead by 1 and behind by 0, and merge_base equals the base.
    • The diff touches 3 files and only changes two aria-label strings plus test locators and assertions. The onclick handlers resetCamera('vel') and resetCamera('cad') are unchanged.
    • Both new labels in src/template.py (lines 1082 and 1100) start with the visible button text Reset View. This matches the intent of axe's label-content-name-mismatch rule.
    • src/template.py contains exactly two buttons labeled "Reset View", which matches the new assertion toHaveCount(2).
    • No test or doc file at the head SHA still references the old names "Reset weekly velocity camera" or "Reset day and hour cadence camera". controls.spec.js line 23 uses the new name.
    • The new loop in accessibility.spec.js iterates over the same locator whose count is asserted right after it, so if no buttons match, the count check still fails.
    • The new regex is built from the visible text "Reset View", which has no regex metacharacters.
    • The PR's claim of "four tests" matches the files: accessibility.spec.js has 3 tests and controls.spec.js has 1.
  • Not verified:
    • I did not run the tests or axe, so whether they pass at runtime is unconfirmed.
    • The PR says axe 4.14.0 flagged the old labels. package.json pins @​axe-core/playwright to 4.13.0, so CI probably runs a different axe version than the one the PR used. The version-independent accessible-name check the PR adds covers this gap.
    • I did not view the screenshots in the PR body.
  • Tests run: None. Reviewer execution is disabled.

HemSoft full-spectrum review • 3 evidence-based passes • Reviewed head 5f89f2109809ffd37071478b65a33865839e9996 • SFL v0

@HemSoft
HemSoft force-pushed the fix/reset-view-accessible-names branch from 5f89f21 to 47d9a69 Compare October 29, 2026 05:44
@HemSoft

HemSoft commented Oct 29, 2026

Copy link
Copy Markdown
Collaborator Author

@sfl review

@hemsoft-sfl-reviewer hemsoft-sfl-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Verdict: PASS • Round 2

Review done by claude-opus-5-5 (adversarial to codex)

The two camera reset buttons now have accessible names that start with their visible text, "Reset View". The browser test, the benchmark selector and a new regression check in the accessibility spec were updated to match. I found no Critical, High, Medium or Low defect: every reference to the old names in the reviewed files was updated, the reset behavior is unchanged, and the new check catches a regression on either button.

🔨 Full-Spectrum Review Summary

Effort: high
Review time: 0m 54s
Token count: Unavailable

Pass 🔴 Critical 🟠 High 🟡 Medium ⚪ Low
🔒 Security 0 0 0 0
✅ Accuracy 0 0 0 0
🧰 Quality & Maintainability 0 0 0 0
Total 0 0 0 0

Key Findings

No material defects found.

Coverage

  • Files reviewed:
    • src/template.py
    • tests/browser/accessibility.spec.js
    • tests/browser/controls.spec.js
    • scripts/benchmark_browser.js
  • Context read:
    • package.json
    • tests/test_template.py
    • tests/browser/walkthrough.spec.js
    • tests/browser/mobile-walkthrough.spec.js
    • tests/browser/descriptions-walkthrough.spec.js
    • tests/browser/report.spec.js
    • tests/browser/static-report.spec.js
    • tests/browser/layout.spec.js
    • tests/browser/fallbacks.spec.js
    • tests/browser/fixture.js
    • tests/browser/integrity.spec.js
    • tests/test_static_editions.py
    • tests/performance/budget.test.js
    • scripts/verify_budget_probe.js
    • src/landing.py
    • quality/browser-performance.md
    • README.md
  • Verified:
    • Compare base_commit.sha is 93c369d8cd708cd2c66529f70325efeab88111e5, the last commit is d6619b17eefb1180f39c3fd41bd0b2074cbde87b, the merge base equals the base, ahead_by is 2 and behind_by is 0
    • The PR's headRefOid and baseRefOid match the reserved SHAs
    • After the change, src/template.py has no copy of the old names Reset weekly velocity camera or Reset day and hour cadence camera, and neither do the other downloaded test, script and documentation files
    • Both button labels in src/template.py (lines 1082 and 1100) start with the visible text Reset View and keep the chart context. The onclick handlers resetCamera('vel') and resetCamera('cad') are unchanged
    • The selectors in controls.spec.js line 23 and benchmark_browser.js line 64 exactly match the new aria-label on the velocity button
    • No text-transform applies to .three-btn or its .three-controls/.three-header ancestors, so innerText returns Reset View and the case-sensitive new RegExp(innerText) check holds
    • The new toHaveCount(2) assertion fails if either button's name stops starting with Reset View, so the regression check is meaningful
    • resetCamera (around line 1698) is unchanged and still restores the camera position that controls.spec.js expects
  • Not verified:
    • Runtime results for the browser tests, axe scan and benchmark: execution was forbidden
    • The PR's claim that axe-core 4.14.0 flags the old names: package.json pins @​axe-core/playwright 4.13.0, and the rule's behavior in either version was not checked
    • The committed files under editions/ were not searched for the old names: their size and separate generation path put them outside this change
    • Screenshot content linked in the PR body was not inspected
  • Tests run: None. Reviewer execution is disabled.

HemSoft full-spectrum review • 3 evidence-based passes • Reviewed head d6619b17eefb1180f39c3fd41bd0b2074cbde87b • SFL v0

@HemSoft
HemSoft merged commit 3354354 into main Oct 29, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

model:gpt-6.1-sol PR authored by gpt-6.1-sol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant