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

Instead of forcing login, display a message offering users the chance to login #1082

Merged
merged 14 commits into from
Feb 22, 2024

Conversation

theosanderson
Copy link
Member

@theosanderson theosanderson commented Feb 22, 2024

related to #829

preview URL: https://nice-login.loculus.org/

Summary

Now for the submit and revise pages we use a standardised Astro component to display a page, with a link to Login. In due course we can add Register to this.

Screenshot

image

PR Checklist

  • All necessary documentation has been adapted.
  • The implemented feature is covered by an appropriate test.

@theosanderson theosanderson added the preview Triggers a deployment to argocd label Feb 22, 2024
@theosanderson theosanderson marked this pull request as draft February 22, 2024 15:44
@theosanderson theosanderson marked this pull request as ready for review February 22, 2024 16:58
@theosanderson theosanderson changed the title Attempts to handle log in nicely Instead of forcing login, display a message offering users the chance to login Feb 22, 2024
Copy link
Contributor

@fengelniederhammer fengelniederhammer left a comment

Choose a reason for hiding this comment

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

Looks good in general 👍

website/src/pages/[organism]/submit/review.astro Outdated Show resolved Hide resolved
website/src/pages/[organism]/submit/index.astro Outdated Show resolved Hide resolved
website/src/components/common/NeedToLogin.astro Outdated Show resolved Hide resolved
website/src/utils/shouldMiddlewareEnforceLogin.ts Outdated Show resolved Hide resolved
website/src/components/common/NeedToLogin.astro Outdated Show resolved Hide resolved
@corneliusroemer
Copy link
Contributor

Doesn't this increase the number of clicks required for users to complete common flows? Is this pattern of asking whether to log in more polite? Not sure I have a strong opinion, but I'd generally favour fewer clicks.

@theosanderson
Copy link
Member Author

theosanderson commented Feb 22, 2024

It's really hostile for a new user to click "Submit" and find themselves redirected to a page to login to an account they don't even have. Mostly one handles that by not showing users buttons that don't apply to them in their current not logged in state, but here we decided we wanted to always show the button to highlight that you can submit. If so we need a pattern like this. We should keep users logged in for a good length of time.

@theosanderson theosanderson merged commit dfff93e into main Feb 22, 2024
12 checks passed
@theosanderson theosanderson deleted the nice-login branch February 22, 2024 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview Triggers a deployment to argocd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants