Skip to content

fix(web): link profile form validation errors to their inputs - #6779

Open
maphew wants to merge 4 commits into
Kilo-Org:mainfrom
maphew:fix/profile-form-error-associations
Open

maphew wants to merge 4 commits into
Kilo-Org:mainfrom
maphew:fix/profile-form-error-associations

Conversation

@maphew

@maphew maphew commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Issue #4172 lists apps/web/src/components/profile/EditProfileDialog.tsx in its representative scope. Both profile URL fields still fail the acceptance criteria:

  • The LinkedIn and GitHub inputs set aria-invalid but carry no aria-describedby, so a screen reader announces the field as invalid without the reason.
  • The error text is rendered in an unlabelled <p> with no stable id, so the input cannot reference it.
  • The save-failure message is not a live region, so it is not announced when it appears.

This is the same defect class as the forms already fixed in #6068: an invalid control whose validation text is not programmatically linked.

Change

  • Give each error message a stable id and role="alert", and reference it from the input with aria-describedby while the error is present.
  • Give the mutation-level save failure role="alert" so it is announced.
  • Import default React so Jest's classic SWC JSX transform can render this component.

Tests

Add EditProfileDialog.test.ts, following the linkedom + react-dom/client pattern already used by EmailInputForm.test.ts. It checks that both invalid URLs link to announced errors, and that the associations clear when the form resets.

Verification

  • Focused test passes: 2 passed (EditProfileDialog.test.ts) under the repository's classic SWC transform without DB hooks.
  • An independent adversarial review found the missing default React import; it is fixed and the focused test was rerun.
  • tsgo --noEmit -p apps/web/tsconfig.json exit 0.
  • oxlint --config .oxlintrc.json clean on both changed files; oxfmt reports no differences; git diff --check clean.
  • Note: the web Jest suite normally runs against a per-worker PostgreSQL database (src/tests/setup/workerSetup.ts). This sandbox has no database or Docker, so the focused test was run with an equivalent config minus the DB setup hooks. CI runs the full suite.

Refs #4172.

@maphew
maphew marked this pull request as ready for review September 27, 2026 15:42
@kilo-code-bot

kilo-code-bot Bot commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

Re-reviewed the two changed files at current HEAD; the branch was rebased and the content is byte-identical to the previously reviewed revision, so the aria-describedby + role="alert" error association (which links and clears with the validation state) and the linkedom/Jest test's global-DOM restore plus React root teardown remain correct.

Files Reviewed (2 files)
  • apps/web/src/components/profile/EditProfileDialog.tsx
  • apps/web/src/components/profile/EditProfileDialog.test.ts
Previous Review Summaries (4 snapshots, latest commit 940631f)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 940631f)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Full review of the two changed files found no correctness, accessibility, security, or memory-leak issues: the aria-describedby/role="alert" wiring clears correctly when errors reset, the open-time reset effect is unchanged, and the new linkedom/Jest test restores globals and unmounts its React root in afterEach.

Files Reviewed (2 files)
  • apps/web/src/components/profile/EditProfileDialog.tsx
  • apps/web/src/components/profile/EditProfileDialog.test.ts

Previous review (commit 0eba64b)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Re-reviewed both changed files at current HEAD; they are byte-identical to the previously reviewed revision (the branch was only rebased), and the aria-describedby/role="alert" wiring, the open-time reset path, and the test's global-DOM / React root teardown remain correct.

Files Reviewed (2 files)
  • apps/web/src/components/profile/EditProfileDialog.tsx
  • apps/web/src/components/profile/EditProfileDialog.test.ts

Previous review (commit 9d3233e)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Full review of the two changed files found no correctness, accessibility, security, or memory-leak issues: the aria-describedby/role="alert" wiring, the open-time reset path, and the test's global-DOM swap and React root teardown all check out.

Files Reviewed (2 files)
  • apps/web/src/components/profile/EditProfileDialog.tsx
  • apps/web/src/components/profile/EditProfileDialog.test.ts

Previous review (commit ddbc59a)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Reviewed the profile-dialog accessibility change (18 changed lines in EditProfileDialog.tsx) and the new 158-line linkedom/Jest test; the aria-describedby + role="alert" wiring matches the established pattern in EmailInputForm.tsx, and no bugs, leaks, or invalid targets were found.

The component now links each URL field's validation text via a stable id/aria-describedby and marks the field/save errors as role="alert". This mirrors the already-fixed pattern in apps/web/src/components/auth/sign-in/EmailInputForm.tsx:65-71. The default React import matches the dominant convention in apps/web (78 components) needed by the classic SWC JSX transform under Jest. The test's global-DOM swap is fully restored in afterEach, the React root is unmounted, and no timers or listeners are left behind. The linkedom resolution from requireFromHere('linkedom') is an existing pattern (EmailInputForm.test.ts:11); sibling tests add a try/catch fallback but resolution does not depend on the directory, so this is not a new defect.

Files Reviewed (2 files)
  • apps/web/src/components/profile/EditProfileDialog.tsx
  • apps/web/src/components/profile/EditProfileDialog.test.ts

Reviewed by deepseek-v4.1-flash · Input: 43.3K · Output: 8.1K · Cached: 384.4K

Review guidance: REVIEW.md from base branch main

@maphew
maphew force-pushed the fix/profile-form-error-associations branch 3 times, most recently from 0eba64b to 940631f Compare October 3, 2026 16:34
maphew added 2 commits October 4, 2026 03:00
Both profile URL fields set aria-invalid but had no aria-describedby,
so screen readers announced the field as invalid without the reason.
Give each error a stable id and role=alert, reference it from the
input, and announce the save failure.

Refs Kilo-Org#4172.
@maphew
maphew force-pushed the fix/profile-form-error-associations branch from 940631f to 8fd6464 Compare October 4, 2026 03:01

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant