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

Allow people to create passwords during registration #91

Merged
merged 6 commits into from
Feb 3, 2022

Conversation

ianconsolata
Copy link
Contributor

@ianconsolata ianconsolata commented Jan 30, 2022

Some considerations before merging:

  • What is the /login page for? When I go to it in my browser the style is way off (for some reasons it is including the header). But functionally, it seems to be doing the same thing as the /register page. Should I update it to accept a password as well, or should I just remove it?
  • What do we actually want the validation / format for the username to be? For now, I just implemented the NSS requirements as stated ("Your username should only contain lowercase letters a-z and numbers 0-9 and without periods or spaces."), but I think it's a little too restrictive and we could maybe relax it a bit? Not totally sure what the downstream effects of that may be though. I think the only thing that this matters for is generating valid subdomains for the username, but I'm not certain.

@vercel
Copy link

vercel bot commented Jan 30, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/mysilio/garden/EUAHQrCbCaBUs2gkZtRPNxVrsANu
✅ Preview: https://garden-git-pages-registerjsx.mysilio.page

I am planning to copy over the data from the prod pod host to the
staging one before merging this PR, so in practice this
change shouldn't be noticed since the data will match. Either way, it
will only affect users of the preview environments, whcih I think is
just us.
@travis
Copy link
Contributor

travis commented Feb 1, 2022

looks good to me! I think I fixed the login page formatting in another commit so it should look at least a little better. It does feel like it will be useful to have a "login" page that's separate from register - even if they look about the same now they will diverge in the future with different messaging and affordances.

re: user names, going with NSS rules now sounds right, I don't have many preconceptions about the "right" rules for a username but happy to chat about that whenever!

@travis travis self-assigned this Feb 1, 2022
@ianconsolata
Copy link
Contributor Author

ianconsolata commented Feb 2, 2022

@travis you're also okay with the breaking change to switch to using the Staging pod host for all preview envs? That was in a later commit, but I forgot to note it in the main PR description. I think it will help us keep cleaner data state as we move to the new garden format, since we won't have to store the env folders anymore.

@travis
Copy link
Contributor

travis commented Feb 3, 2022

yep! sounds like the right thing to me, would have thought it already worked that way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants