Skip to content

Remove Vuetify from Forgot password page - #6334

Merged
akolson merged 6 commits into
learningequality:unstablefrom
adibmbrk:remove-vuetify-forgot-password
Nov 2, 2026
Merged

akolson merged 6 commits into
learningequality:unstablefrom
adibmbrk:remove-vuetify-forgot-password

Conversation

@adibmbrk

@adibmbrk adibmbrk commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Removes Vuetify from the Forgot password page as part of #5060.

  • MessageLayout → StudioMessageLayout
  • VForm → <form> with generateFormMixin validation, following RequestNewActivationLink
  • Banner → StudioBanner
  • EmailField → StudioEmailField
  • Email errors appear only after blur or submit, matching the previous validate-on-blur behaviour
  • StudioMessageLayout gets width: 100%, so its forms fit small screens. This also fixes the same overflow on the activation link page
  • The error banner has role="alert", so screen readers announce it
  • Unit tests updated to check validation messages, blur behaviour and the error banner

References

Closes #5932

Reviewer guidance

  1. Go to /accounts/#/forgot-password
  2. Submit with an empty email → "This field is required"
  3. Type an invalid email → no error until blur or submit
  4. Submit with a valid email → redirects to the instructions sent page
  5. To see the error banner, temporarily make sendPasswordResetLink reject

Expected UI changes: the error banner has black text and an error icon, plus minor KDS styling differences.

Before

Screenshot 2026-10-28 at 06 27 49

After

Screenshot 2026-10-28 at 06 23 54

AI usage

Used Claude Code to plan, implement and test the change.

@adibmbrk
adibmbrk requested review from a team and leq-bot October 26, 2026 10:48
@learning-equality-bot

Copy link
Copy Markdown
Contributor

👋 Hi @adibmbrk, thanks for contributing!

For the review process to begin, please verify that the following is satisfied:

  • Contribution is aligned with our contributing guidelines

  • Pull request description has correctly filled AI usage section & follows our AI guidance:

    AI guidance

    State explicitly whether you didn't use or used AI & how.

    If you used it, ensure that the PR is aligned with Using AI as well as our DEEP framework. DEEP asks you:

    • Disclose — Be open about when you've used AI for support.
    • Engage critically — Question what is generated. Review code for correctness and unnecessary complexity.
    • Edit — Review and refine AI output. Remove unnecessary code and verify it still works after your edits.
    • Process sharing — Explain how you used the AI so others can learn.

    Examples of good disclosures:

    "I used Claude Code to implement the component, prompting it to follow the pattern in ComponentX. I reviewed the generated code, removed unnecessary error handling, and verified the tests pass."

    "I brainstormed the approach with Gemini, then had it write failing tests for the feature. After reviewing the tests, I used Claude Code to generate the implementation. I refactored the output to reduce verbosity and ran the full test suite."

📢✨ Before we assign a reviewer:

  • @rtibblesbot will pre-review this pull request. Its comments are generated by an LLM, and should be evaluated accordingly.
  • We'll also invite community pre-review. See the community review guidance for both authors and reviewers.

Also check that issue requirements are satisfied & you ran pre-commit locally.

Pull requests that don't follow the guidelines will be closed.

Reviewer assignment can take up to 2 weeks.

@rtibblesbot

rtibblesbot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

🟡 Waiting for changes

Outcome: reviewed a0946d7

Last updated: 2026-11-02 16:35 UTC

@rtibblesbot rtibblesbot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@rtibblesbot rtibblesbot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@rtibblesbot
rtibblesbot dismissed their stale review October 26, 2026 11:25

Superseded by my review of 3c46df4: no blocking findings remain.

@akolson

akolson commented Oct 27, 2026

Copy link
Copy Markdown
Member

@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.

@adibmbrk

Copy link
Copy Markdown
Contributor Author

Done @akolson

@akolson akolson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@adibmbrk

adibmbrk commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Hi @akolson! I have addressed the review comments

@rtibblesbot rtibblesbot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 akolson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adibmbrk Thanks for the quick follow-up. All three points are addressed:

  • The form now fits small screens, because StudioMessageLayout gets width: 100% (3dbf8df).
  • The error banner has role="alert", and the test finds it by role (738c56e).
  • The tests use userEvent and createTranslator (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.

@akolson
akolson merged commit 865e06b into learningequality:unstable Nov 2, 2026
22 checks passed
@rtibblesbot

Copy link
Copy Markdown
Contributor

Quick check on rtibblesbot's review of this PR: react 👍 helpful · 😕 mixed · 👎 not helpful. Reply with anything specific.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Remove Vuetify from Studio] 'Forgot password' page

3 participants