test(#10722): rendered-geometry 44px tap-target gate + semantic assertions - #11868
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
…me upgrades New real-Chromium ui-smoke gate tap-target-geometry-all-views.spec.ts asserts every interactive element's rendered bbox meets the 44x44px minimum + role<->DOM coherence across app views; all-views-interaction.spec.ts upgrades a subset of 'no page error' assertions to semantic outcomes. Test-only (no product source). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
c98431b to
cbfd5f8
Compare
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
I ran the new gate against current develop and pushed a follow-up commit (
Validation after the follow-up:
Remaining failing routes: browser, stream, automations, inventory, documents, files, plugins, fine-tuning, relationships, memories. The remaining failures are real geometry/name findings across plugin/app-specific controls, especially 40px form fields/buttons and smaller tab/filter/icon controls. This PR should still not merge until those are fixed or explicitly documented with justified exceptions. |
|
Validation attempted on rebased branch against Command: Result: failed. The app/view build completed and the new Chromium geometry gate ran across 33 routes: 23 passed, 10 failed. Failing views: Representative failures:
Leaving unmerged per the PR body requirement: run the gate and fix real violations or document intentional exceptions before merge. |
|
HOLD — per your own do-not-merge note, plus one real defect. The gate itself is real (rendered geometry, no soft-pass paths, empty To merge: fix the paired |
…-role overrides + require visibility Running the gate surfaced that the role<->DOM coherence check false-positived on every ARIA composite/widget role (role=combobox on the chat composer textarea and on the logs filter buttons), and ran against hidden 0x0 portal internals. ARIA widget roles (combobox/searchbox/spinbutton/switch/menuitem/tab/option/…) are DESIGNED to layer onto native interactive elements — only an interaction-model swap (link<->button) or a non-interactive reclassification (heading/img/ presentation) is a real defect. Exempt the widget overrides and gate the coherence check on isVisible(). With that fixed the gate reports only GENUINE geometry violations — a real sub-44px a11y backlog (see PR/issue inventory). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Update — gate now RUNS and is CORRECT; it surfaced a real sub-44px a11y backlogGot the gate running (the earlier "webServer won't boot" was a port-31337 collision with an adb-forward, unrelated to the gate). Two rounds: Round 1 — fixed a gate false-positive. The role↔DOM coherence check flagged every ARIA composite/widget role — Round 2 — the gate now reports only GENUINE geometry violations. 25 views fail on real sub-44px controls — a legitimate a11y backlog:
Disposition — do NOT merge as a blocking gate yetThe gate is correct and valuable, but the app has a real touch-target backlog. Making it green requires an a11y sizing pass across those shared components (sidebar toggle, swatch buttons, welcome-card actions, log filters), and every one of those is a UI change that needs the |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Updated the branch with commit 8384a1e (fix(ui): finish tap target geometry gate).\n\nValidation run from /tmp/eliza-11868-pr/packages/app:\n\n |
…ch-action (#11853 sibling) (#11904) The existing *-gate.test.ts files are all STYLE gates. Two production UI bugs slipped past every one of them plus manual QA because they were MECHANICS bugs (the pixels looked right, the interaction was dead): (a) drawer-not-scrollable: a clamped-height (max-h-[..vh]) + flex-col + overflow-hidden shell with no inner overflow-y-auto scroll body — taller content is dead-clipped and unreachable. (b) broken-swipe: a useHorizontalPager surface with no explicit touch-action — the browser default (auto) steals the horizontal pan and fires pointercancel, so the flick never commits on touch. Adds a static (no-browser) vitest gate scoped to the real drawer/pager surfaces (McpDetailDrawer, Launcher, HomeLauncherSurface). Comments are stripped before scanning and the drawer scroll-body triad is checked per-element (not per-file) so a comment mention or a decoy scroller can't mask a regression. Self-tests plant each bug shape (incl. the exact false-negatives codex flagged) and assert the detector fires. Green against develop; fails on both planted real-bug shapes. This is the MECHANICS gap not covered by #11868 (44px tap-target), #11877 (focus order), or #11898 (test-red clearing). Enforcement is #11853's job. — [sol-orch] Co-authored-by: wakesync <shadow@shad0w.xyz>
…fix selectors/roles/report path + real 44px component fixes (review salvage) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
LifeOps Benchmark —
|
|
❌ PR title does not match the required pattern. Please use one of these formats:
|
2 participants
What
Adds a reusable rendered-geometry gate (
tap-target-geometry-all-views.spec.ts, real-Chromium ui-smoke, Pixel-7 coarse-pointer viewport) that walks the canonical built-in view enumeration, measures realgetBoundingClientRect()for every standalone interactive control, and asserts:role=button/linkmust be keyboard-focusable; every control exposes an accessible name).Legitimately-sub-44 controls (inline prose links, native checkbox/radio whose
<label>is the real tap surface, nested/off-screen/disabled) are excluded in-page with a documented reason and surfaced in the report, never silently dropped.all-views-interaction.spec.tsupgrades a subset of "no page error" assertions to semantic outcomes. Test-only — no product source changed.This is salvaged from a rate-limited agent lane that did not get to run the gate. The gate is designed to FAIL on genuinely-under-44px controls (the sibling
tap-target-geometry.spec.tsdocuments known regressions like a 36px ShellBackButton). Before merge it needs one real ui-smoke run to triage what it reports — any real violations get fixed at the source or added toDOCUMENTED_EXCEPTIONSwith a written reason. Biome + structural typecheck are clean; the gate logic is complete.Do not merge until the gate has been run green. Refs #10722.
🤖 Generated with Claude Code