Repository navigation
Finish public form polish: error stability, group a11y, mobile hints (#211) - #230
Conversation
Reviewer's GuideCompletes the remaining public-form polish by preventing support-form layout shifts, making checkbox-group errors announce correctly on the focused controls, adding next-field mobile keyboard hints while preserving natural textarea and checkbox behavior, and updating stale submission-architecture documentation. Tests cover the accessibility and interaction changes, with the stated lint, typecheck, build, and full Vitest suite passing. Sequence diagram for support request submission and fallbacksequenceDiagram
actor Visitor
participant SupportForm
participant API as SupportRequestsAPI
participant Handoff as MailtoOrWhatsApp
Visitor->>SupportForm: Submit support request
SupportForm->>API: POST /api/support-requests
alt Submission succeeds
API-->>SupportForm: Reference
SupportForm-->>Visitor: Show success panel
else Recoverable failure
API-->>SupportForm: Request failure
SupportForm-->>Visitor: Preserve entered values
Visitor->>Handoff: Use mailto or WhatsApp fallback
end
Flow diagram for accessible support-form validationflowchart TD
Blur[User blurs field or submits form] --> Validate[Validate support form]
Validate --> Invalid{Invalid input?}
Invalid -->|No| Continue[Continue submission]
Invalid -->|Yes| Slots[Keep error slots mounted with reserved height]
Slots --> Focus[Focus first invalid control]
Focus --> Checkbox[Focused support checkbox]
Checkbox --> Announce[Announce sr-types-error via aria-describedby]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
⛔ Files ignored due to path filters (9)
📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe contact and support request forms add keyboard hints to selected single-line inputs. The support request form conditionally renders validation errors and associates support-type errors with each checkbox. Tests and a Playwright script cover form validation states. ChangesPublic form updates
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to No actionable issue is established for the public-form changes; they are mergeable after normal checks. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 3 systems. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description covers the main form accessibility and keyboard-hint changes, but it states that errors reserve space. The implementation summary and commit message state that errors mount only when a message exists.
Comment |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="components/donations/support-request-form.tsx" line_range="48" />
<code_context>
return (
- <p id={id} className="text-xs text-destructive">
- {message}
+ <p id={id} className={`min-h-4 text-xs text-destructive${message ? "" : " invisible"}`}>
+ {message || " "}
</p>
</code_context>
<issue_to_address>
**Validation shifts later fields**
When a long validation message wraps onto multiple lines at a narrow viewport, `min-h-4` reserves only one line, so `FieldError` grows when the message appears and pushes later fields down, shifting the layout.
Reserve enough height for wrapped validation messages so the error slot does not grow when they appear.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: components/donations/support-request-form.tsx:48
…211) The bulk of #211 already landed via #218 (input types, autocomplete, touched/blur validation, focus-to-first-invalid, pending announcements, idempotent resubmission, preserved values, fallbacks). This closes the remaining gaps found auditing both public forms against the acceptance criteria: - Support-request form: reserve inline error slots (min-h + invisible) like the contact form, so errors appearing on blur/submit no longer push every field below them down mid-read - Support-type checkbox group: the group error was described on the fieldset, but focus lands on a checkbox — aria-describedby/aria-invalid now sit on each box so the error is actually announced in context - enterKeyHint="next" on single-line inputs in both forms so mobile keyboards offer field-to-field traversal; textareas/checkboxes keep their natural Enter behavior - lib/support-request.ts: fix stale docstring — it still described the pre-#189 mailto-only architecture; the boundary POST is primary and these helpers are the explicit recoverable-failure fallback No backend contract, validation rule, or analytics changes. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
focus:ring also paints on mouse click — a pointer user sees a lingering focus halo on a control that is already visibly checked. focus-visible keeps the indicator for keyboard users where it is needed. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
b283be0 to
259e224
Compare
…ompact (#211) The reserved-slot approach (min-h-4 + invisible text, copied from the much shorter contact form) padded every untouched field with ~22px of dead space, stretching the long request form in the owner preview. FieldError now mounts only when a message exists, restoring the compact two-column rhythm; a field group grows locally when its error appears, which is the accepted tradeoff. The touched-gated aria-describedby / aria-invalid wiring, checkbox-group association, focus-to-first-invalid, and all validation semantics are unchanged. Tests: replaced the mounted-slot assertion with behavior coverage — untouched form mounts no error nodes, the message mounts wired to its control, and both clear on correction. The donate axe scan now asserts the on-demand errors are visible instead of checking an invisible class. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Summary
Closes #211 (complements canonical #218, delivered in #226). Auditing both public forms against the #211 acceptance criteria showed the bulk was already in place — correct input types,
autocomplete,inputMode, touched/blur validation timing, focus-to-first-invalid, pending-state announcements, idempotent resubmission, preserved values, and email/WhatsApp fallbacks. This PR closes the remaining gaps. No backend contract, validation rule, or analytics changes.Forms audited
/contactinquiry form (mailto + WhatsApp handoff)/donatesupport-request form (POST/api/support-requestswith fallbacks)/partnersaccommodation search (non-submission — already correct:type="search",autoComplete="off",enterKeyHint="search")Changes
support-request-form.tsx):FieldErrorconditionally mounts only when a validation message exists — untouched fields reserve no error space, keeping the long form compact and deliberate.aria-describedbyis present only while its error node exists; correcting a field unmounts the error and drops the association. Error appearance causes only local field-group growth — the intentional tradeoff: a small bounded expansion when a message is actually shown instead of pre-reserved dead space under every field for errors that may never appear. (The short contact form keeps its established reserved-slot convention.)aria-describedby/aria-invalidsit on each support-type checkbox — failed-submission focus lands on a box, and afieldset's description isn't announced for its children — so whichever option receives focus announces "Please choose at least one type of support."focus-visibleso pointer users don't see a lingering focus halo on an already-checked control.enterKeyHint="next"on every single-line input in both forms (name, organization, email, phone, amount, beneficiaries, timing, reference URL, WhatsApp, dates, party size, certification, logged dives). Textareas and checkboxes deliberately keep natural Enter behavior.lib/support-request.ts): updated to reflect the actual contract —/api/support-requestsis primary; the mailto/WhatsApp helpers are the explicit recoverable-failure fallback (keeping the Contact form: route inquiries through a respond.io Custom Channel to fix shared-contact identity collapse #104 shared-sender rationale).Tests
donate.test.tsx: support-type group error is associated with every checkbox; error rendering is behavior-verified — an untouched form mounts no error nodes, a failed validation mounts the message under its control wired viaaria-describedby, and correcting the field unmounts the error and drops the association;enterkeyhintcoverage incl. negative textarea/checkbox checks.accessibility.spec.ts(E2E): the donate error-state axe scan asserts the on-demand errors are visible before scanning, including the checkbox-group error.contact-form.test.tsx:enterkeyhinton name/email, absent on the message textarea.npm run lint,typecheck,check,build:test— clean.npx vitest run --coverage— 524/524 pass. Playwright accessibility spec — 29 pass, 0 violations.Test plan
Generated with Devin
Summary by Sourcery
Polish the public forms’ validation presentation, accessibility announcements, and mobile keyboard navigation without changing submission contracts or validation rules.
Bug Fixes:
Enhancements:
Tests:
Chores:
Summary by CodeRabbit