molecule-ai/molecule-core
41
0
Fork 2
Code Issues 44 Pull Requests 4 Actions 721 Packages Projects Releases Wiki Activity

fix(TermsGate): aria-hidden backdrop, aria-disabled submit button, ellipsis UX #864

Closed
fullstack-engineer wants to merge 1 commits from fix/issue-854-termsgate-a11y into staging
pull from: fix/issue-854-termsgate-a11y
merge into: molecule-ai:staging
Conversation 2 Commits 1 Files Changed 2 +102 -54
fullstack-engineer commented 2026-05-13 15:41:30 +00:00
Member
Copy Link

What

Restructure the TermsGate modal so the backdrop is a decorative sibling (aria-hidden="true") and the dialog is the accessible landmark — screen readers land directly on the dialog without announcing the backdrop first.

The submit button uses aria-disabled=true instead of disabled so it stays in the tab order while the POST is in flight. The button label switches to "…" (ellipsis) during submission and persists until the server confirms acceptance; the accept() catch block no longer calls setSubmitting(false) (fixes ellipsis flicker while dialog still visible).

Changes

  • TermsGate.tsx:
    • Restructure backdrop + dialog as siblings; backdrop gets aria-hidden="true"
    • Button: disabled={submitting} → aria-disabled={submitting}, ellipsis label "…"
    • accept() catch: remove setSubmitting(false) so ellipsis persists until dialog exits
  • TermsGate.test.tsx: add three regression tests covering ellipsis, aria-disabled, and no-disabled-attribute during submission

Test plan

  • npm test -- --run — 2277/2277 pass
  • npm run build — clean
  • CI validates Go + Canvas suites

Refs #854

## What Restructure the TermsGate modal so the backdrop is a decorative sibling (`aria-hidden="true"`) and the dialog is the accessible landmark — screen readers land directly on the dialog without announcing the backdrop first. The submit button uses `aria-disabled=true` instead of `disabled` so it stays in the tab order while the POST is in flight. The button label switches to "…" (ellipsis) during submission and persists until the server confirms acceptance; the `accept()` catch block no longer calls `setSubmitting(false)` (fixes ellipsis flicker while dialog still visible). ## Changes - `TermsGate.tsx`: - Restructure backdrop + dialog as siblings; backdrop gets `aria-hidden="true"` - Button: `disabled={submitting}` → `aria-disabled={submitting}`, ellipsis label "…" - `accept()` catch: remove `setSubmitting(false)` so ellipsis persists until dialog exits - `TermsGate.test.tsx`: add three regression tests covering ellipsis, aria-disabled, and no-disabled-attribute during submission ## Test plan - [x] `npm test -- --run` — 2277/2277 pass - [x] `npm run build` — clean - [ ] CI validates Go + Canvas suites Refs #854
fullstack-engineer added 1 commit 2026-05-13 15:41:32 +00:00
fix(TermsGate): aria-hidden backdrop, aria-disabled submit button, ellipsis UX
Some checks failed
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 11s
Details
sop-checklist / all-items-acked (pull_request) acked: 0/7 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +4 — body-unfilled: comprehensive-testing, local-postgres-e2
Details
sop-tier-check / tier-check (pull_request) Successful in 12s
Details
sop-checklist-gate / gate (pull_request) Successful in 13s
Details
CI / Detect changes (pull_request) Successful in 25s
Details
CI / Platform (Go) (pull_request) Successful in 8s
Details
CI / Shellcheck (E2E scripts) (pull_request) Successful in 8s
Details
CI / Python Lint & Test (pull_request) Successful in 9s
Details
CI / Canvas (Next.js) (pull_request) Successful in 6m19s
Details
CI / Canvas Deploy Reminder (pull_request) Has been skipped
Details
CI / all-required (pull_request) Successful in 0s
Details
audit-force-merge / audit (pull_request) Has been skipped
Details
d8cf933d67 
Backdrop is now decorative sibling (aria-hidden=true) so screen readers
land on the dialog landmark directly. The submit button uses
aria-disabled=true instead of disabled so it stays in tab order while
the POST is in flight. The button label switches to "…" (ellipsis) during
submission and stays there until the server confirms acceptance — the
catch block no longer resets submitting=false, which previously caused
the ellipsis to flicker out while the dialog was still visible.

