-
-
Notifications
You must be signed in to change notification settings - Fork 64
Updated signup form #49
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
Conversation
|
Looks great! Thank you!
Hmm yeah I think translations is important, so I would prioritize it being translatable. |
|
I updated the element button inside allauth so every button have some style. |
| {% csrf_token %} | ||
| {% element fields form=form unlabeled=True %} | ||
| {% endelement %} | ||
| {% bootstrap_form form show_help=False layout="horizontal" %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used bootstrap_form to style the signup form, but this does not reflect other forms. Is it better to make some updates to allauth element fields as I did for the button instead?
I know the intention to switch to Tailwind instead of Bootstrap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thank you! What we have right now is great. Since the work to move to tailwind hasn't started yet, I think it's ok if you want to adjust the other forms to. Either in this same PR or as a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect. If the solution meets expectations, I’d close this PR with the updated signup form.
|
@raffaellasuardini checking if you're still working on this. |
|
Hi @Mariatta I think this could be a finished job, is it ok with you? Do you want me to fix the other forms too? |
|
Could you resolve conflicts please? |
|
To solve the conflict I will create a new remote to fetch and when I'm in my branch issue_9 can i merge it? is that correct @Mariatta ? |
|
Yes, pull the main branch into your own branch. |
✅ Deploy Preview for pyladiescon-portal-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Mariatta
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! I went ahead to resolve the conflict.
|
Thank you @Mariatta and sorry for the delay |
Updated the signup form using django-bootstrap
I also updated the submit button, but now it's not as translatable as before. Is it better to use plain Bootstrap?