Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed registration form, cleaned up login flow #3821

Merged
merged 5 commits into from Mar 22, 2024

Conversation

wes-otf
Copy link
Contributor

@wes-otf wes-otf commented Mar 19, 2024

Description

Fixes #3813. Main changes:

  • Got rid of the registration form in favor of using the /auth/ view. Having two places where the user can potentially register is confusing.
  • Cleaned up different aspects of the login flow that were mentioned in Login and signup form improvements #3813, like inconsistency in terminology (Login vs Log in), removal of buttons where they don't need to be, etc.
  • Added a button on the password-ed login to bring the user back to the passwordless log in/register view.
  • Have buttons in column rather than row on both mobile & desktop view and made them the same width (18 rem).
    • Should we center the login page for desktop? This was done as it felt awkward to have everything on the left side while the buttons crept to the center
  • Added icons to Log in with <ORG> email & Log in or register via email for consistency
  • Moved login buttons to their own templates as some were being reused
  • Added missing translation blocks

Screenshots

Desktop Views

Homepage
Screenshot 2024-03-19 at 17 40 58


Passwordless Login
Screenshot 2024-03-19 at 17 09 34


Password-ed Login
Screenshot 2024-03-19 at 17 09 53


Mobile Views
Homepage


Passwordless Login


Password-ed Login

Test Steps

  • Ensure you're logged out
  • On the home page, confirm a Log in button appears in the top right corner
  • Select the Log in button, and confirm you get taken to the passwordless login page (/auth/)
  • Select the Log in with password button and confirm you get taken to the password-ed page (/login/)
  • Confirm there is a button at the bottom of the page reading Log in without password
  • Select this button and confirm you get taken to the passwordless login page (/auth/)

@frjo
Copy link
Contributor

frjo commented Mar 20, 2024

Nice work, and fast!

I found three issues I think we can fix in this PR.

On the front page we show a login button and you can signup with that form but I think the button should express that. Maybe "Login or Signup" when ENABLE_PUBLIC_SIGNUP is set?

Skärmavbild 2024-03-20 kl  09 47 52

The button back to the e-mail only login should take ENABLE_PUBLIC_SIGNUP in to account.

Skärmavbild 2024-03-20 kl  09 47 17

The password reset form needs work as well.

Skärmavbild 2024-03-20 kl  10 00 49

@wes-otf
Copy link
Contributor Author

wes-otf commented Mar 20, 2024

@frjo great catches, thanks for taking a look! All should be resolved now, along with some CSS cleanup & addition translation tags added.

It's weird stylelint flagged my variable use with that media-query-no-invalid. Some quick research suggests that rule should only apply to plain CSS

@frjo frjo added Type: Enhancement This is an improvement of an existing thing (not a new thing, which would be a feature). Type: Major Major change, used in release drafter labels Mar 21, 2024
@frjo frjo force-pushed the enhancement/login-signup-improvements branch from 7f66695 to f96cb89 Compare March 21, 2024 13:46
@frjo frjo added Status: Needs testing Tickets that need testing/qa Status: Needs dev testing 🧑‍💻 Tasks that should be tested by the dev team labels Mar 21, 2024
@frjo
Copy link
Contributor

frjo commented Mar 21, 2024

I rebased this PR and made some minor changes to text of a button.

@@ -4,5 +4,5 @@
href="{% url 'users:passwordless_login_signup' %}{% if next %}?next={{next}}{% endif %}"
>
{% heroicon_mini "envelope" size=18 class="inline align-text-bottom me-1" aria_hidden=true %}
{% trans "Log in " %}{% if ENABLE_PUBLIC_SIGNUP %} {% trans "or" %} {% trans "Sign in" %}{% endif %} {% trans "via" %} {% trans "e-mail" %}
{% trans "Log in without password" %}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like this a lot better, should we indicate that this is where you would also go to signup though as that isn't given anywhere else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(if enabled)

Copy link
Contributor

Choose a reason for hiding this comment

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

My thinking was that if you clicked your way to the password login form you are most likely not needing to create an account.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fair enough, all good by me!

