Move password reset form to separate route, view #1390

alectrocute · 2023-06-19T13:03:41Z

Hi Lemdevs!

Our password reset form has not the greatest UX at the moment. It appears like it's a link, but it's actually a button that takes the email field's value and resets the password upon click. There's also critical instructions hidden in a title attr. It just doesn't make much sense to me.

I propose we:

Move the password reset form to a new route.

Translation PR to merge first:

LemmyNet/lemmy-translations#67

Thanks all!

src/shared/components/home/login-reset.tsx

alectrocute · 2023-06-19T14:57:42Z

Just realized something: We also should redirect back to the login route on success.

…ctrocute/lemmy-ui into refactor-password-reset-form

alectrocute · 2023-06-19T15:16:55Z

Made the changes. Accidentally started tracking lemmy-translations. How do I remove this from my PR? Also, should we git ignore this?

SleeplessOne1917

Besides my other comments, try using the latest version of the translations by running git submodule update --remote --recursive.

SleeplessOne1917 · 2023-06-19T23:41:22Z

src/shared/components/home/login-reset.tsx

+          title={this.documentTitle}
+          path={this.context.router.route.match.url}
+        />
+        <div className="row">


If there's only going to be one row, why not make the parent div have a className of container-lg row and get rid of this element?

Primarily to make it match up with /login. I've refactored this in my upcoming changes.

SleeplessOne1917 · 2023-06-19T23:41:50Z

src/shared/components/home/login-reset.tsx

+
+  loginResetForm() {
+    return (
+      <div>


Is this div necessary?

SleeplessOne1917 · 2023-06-19T23:43:57Z

src/shared/components/home/login-reset.tsx

+          <h5>{capitalizeFirstLetter(i18n.t("forgot_password"))}</h5>
+
+          <div className="form-group row">
+            <label className="col-form-label col-sm-10">


Does this have to be a form-group with a label as a child? I typically think of label elements being associated with input elements.

Once the bootstrap 5 stuff gets merged shortly, it'll have to conform to the way it does form anyway, and might need changes.

SleeplessOne1917 · 2023-06-19T23:45:58Z

src/shared/components/home/login-reset.tsx

+          </div>
+
+          <div className="form-group row">
+            <label


What would this label look like if it was a child of the same div as the input its for?

As long as it follows the bootstrap conventions, it should be okay.

It would look like this @SleeplessOne1917

src/shared/components/home/login-reset.tsx

SleeplessOne1917 · 2023-06-20T00:22:29Z

Also, this PR makes me wonder what the verify email page is for.

alectrocute · 2023-06-20T00:42:26Z

@SleeplessOne1917 I was thinking about this. Should we just make it part of the login page?

SleeplessOne1917 · 2023-06-20T00:43:20Z

@dessalines @Nutomic any input on this?

alectrocute · 2023-06-20T00:59:03Z

Also, this PR makes me wonder what the verify email page is for.

It’s the page linked to in the password reset email.

dessalines · 2023-06-20T11:47:12Z

VerifyEmail is when the back-end sends you an email with a token, that links to that verifyemail page. When you load it, it should send that token back to the server, confirming that you received the email and verified it.

dessalines · 2023-06-20T11:49:34Z

src/shared/components/home/login-reset.tsx

+          <h5>{capitalizeFirstLetter(i18n.t("forgot_password"))}</h5>
+
+          <div className="form-group row">
+            <label className="col-form-label col-sm-10">


Once the bootstrap 5 stuff gets merged shortly, it'll have to conform to the way it does form anyway, and might need changes.

dessalines · 2023-06-20T11:51:11Z

src/shared/components/home/login-reset.tsx

+          </div>
+
+          <div className="form-group row">
+            <label


As long as it follows the bootstrap conventions, it should be okay.

src/shared/components/home/login-reset.tsx

…efactor-password-reset-form

dessalines

LGTM

SleeplessOne1917 · 2023-06-21T00:34:32Z

src/shared/components/home/login-reset.tsx

+  handleEmailInputChange(i: LoginReset, event: any) {
+    i.setState(s => ((s.form.email = event.target.value.trim()), s));
+  }
+
+  async handlePasswordReset(i: LoginReset, event: any) {
+    event.preventDefault();
+
+    const email = i.state.form.email;
+
+    if (email && validEmail(email)) {
+      i.setState(s => ((s.form.loading = true), s));
+
+      const res = await HttpService.client.passwordReset({ email });
+
+      if (res.state == "success") {
+        toast(i18n.t("reset_password_mail_sent"));
+        i.context.router.history.push("/login");
+      }
+
+      i.setState(s => ((s.form.loading = false), s));
+    }
+  }


These can be defined at the module level instead of as class methods since they get this passed in.

Where would the module level be exactly, @SleeplessOne1917? In a @utils/app export? I'm sorry, still a bit of a Inferno noob.

I very much prefer the class methods even if this gets passed in, if only for organizations-sakes.

dessalines · 2023-06-26T18:33:44Z

Once you get the conflicts handled we can merge this.

alectrocute · 2023-06-26T18:50:22Z

@dessalines We should be good to go.

dessalines

Sweet, thx.

* lemmy/main: fix vote button alignment Fix feedback on banning an unbanning remove icon (LemmyNet#1618) Indicate valid and invalid fields in signup form (LemmyNet#1450) capitalize button (LemmyNet#1616) Move password reset form to separate route, view (LemmyNet#1390) feat(UI): Reduce base font size (LemmyNet#1591) Fix: missing semantic css classes and html elements (LemmyNet#1583)

* rework password reset form * make self-suggested changes * cleaning * validate in handlePasswordReset as well * update submodule * partially make suggested changes * make suggested changes * resolve merge conflicts * resolve merge conflicts * resolve merge conflicts --------- Co-authored-by: Dessalines <dessalines@users.noreply.github.com>

* lemmy/main: fix vote button alignment partially revert change Fix feedback on banning an unbanning remove icon (LemmyNet#1618) Indicate valid and invalid fields in signup form (LemmyNet#1450) capitalize button (LemmyNet#1616) Move password reset form to separate route, view (LemmyNet#1390) feat(UI): Reduce base font size (LemmyNet#1591) Fix: missing semantic css classes and html elements (LemmyNet#1583) remove hook entirely chore(DX): Add prettier to eslint config for use with editors fix bug collapsing previews when voting fix: Remove unnecessary string interpolations fix: Remove unnecessary class fix: Remove unnecessary classes fix: Restore removed classes fix: Remove wrapping li's fix: Remove extraneous classes fix: Move things back to where they were chore: Separate post mod buttons into functions

rework password reset form

f3c3119

alectrocute requested review from dessalines and SleeplessOne1917 as code owners June 19, 2023 13:03

alectrocute mentioned this pull request Jun 19, 2023

Add reset_password string LemmyNet/lemmy-translations#67

Merged

Merge branch 'main' into refactor-password-reset-form

54c8961

alectrocute commented Jun 19, 2023

View reviewed changes

src/shared/components/home/login-reset.tsx Show resolved Hide resolved

alectrocute added 3 commits June 19, 2023 11:12

make self-suggested changes

12c3408

Merge branch 'refactor-password-reset-form' of https://github.com/ale…

33b19f7

…ctrocute/lemmy-ui into refactor-password-reset-form

cleaning

cbed4be

alectrocute and others added 3 commits June 19, 2023 11:19

validate in handlePasswordReset as well

2a5c247

Merge branch 'main' into refactor-password-reset-form

f4d6a04

Merge branch 'main' into refactor-password-reset-form

2cf2ab1

SleeplessOne1917 reviewed Jun 19, 2023

View reviewed changes

dessalines reviewed Jun 20, 2023

View reviewed changes

alectrocute added 4 commits June 20, 2023 15:07

update submodule

ef5fe32

partially make suggested changes

29c03a1

Merge branch 'main' of https://github.com/alectrocute/lemmy-ui into r…

523558e

…efactor-password-reset-form

make suggested changes

20b515b

alectrocute requested review from dessalines and SleeplessOne1917 June 20, 2023 19:33

dessalines approved these changes Jun 20, 2023

View reviewed changes

SleeplessOne1917 reviewed Jun 21, 2023

View reviewed changes

dessalines approved these changes Jun 26, 2023

View reviewed changes

Merge branch 'main' into refactor-password-reset-form

f02c913

alectrocute requested a review from jsit as a code owner June 26, 2023 18:42

alectrocute added 3 commits June 26, 2023 14:45

resolve merge conflicts

db4fc43

resolve merge conflicts

a6fd296

resolve merge conflicts

ec2a19e

alectrocute added this to the 0.18.1 milestone Jun 26, 2023

dessalines approved these changes Jun 26, 2023

View reviewed changes

Merge branch 'main' into refactor-password-reset-form

b76bc11

dessalines merged commit 96cf021 into LemmyNet:main Jun 26, 2023
1 check was pending

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move password reset form to separate route, view #1390

Move password reset form to separate route, view #1390

alectrocute commented Jun 19, 2023 •

edited

alectrocute commented Jun 19, 2023

alectrocute commented Jun 19, 2023

SleeplessOne1917 left a comment

SleeplessOne1917 Jun 19, 2023

alectrocute Jun 20, 2023

SleeplessOne1917 Jun 19, 2023

SleeplessOne1917 Jun 19, 2023

dessalines Jun 20, 2023

SleeplessOne1917 Jun 19, 2023

dessalines Jun 20, 2023

alectrocute Jun 20, 2023

SleeplessOne1917 commented Jun 20, 2023

alectrocute commented Jun 20, 2023 •

edited

SleeplessOne1917 commented Jun 20, 2023

alectrocute commented Jun 20, 2023

dessalines commented Jun 20, 2023

dessalines Jun 20, 2023

dessalines Jun 20, 2023

dessalines left a comment

SleeplessOne1917 Jun 21, 2023

alectrocute Jun 21, 2023

dessalines Jun 26, 2023

dessalines commented Jun 26, 2023

alectrocute commented Jun 26, 2023

dessalines left a comment

Move password reset form to separate route, view #1390

Move password reset form to separate route, view #1390

Conversation

alectrocute commented Jun 19, 2023 • edited

alectrocute commented Jun 19, 2023

alectrocute commented Jun 19, 2023

SleeplessOne1917 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SleeplessOne1917 commented Jun 20, 2023

alectrocute commented Jun 20, 2023 • edited

SleeplessOne1917 commented Jun 20, 2023

alectrocute commented Jun 20, 2023

dessalines commented Jun 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dessalines left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dessalines commented Jun 26, 2023

alectrocute commented Jun 26, 2023

dessalines left a comment

Choose a reason for hiding this comment

alectrocute commented Jun 19, 2023 •

edited

alectrocute commented Jun 20, 2023 •

edited