Repository navigation
[Remove Vuetify from Studio] 'Reset password' page - #6145
LightCreator1007 wants to merge 3 commits into
Conversation
Replace the four Vuetify-backed pieces of the reset password page with their Studio/KDS equivalents, per learningequality#5931: MessageLayout -> StudioMessageLayout VForm -> <form> + generateFormMixin Banner -> StudioBanner PasswordField -> StudioPasswordField All four replacements already existed and are in use by sibling pages; no new components are introduced. The remaining pages under pages/resetPassword/ were migrated earlier, so this brings the last Vuetify page in that directory into line. Two details preserve existing behaviour: - The submit handler validates and submits the raw this.form values rather than formMixin's clean(), which trims every field. Passwords must keep any leading or trailing spaces the user typed. - A touched map gates error display until blur, since formMixin's setters otherwise flag an error on every keystroke, where the VForm version validated on blur. Strings, submit payload, redirect target, and the length and match rules are unchanged. resetPassword.spec.js required no changes and still passes.
|
👋 Hi @LightCreator1007, thanks for contributing! For the review process to begin, please verify that the following is satisfied:
Also check that issue requirements are satisfied & you ran Pull requests that don't follow the guidelines will be closed. Reviewer assignment can take up to 2 weeks. |
|
📢✨ Before we assign a reviewer, we'll turn on |
🟡 Waiting for changesLast updated: 2026-10-06 17:11 UTC |
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #6145: four regressions block merge. The migration follows the sign-in page's form pattern. The happy path, bad-token banner, whitespace preservation and RTL all work in the browser.
CI passing. Manual QA: axe AA finds 0 violations on every state. No console errors.
- blocking: blurring an empty field shows no "required" error (inline)
- blocking: editing the first password doesn't re-check the confirmation (inline)
- blocking: form overflows a 360px viewport (inline)
- blocking:
resetPassword.spec.jsnot updated, which the AC requires (inline) - suggestion: move focus to the first invalid field on a failed submit (inline)
- nitpick: trim the process context from the
touchedcomment (inline)
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Ran a phased review pipeline over the pull request diff:
- Classified the diff to select review passes (core, frontend, backend) and whether manual QA was required
- Core review pass checked correctness, design, architecture, testing, completeness, and DRY/SRP/Rule-of-Three principles
- Specialized frontend/backend review passes applied framework-specific lenses where those files changed
- For UI changes: manual QA and an accessibility audit against a live dev server, when available
- Checked CI status and linked issue acceptance criteria
- Synthesized one review from those passes and chose the verdict from the findings, CI status, and QA evidence
|
Hi @LightCreator1007! If you can, could you please look into the review feedback? Otherwise, please let us know if you’re unable to proceed. Thanks! |
- Derive displayed field errors from form state and touched flags, so blurring an empty field shows the required error and editing the new password re-checks the confirmation - Move focus to the first invalid field on a failed submit - Give StudioMessageLayout's slot container a full width so forms no longer overflow narrow viewports - Cover blur validation, confirmation re-checks, required errors, the failure banner, focus handling and untrimmed passwords in the spec
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #6145: all 6 prior findings resolved; one new suggestion inline.
CI passing. Manual QA did not run, so the UI is unverified.
Prior-finding status
RESOLVED — ResetPassword.vue:27 — blurring an empty field shows no required error
RESOLVED — ResetPassword.vue:65 — editing the first password doesn't re-check the confirmation
RESOLVED — ResetPassword.vue:154 — form overflows a 360px viewport
RESOLVED — ResetPassword.vue:108 — resetPassword.spec.js not updated
RESOLVED — ResetPassword.vue:116 — move focus to the first invalid field on failed submit
RESOLVED — ResetPassword.vue:80 — trim process context from the touched comment
Written by rtibblesbot, an LLM-based coding agent.
Superseded by my review of 1d85abb: no blocking findings remain.
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #6145: all 7 prior findings resolved. Manual QA did not run. Commenting rather than approving.
Prior-finding status
RESOLVED — ResetPassword.vue:26 — blurring an empty field shows no required error
RESOLVED — ResetPassword.vue — editing the first password doesn't re-check the confirmation
RESOLVED — ResetPassword.vue:171 — form overflows a 360px viewport
RESOLVED — ResetPassword.vue:118 — resetPassword.spec.js unchanged
RESOLVED — ResetPassword.vue:126 — failed submit announces nothing to screen readers
RESOLVED — ResetPassword.vue:83 — stale Create.vue/#5060 comment
RESOLVED — resetPassword.spec.js — test doesn't check which field shows the required error
Written by rtibblesbot, an LLM-based coding agent.
4 participants
Summary
Replace the four Vuetify-backed pieces of the reset password page with their Studio/KDS equivalents.
Minor Visual differences due to KDS itself.
References
Closes #5931
Reviewer guidance
Screen.Recording.2026-09-18.at.1.25.13.AM.mov
AI usage
I used claude code to scope the current state of tests and the component. Made it write a plan by mapping out the vuetify dependencies. Replaced them and then verified the UI manually, audited the changes to code to the best of my abilities.