-
Notifications
You must be signed in to change notification settings - Fork 40
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
New Login Page Design & Unifying Login/CreateUser #269
Conversation
adds alt tag, comments out unsued import, removes unused prop/social code
+ fixes issue where 5th image is never called
Emmy the MVP 💯 All the changes are b e a u t i f u l as is, so I'm eager to roll this out when the remainder of the todo list is check off. I'll keep an eye on this going forward! |
makes Login toggle between CreateUserForm and LoginForm based on prop; removes CreateUser
added to button + highlight
Ok with that, we've hit everything on the to-do list other than the bugfix on large screens - @emmyc can you take a look at that when you have a chance. Stretch goals should be adding some sort of testing, refactoring some of this code into util/helper functions, and making this much more mobile-friendly (though it does work as intended on mobile, just could be spicier). @krashanoff you can probably look to review/merge whenever. |
@krashanoff just pushed a commit that should fix the svg not filling for the screen being too large, lmk what you think! I will say, the actual image itself looks slightly strange, but I think it's nbd. As per super short screens & mobile, we probably do need to do something about it eventually - but also not sure as to what the plan is. |
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.
We will probably have to go ahead and vertically center the image when working with tall viewports later on, but even in its current state things are looking an echelon above where things were at with our previous login page. I'd say this looks good to merge to me. Thanks a ton @malsf21 and @emmyc!
@emmyc killing the game 💯 (more docs coming soon)
to-do:
add a bit more information to create useralso, should we consider randomizing the SVG? just a thought 😮
and... tests?
and eventually, we should have a static feature page. probably out of scope for this PR!