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

Add sign up link to login screen and fix registration form validation #1395

Merged

Conversation

3 participants
@cw1998
Copy link
Contributor

commented Apr 16, 2019

I think people expect to find a sign up link close to the login form, so added one next to forgot password if registration is enabled. Also fixed #1239 while I was there.

image

image

Hope this helps.

@JtheBAB

This comment has been minimized.

Copy link

commented Apr 17, 2019

I would vote that this "sign up" link can be hidden with the admin interface.

@cw1998

This comment has been minimized.

Copy link
Contributor Author

commented Apr 17, 2019

@JtheBAB the sign up link the header can be hidden by disabling new registrations in the admin interface. The link here will behave in the same way. I may be misunderstanding - do you mean hiding the link while keeping registration enabled?

@JtheBAB

This comment has been minimized.

Copy link

commented Apr 18, 2019

Ah ok. Then it is ok :)
I was looking in my interface and somehow missed this....

@ssddanbrown

This comment has been minimized.

Copy link
Member

commented Apr 18, 2019

Thanks @cw1998 for this and for also writing the tests, That's great.

The "Already have an account?" link looks great. I might play around with the "Sign up" link placement if that's okay? It seems a little out-of-place next to the "Forgot password?" link, which has it's position very focused on the context of the task (Under the password field).

@cw1998

This comment has been minimized.

Copy link
Contributor Author

commented Apr 18, 2019

Hi @ssddanbrown , yeah absolutely. The placement seemed tidier than adding a new line, but I'm not 100% happy with the use of the bullet point to separate it.

I actually meant to add a point in my first comment about the text.
On the login page says "Already have an account?", so I think something like can be done for the sign up link as well, but needs discussion as there's obviously translation work on the back of it,
Some things I had thought of were:

  • Need an account?
  • Don't have an account?
  • Create a new account
  • Register an account
@cw1998

This comment has been minimized.

Copy link
Contributor Author

commented Apr 19, 2019

@ssddanbrown Had a play around with the sign up link, and I think it looks good as a button. What do you think? Although it might confuse people if they fill out the form and click sign up, thinking it will create an account...
An option to consider anyhow.

image

Changes are on the following branch, so no changes to this PR.
https://github.com/cw1998/BookStack/tree/feature/sign-up-button

@ssddanbrown ssddanbrown added this to the v0.26.0 milestone Apr 20, 2019

@ssddanbrown

This comment has been minimized.

Copy link
Member

commented Apr 21, 2019

I've had a play about and landed on this:

image

Looks better when you don't have so many social auth options 😆.

"Sign up" as a button looked okay but I think your point about creating confusion is very much true. I think it sits well at the bottom, after all options that relate to logging-in, as a link so it's obvious it's not a form control/action but instead a different location.

Thanks again for your help on this one!

@ssddanbrown ssddanbrown merged commit c8cf673 into BookStackApp:master Apr 21, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@cw1998

This comment has been minimized.

Copy link
Contributor Author

commented Apr 21, 2019

Looks great! 👍🏻👍🏻

@cw1998 cw1998 deleted the cw1998:fix/registraion-form-validation branch Apr 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.