Repository navigation
Remove Vuetify from Forgot password page - #6334
Conversation
Signed-off-by: adibmbrk <[email protected]>
|
👋 Hi @adibmbrk, thanks for contributing! For the review process to begin, please verify that the following is satisfied:
📢✨ Before we assign a reviewer:
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. |
🟡 Waiting for changesOutcome: reviewed Last updated: 2026-11-02 16:35 UTC |
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #6334 is blocked on missing screenshots. The code follows the merged RequestNewActivationLink.vue pattern (#6026) and covers all four replacements the issue lists.
CI: only Pull Request Template / Check template fails, on the screenshot placeholder. Manual QA did not run.
- blocking: screenshots missing. The PR body still has
<!-- TODO: add screenshots -->. The issue's acceptance criteria require screenshots. Add before/after for the default state, the required-field error, the invalid-email error and the error banner (force it temporarily). Add RTL and mobile widths too. - suggestion: validation now runs on every keystroke; note it in the PR body (inline).
- suggestion: validation-message assertions match on text alone; scope them to the email field (inline).
Next step: Needs a full code review by a human engineer, then manual QA once that reviewer sets Needs QA.
- The author is outside the team: confirm this should be built at all.
- User-visible files changed:
contentcuration/contentcuration/frontend/accounts/pages/resetPassword/ForgotPassword.vue. - Check that the required-field error, the invalid-email error and the error banner render correctly at mobile width and in RTL.
- Confirm that errors showing on each keystroke, rather than on blur, is acceptable.
Written by rtibblesbot, an LLM-based coding agent.
Signed-off-by: adibmbrk <[email protected]>
Signed-off-by: adibmbrk <[email protected]>
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #6334: both prior findings are resolved. Manual QA did not run. The UI is unverified.
Prior-finding status
RESOLVED — contentcuration/contentcuration/frontend/accounts/pages/resetPassword/ForgotPassword.vue:25 — validation runs on every keystroke instead of on blur
RESOLVED — contentcuration/contentcuration/frontend/accounts/pages/tests/forgotPassword.spec.js:50 — text assertions match anywhere on the page
Next step: Needs a full code review by a human engineer, then manual QA once that reviewer sets Needs QA.
- The author is outside the team: confirm this should be built at all.
- User-visible files changed:
contentcuration/contentcuration/frontend/accounts/pages/resetPassword/ForgotPassword.vue. - Check the email error appears only after blur or submit, and the reset link sends for a valid email.
Written by rtibblesbot, an LLM-based coding agent.
Superseded by my review of 3c46df4: no blocking findings remain.
|
@adibmbrk Thanks for your work on this PR! Could you please update it to use the required PR template? Also be sure add screenshots to help reviewers assess your changes more thoroughly, as this is frontend focused issue. |
|
Done @akolson |
akolson
left a comment
There was a problem hiding this comment.
Thanks @adibmbrk! The migration follows the issue and the RequestNewActivationLink page closely, and the blur behaviour matches the old field.
CI passing. Manual QA: empty submit, the blur error, the valid submit, the failure banner and RTL all work, with no console errors.
Signed-off-by: adibmbrk <[email protected]>
Signed-off-by: adibmbrk <[email protected]>
Signed-off-by: adibmbrk <[email protected]>
|
Hi @akolson! I have addressed the review comments |
rtibblesbot
left a comment
There was a problem hiding this comment.
PR #6334: all 3 prior findings are resolved. No new findings. CI is passing. Manual QA did not run, so the UI is unverified.
Prior-finding status
RESOLVED — contentcuration/contentcuration/frontend/accounts/pages/resetPassword/ForgotPassword.vue — validation runs on every keystroke
RESOLVED — contentcuration/contentcuration/frontend/accounts/pages/tests/forgotPassword.spec.js — validation assertions match text anywhere on the page
RESOLVED — contentcuration/contentcuration/frontend/accounts/pages/tests/forgotPassword.spec.js — praise: banner text and its absence on default render
Next step: Needs a full code review by a human engineer, then manual QA once that reviewer sets Needs QA.
- The author is outside the team: confirm this should be built at all.
- User-visible files changed:
contentcuration/contentcuration/frontend/accounts/components/StudioMessageLayout.vue,contentcuration/contentcuration/frontend/accounts/pages/resetPassword/ForgotPassword.vue. - Check that an email error appears only on blur or submit, not while typing.
- Check that the forgot password form fills the full width on small screens.
- Check that a screen reader announces the error banner when it appears.
Written by rtibblesbot, an LLM-based coding agent.
akolson
left a comment
There was a problem hiding this comment.
@adibmbrk Thanks for the quick follow-up. All three points are addressed:
- The form now fits small screens, because
StudioMessageLayoutgetswidth: 100%(3dbf8df). - The error banner has
role="alert", and the test finds it by role (738c56e). - The tests use
userEventandcreateTranslator(a0946d7).
CI passes.
I updated the description with two lines for the StudioMessageLayout width fix and the new role="alert" on the error banner, because both changes go further than the field swaps it listed.
|
Quick check on rtibblesbot's review of this PR: react 👍 helpful · 😕 mixed · 👎 not helpful. Reply with anything specific. |
Summary
Removes Vuetify from the Forgot password page as part of #5060.
MessageLayout→StudioMessageLayoutVForm→<form>withgenerateFormMixinvalidation, followingRequestNewActivationLinkBanner→StudioBannerEmailField→StudioEmailFieldvalidate-on-blurbehaviourStudioMessageLayoutgetswidth: 100%, so its forms fit small screens. This also fixes the same overflow on the activation link pagerole="alert", so screen readers announce itReferences
Closes #5932
Reviewer guidance
/accounts/#/forgot-passwordsendPasswordResetLinkrejectExpected UI changes: the error banner has black text and an error icon, plus minor KDS styling differences.
Before
After
AI usage
Used Claude Code to plan, implement and test the change.