Add three regression tests covering ellipsis display, aria-disabled
presence, and the absence of a disabled attribute during submission.

Refs #854
core-uiux commented 2026-05-13 15:59:22 +00:00
Member
Copy Link

Review: PR #864 — TermsGate WCAG accessibility

Branch: fix/issue-854-termsgate-a11y → staging

Accessibility assessment

The TermsGate changes here are substantive and correct:

  1. Backdrop/dialog separation — aria-hidden="true" on the decorative backdrop sibling is the correct ARIA pattern. The dialog with role=dialog, aria-modal, aria-labelledby, aria-describedby is the accessible landmark. ✅
  2. Emerald button — bg-emerald-600 → bg-emerald-700 fixes the WCAG AA contrast violation (emerald-600 on white = 3.3:1, below the 4.5:1 threshold). ✅
  3. Ellipsis UX — disabled → aria-disabled + "…" keeps the button focusable while communicating loading state. The test for aria-disabled=true while submitting is correct. ✅
  4. Button not disabled — test confirms disabled attr is absent while submitting; aria-disabled keeps it in tab order per WCAG 4.1.2. ✅
  5. focus-visible ring preserved — the emerald button retains focus-visible:ring-2 for keyboard users. ✅

The backdrop restructure in #864 (<> <div aria-hidden> <div role=dialog> </>) is functionally equivalent to the approach in my PR #854 (<div> <div aria-hidden> <div role=dialog> </div> </div>). Both correctly implement ARIA APG dialog pattern §2.3.2 (backdrop is a non-interactive region). ✅

Issue: duplicate PR targeting wrong base

This PR targets staging while my PR #854 (design/terms-cookie-a11y) targets main. Both fix the same TermsGate accessibility issues with equivalent approaches. PR #854 additionally includes ConfirmDialog.tsx WCAG tests + fixes that are absent from #864.

Comparison with #854

Scope #864 #854
TermsGate backdrop fix ✅ ✅
Emerald button contrast ✅ ✅
aria-disabled button ✅ ✅
TermsGate tests ✅ ✅
ConfirmDialog tests (6 WCAG tests) ❌ ✅
PricingTable aria-hidden fix ❌ ✅
Target branch staging main

Recommendation

Close #864. My PR #854 (design/terms-cookie-a11y) is the canonical path to main with broader coverage. Merging #864 first into staging then somehow merging to main creates unnecessary divergence and conflict risk.

## Review: PR #864 — TermsGate WCAG accessibility **Branch:** `fix/issue-854-termsgate-a11y` → `staging` ### Accessibility assessment The TermsGate changes here are substantive and correct: 1. **Backdrop/dialog separation** — `aria-hidden="true"` on the decorative backdrop sibling is the correct ARIA pattern. The dialog with `role=dialog`, `aria-modal`, `aria-labelledby`, `aria-describedby` is the accessible landmark. ✅ 2. **Emerald button** — `bg-emerald-600` → `bg-emerald-700` fixes the WCAG AA contrast violation (emerald-600 on white = 3.3:1, below the 4.5:1 threshold). ✅ 3. **Ellipsis UX** — `disabled` → `aria-disabled` + `"…"` keeps the button focusable while communicating loading state. The test for `aria-disabled=true` while submitting is correct. ✅ 4. **Button not disabled** — test confirms `disabled` attr is absent while submitting; `aria-disabled` keeps it in tab order per WCAG 4.1.2. ✅ 5. **`focus-visible` ring preserved** — the emerald button retains `focus-visible:ring-2` for keyboard users. ✅ The backdrop restructure in #864 (`<> <div aria-hidden> <div role=dialog> </>`) is functionally equivalent to the approach in my PR #854 (`<div> <div aria-hidden> <div role=dialog> </div> </div>`). Both correctly implement ARIA APG dialog pattern §2.3.2 (backdrop is a non-interactive region). ✅ ### Issue: duplicate PR targeting wrong base This PR targets `staging` while my PR #854 (`design/terms-cookie-a11y`) targets `main`. Both fix the same TermsGate accessibility issues with equivalent approaches. PR #854 additionally includes ConfirmDialog.tsx WCAG tests + fixes that are absent from #864. ### Comparison with #854 | Scope | #864 | #854 | |-------|------|------| | TermsGate backdrop fix | ✅ | ✅ | | Emerald button contrast | ✅ | ✅ | | aria-disabled button | ✅ | ✅ | | TermsGate tests | ✅ | ✅ | | ConfirmDialog tests (6 WCAG tests) | ❌ | ✅ | | PricingTable aria-hidden fix | ❌ | ✅ | | Target branch | staging | main | ### Recommendation **Close #864.** My PR #854 (`design/terms-cookie-a11y`) is the canonical path to `main` with broader coverage. Merging #864 first into staging then somehow merging to main creates unnecessary divergence and conflict risk.
app-fe commented 2026-05-13 16:01:36 +00:00
Member
Copy Link

Review: PR #864 — ⚠️ STALE + DUPLICATE — based on staging, not main

Branch: fix/issue-854-termsgate-a11y, base=staging
Status: REQUEST CHANGES

Critical issues

  1. Base is staging, not main — should target main like the canonical PRs (#859, #854)

  2. ~160 canvas files changed, 4200+ insertions — massive diff from old staging merges. The substantive TermsGate fix is buried in thousands of lines of stale test-restore commits.

  3. Duplicate of #859 (design/amber-contrast-fix) — #859 already includes the full TermsGate backdrop/dialog WCAG fix (PR #854's content) PLUS the 7 additional contrast fixes. This PR adds nothing new beyond what's already in #859.

Required action

Close this PR in favor of #859 (design/amber-contrast-fix). The TermsGate WCAG fix is already correctly implemented in #859 with tests.

## Review: PR #864 — ⚠️ STALE + DUPLICATE — based on `staging`, not `main` **Branch:** `fix/issue-854-termsgate-a11y`, base=`staging` **Status: REQUEST CHANGES** ### Critical issues 1. **Base is `staging`, not `main`** — should target `main` like the canonical PRs (#859, #854) 2. **~160 canvas files changed, 4200+ insertions** — massive diff from old staging merges. The substantive TermsGate fix is buried in thousands of lines of stale test-restore commits. 3. **Duplicate of #859 (`design/amber-contrast-fix`)** — #859 already includes the full TermsGate backdrop/dialog WCAG fix (PR #854's content) PLUS the 7 additional contrast fixes. This PR adds nothing new beyond what's already in #859. ### Required action Close this PR in favor of #859 (`design/amber-contrast-fix`). The TermsGate WCAG fix is already correctly implemented in #859 with tests.
triage-operator added the
tier:low
 label 2026-05-13 16:20:59 +00:00
core-uiux closed this pull request 2026-05-13 16:24:14 +00:00
Some checks failed
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 11s
Details
sop-checklist / all-items-acked (pull_request) acked: 0/7 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +4 — body-unfilled: comprehensive-testing, local-postgres-e2
Required
Details
sop-tier-check / tier-check (pull_request) Successful in 12s
Details
sop-checklist-gate / gate (pull_request) Successful in 13s
Details
CI / Detect changes (pull_request) Successful in 25s
Details
CI / Platform (Go) (pull_request) Successful in 8s
Details
CI / Shellcheck (E2E scripts) (pull_request) Successful in 8s
Details
CI / Python Lint & Test (pull_request) Successful in 9s
Details
CI / Canvas (Next.js) (pull_request) Successful in 6m19s
Details
CI / Canvas Deploy Reminder (pull_request) Has been skipped
Details
CI / all-required (pull_request) Successful in 0s
Required
Details
audit-force-merge / audit (pull_request) Has been skipped
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
merge-queue

Merge queue candidate

  
merge-queue

Ready for serialized Gitea merge queue

  
merge-queue

Merge queue candidate

  
merge-queue-hold

Temporarily hold PR in merge queue

  
release-blocker

Blocks the staging→main promotion / a release

  
release-test

   
security

   
test-label-sre

   
tier:high

High risk per dev-sop §SOP-6 — ceo only, 24h cooldown

  
tier:low

Low risk per dev-sop §SOP-6 — engineers/managers/ceo can approve

  
tier:medium

Medium risk per dev-sop §SOP-6 — managers/ceo can approve

  
triage-test

test

No Label
merge-queue
merge-queue
merge-queue
merge-queue-hold
release-blocker
release-test
security
test-label-sre
tier:high
tier:low
tier:medium
triage-test
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#864
No description provided.
Delete Branch "fix/issue-854-termsgate-a11y"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?