Change /send_password_reset to /forgot_password #19

notplants · 2021-11-10T11:38:32Z

notplants commented

2021-11-10 11:38:32 +00:00

This PR is a small change.

Changed the name of /send_password_reset to /forgot_password, which I thought was more idiomatic, and less likely to be confused with the /reset_password page (the page where you actually reset your password, once you get the secret link).

I also removed the "cancel" button from the login page (not really sure what it means to cancel, when logging in),

and added a "Forgot Password?" gray link at 50px below the login b utton.

This PR is a small change. Changed the name of /send_password_reset to /forgot_password, which I thought was more idiomatic, and less likely to be confused with the /reset_password page (the page where you actually reset your password, once you get the secret link). I also removed the "cancel" button from the login page (not really sure what it means to cancel, when logging in), and added a "Forgot Password?" gray link at 50px below the login b utton.

notplants added 1 commit 2021-11-10 11:38:33 +00:00

Change /send_password_reset to /forgot_password a4f459e1fc

notplants commented

2021-11-10 12:09:11 +00:00

![](https://server.commoninternet.net/nextcloud/apps/files_sharing/publicpreview/nBprNRSNZSqKsRa?x=2514&y=734&a=true&file=Screen%2520Shot%25202021-11-10%2520at%252012.32.10%2520PM.png&scalingup=0https://)

notplants requested review from glyph 2021-11-10 12:09:44 +00:00

glyph requested changes 2021-11-10 12:44:55 +00:00

glyph left a comment

Besides two minor styling change requests it all looks great!

peach-web/templates/login.html.tera Outdated

						
				@ -26,2 +26,4 @@

				          <div class="capsule center-text flash-message font-failure">{{ flash_msg }}.</div>

				          {%- endif -%}

				         <div class="forgot-password center-text" style="margin-top: 50px;">

glyph commented

2021-11-10 12:44:02 +00:00

Some small (opinionated) styling suggestions here:

I would recommend setting margin-top: 25px instead of 50px.

Then, on line 30, remove the entire style=" ... " section and use classes from our pattern library instead: class="label-small link". The CSS styles in the library definitely need some work but I quite like sticking with classes whenever possible (makes it much easier to deploy changes across the entire interface by only tweaking a class or two, rather than having to manually edit the style of each individual element).

That will make the font near-black for now. We can set it to gray once I've fixed the classes :)

Some small (opinionated) styling suggestions here: I would recommend setting `margin-top: 25px` instead of `50px`. Then, on line 30, remove the entire `style=" ... "` section and use classes from our pattern library instead: `class="label-small link"`. The CSS styles in the library definitely need some work but I quite like sticking with classes whenever possible (makes it much easier to deploy changes across the entire interface by only tweaking a class or two, rather than having to manually edit the style of each individual element). That will make the font near-black for now. We can set it to gray once I've fixed the classes :)

glyph commented

2021-11-10 12:49:02 +00:00

Oh yeah, one other thing I noticed. You have forgot-password as a class on L29. Perhaps this is meant to be the id attribute value?

Oh yeah, one other thing I noticed. You have `forgot-password` as a class on L29. Perhaps this is meant to be the `id` attribute value?

notplants commented

2021-11-10 13:57:12 +00:00

Thanks, made these changes. I'm not totally aware of all of the classes are available in the pattern library, but will hopefully become aware of more of them over time through code reviews. Please feel free to point them out and ways to bring the css more in alignment with the rest of the codebase.

I don't have any preference for gray or near-black, as you feel. The font looks better now too that it matches.

Thanks, made these changes. I'm not totally aware of all of the classes are available in the pattern library, but will hopefully become aware of more of them over time through code reviews. Please feel free to point them out and ways to bring the css more in alignment with the rest of the codebase. I don't have any preference for gray or near-black, as you feel. The font looks better now too that it matches.

❤️ 1

notplants commented

2021-11-10 13:57:33 +00:00

forgot-password was accidentally put as a class name while I was trying to figure out how to style it lol

😆 1

glyph marked this conversation as resolved

notplants added 1 commit 2021-11-10 13:53:24 +00:00

Changes to CSS ae9254c0c6

notplants added 1 commit 2021-11-10 13:57:52 +00:00

Merge branch 'main' into forgot-password 982d5d2811

notplants merged commit 82a87706b7 into main

2021-11-10 13:58:18 +00:00