Skip to content

Add Login/Logout functionality to mobile layout #1543

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

Merged
merged 11 commits into from
Aug 22, 2020

Conversation

ghalestrilo
Copy link
Collaborator

Requires #1528

This PR adds an option to the <MobileIDEView /> Dropdown menu, allowing the user to log in and out successfully. The <Dropdown /> component has also been changed.

  • has no linting errors (npm run lint)
  • is from a uniquely-named feature branch and has been rebased on top of the latest master. (If I was asked to make more changes, I have made sure to rebase onto master then too)
  • is descriptively named and links to an issue number, i.e. Fixes #123

@ghalestrilo ghalestrilo self-assigned this Aug 12, 2020
@catarak
Copy link
Member

catarak commented Aug 12, 2020

Yay this is so exciting! Two things I noticed:

  • Screen Shot 2020-08-12 at 3 11 40 PM 1

The dropdown menu items are not centered instead of left-aligned.

  • Screen Shot 2020-08-12 at 3 12 06 PM

This nav should probably match the nav styling of what's on mobile! Thought it also makes me wonder... there's no p5.js logo on the mobile design and maybe it should be somewhere?

@ghalestrilo
Copy link
Collaborator Author

I aligned the dropdown items! As discussed, I think it makes more sense to place the p5 logo on a new branch

@catarak
Copy link
Member

catarak commented Aug 19, 2020

Screen Shot 2020-08-19 at 4 52 56 PM

Possibly makes sense to put this in another PR (I'm going to approve this and let you make that decision), but other thoughts: - When you click "back to editor" it takes you to the desktop view - When the translation is enabled the Nav gets all smooshed

catarak
catarak previously approved these changes Aug 19, 2020
@ghalestrilo
Copy link
Collaborator Author

Ok! I think the p5 logo makes sense to go in another branch, and about the redirect: #1564 fixes it by making the mobile view render on the index endpoint, when appropriate. I'll work on the CSS to fix the header and merge this

@ghalestrilo
Copy link
Collaborator Author

ghalestrilo commented Aug 21, 2020

Updated the header on both <SignupView /> and <LoginView /> on small resolutions. Some notes here:

  1. A workaround was necessary to implement width detection. This will be fixed in Feature/switch mobile desktop #1564 , which implements more robust responsiveness.
  2. Sign Up and Login In options were hidden from the header (on mobile) because they're already available on-screen
  3. Some margins and paddings were reduced to fit content better on small resolutions.

Screenshot from 2020-08-21 15-08-54

I'll merge this PR because others depend on it, but I we should test this on different resolutions before deploying, and open issues if changes are required.

Edit: Needs a new review before merging ✨ @catarak please review it again when you can 😬

@ghalestrilo ghalestrilo requested a review from catarak August 21, 2020 18:23
@ghalestrilo
Copy link
Collaborator Author

Just made a final update: right-aligning the language dropdown so it's visible on mobile

@catarak
Copy link
Member

catarak commented Aug 21, 2020

Looks great to me 🌈

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