@frjo frjo merged commit 9a9a9e8 into main Mar 22, 2024
10 checks passed
@frjo frjo added Type: Minor Minor change, used in release drafter and removed Type: Major Major change, used in release drafter labels Mar 22, 2024
@theskumar theskumar deleted the enhancement/login-signup-improvements branch March 25, 2024 12:37
wes-otf added a commit to OpenTechFund/apply-app that referenced this pull request Apr 1, 2024
Fixes HyphaApp#3813. Main changes:

- [x] Got rid of the registration form in favor of using the `/auth/`
view. Having two places where the user can potentially register is
confusing.
- [x] Cleaned up different aspects of the login flow that were mentioned
in HyphaApp#3813, like inconsistency in terminology (`Login` vs `Log in`),
removal of buttons where they don't need to be, etc.
- [x] Added a button on the password-ed login to bring the user back to
the passwordless log in/register view.
- [x] Have buttons in column rather than row on *both* mobile & desktop
view and made them the same width (`18 rem`).
- Should we center the login page for desktop? This was done as it felt
awkward to have everything on the left side while the buttons crept to
the center
- [x] Added icons to `Log in with <ORG> email` & `Log in or register via
email` for consistency
- [x] Moved login buttons to their own templates as some were being
reused
- [x] Added missing translation blocks
wes-otf added a commit to OpenTechFund/apply-app that referenced this pull request Apr 1, 2024
Fixes HyphaApp#3813. Main changes:

- [x] Got rid of the registration form in favor of using the `/auth/`
view. Having two places where the user can potentially register is
confusing.
- [x] Cleaned up different aspects of the login flow that were mentioned
in HyphaApp#3813, like inconsistency in terminology (`Login` vs `Log in`),
removal of buttons where they don't need to be, etc.
- [x] Added a button on the password-ed login to bring the user back to
the passwordless log in/register view.
- [x] Have buttons in column rather than row on *both* mobile & desktop
view and made them the same width (`18 rem`).
- Should we center the login page for desktop? This was done as it felt
awkward to have everything on the left side while the buttons crept to
the center
- [x] Added icons to `Log in with <ORG> email` & `Log in or register via
email` for consistency
- [x] Moved login buttons to their own templates as some were being
reused
- [x] Added missing translation blocks
wes-otf added a commit that referenced this pull request May 7, 2024
Fixes #3813. Main changes:

- [x] Got rid of the registration form in favor of using the `/auth/`
view. Having two places where the user can potentially register is
confusing.
- [x] Cleaned up different aspects of the login flow that were mentioned
in #3813, like inconsistency in terminology (`Login` vs `Log in`),
removal of buttons where they don't need to be, etc.
- [x] Added a button on the password-ed login to bring the user back to
the passwordless log in/register view.
- [x] Have buttons in column rather than row on *both* mobile & desktop
view and made them the same width (`18 rem`).
- Should we center the login page for desktop? This was done as it felt
awkward to have everything on the left side while the buttons crept to
the center
- [x] Added icons to `Log in with <ORG> email` & `Log in or register via
email` for consistency
- [x] Moved login buttons to their own templates as some were being
reused
- [x] Added missing translation blocks
wes-otf added a commit that referenced this pull request May 8, 2024
Fixes #3813. Main changes:

- [x] Got rid of the registration form in favor of using the `/auth/`
view. Having two places where the user can potentially register is
confusing.
- [x] Cleaned up different aspects of the login flow that were mentioned
in #3813, like inconsistency in terminology (`Login` vs `Log in`),
removal of buttons where they don't need to be, etc.
- [x] Added a button on the password-ed login to bring the user back to
the passwordless log in/register view.
- [x] Have buttons in column rather than row on *both* mobile & desktop
view and made them the same width (`18 rem`).
- Should we center the login page for desktop? This was done as it felt
awkward to have everything on the left side while the buttons crept to
the center
- [x] Added icons to `Log in with <ORG> email` & `Log in or register via
email` for consistency
- [x] Moved login buttons to their own templates as some were being
reused
- [x] Added missing translation blocks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs dev testing 🧑‍💻 Tasks that should be tested by the dev team Status: Needs testing Tickets that need testing/qa Type: Enhancement This is an improvement of an existing thing (not a new thing, which would be a feature). Type: Minor Minor change, used in release drafter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Login and signup form improvements
2 participants