fix(#867): the last two auth forms now say which field they are about - #871
Merged
Merged
Conversation
ForgotPasswordForm and ResetPasswordForm had the identical shape #866 fixed in SignInForm and SignUpForm: field-specific validation funnelled into a single form-level alert with no `id`, and no input carrying `aria-invalid` or `aria-describedby`. Leaving two of four forms behind was worse than doing all four or none. ResetPasswordForm is the sharper case — TWO conditions shared one string, a password that fails validatePassword and a confirmation that does not match, so "Passwords do not match" was announced with nothing saying which of the two boxes to fix. It is announced against the CONFIRMATION now: that is the one the user is being asked to change. Deliberately still form-level: the rate limit, the captcha challenge, the Supabase update failure, and the password-reset response — the last of those is generic on purpose so it cannot be used to confirm whether an address is registered. BOTH ACCESSIBILITY TEST FILES WERE GENERATOR STUBS THAT ASSERTED NOTHING `should have proper ARIA attributes` in each contained only example comments — rendering the form and asserting nothing, in the file whose job is accessibility. Two more instances of #396 on top of the two found in #866, which makes four of four auth forms shipped that way. The ResetPasswordForm test is table-driven over both conditions and checks the half that makes it worth having: the OTHER field must stay clean. An implementation that marked every input invalid would satisfy "the error is associated" and still tell the user nothing. MUTATION-VERIFIED, both directions: - routing the mismatch to `password` instead of `confirmPassword` fails exactly the mismatch case - removing `aria-describedby` while still rendering the message fails with "the email input must point at its error: expected null to be truthy" THREE THINGS THAT COST TIME, ALL THE SAME SHAPE 1. My first test password for the mismatch case was `LongEnough123`, which has no special character — so validatePassword rejected it FIRST, the mismatch branch never ran, and the test asserted against a state it never reached. Both values must pass validation for the mismatch to be reachable at all. 2. `passwordValidation.error` is `string | null`. The old `setError` accepted null; a typed field message does not. It falls back to a real sentence rather than a cast, so an invalid password without a message still says something. 3. My first patch attempt matched three of four anchors silently and wrote the file anyway. Each edit now asserts its own anchor AND that the anchor is unique — a patch that half-applies is the same defect class as a guard that half-matches. lint clean, 4734 vitest tests, tsc --noEmit clean. Closes #867 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The gap
ForgotPasswordFormandResetPasswordFormhad the identical shape #866 fixed inSignInFormandSignUpForm: field-specific validation funnelled into a single form-level alert with noid, and no input carryingaria-invalidoraria-describedby. Leaving two of four forms behind was worse than doing all four or none.ResetPasswordFormis the sharper case — two conditions shared one string::36validatePasswordresult:41So "Passwords do not match" was announced with nothing saying which of the two boxes to fix. It's announced against the confirmation now — that's the one the user is being asked to change.
Deliberately still form-level: the rate limit, the captcha challenge, the Supabase update failure, and the password-reset response. That last one is generic on purpose so it can't be used to confirm whether an address is registered.
Both accessibility test files were stubs that asserted nothing
should have proper ARIA attributesin each contained only example comments — rendering the form and asserting nothing, in the file whose job is accessibility. Two more instances of #396 on top of the two in #866, which makes four of four auth forms shipped that way.The
ResetPasswordFormtest is table-driven over both conditions and checks the half that makes it worth having: the other field must stay clean. An implementation that marked every input invalid would satisfy "the error is associated" and still tell the user nothing.Mutation-verified, both directions:
passwordinstead ofconfirmPassword→ fails exactly the mismatch casearia-describedbywhile still rendering the message →the email input must point at its error: expected null to be truthyThree things that cost time, all the same shape
LongEnough123— no special character, sovalidatePasswordrejected it first, the mismatch branch never ran, and the test asserted against a state it never reached. Both values have to pass validation for the mismatch to be reachable at all.passwordValidation.errorisstring | null. The oldsetErroraccepted null; a typed field message doesn't. It falls back to a real sentence rather than a cast.lint clean, 4734 vitest tests,
tsc --noEmitclean.Closes #867
🤖 Generated with Claude Code