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.

Sign up for GitHub

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

Jump to bottom

Mobile layout #134

Merged
merged 7 commits into from
Nov 26, 2020
Merged

Mobile layout #134

merged 7 commits into from
Nov 26, 2020

Conversation

neokore
Copy link
Contributor

@neokore neokore commented Nov 26, 2020

@vercel
Copy link

vercel bot commented Nov 26, 2020 •
edited
Loading

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/carto-frontend/cra-template-carto/b6evx416x
✅ Preview: https://cra-template-carto-git-feature-ch121312mobile-support.carto-frontend.vercel.app

@VictorVelarde
Copy link
Contributor

VictorVelarde commented Nov 26, 2020 •
edited
Loading

The space for the carto username is a bit short
image, also it appears as a link, with no pointer, that I'm not sure about.

Maybe we could just move the username into the dialog itself, more like in
image, but simplified

@VictorVelarde
Copy link
Contributor

Mobile looks really nice, great work!

VictorVelarde
VictorVelarde reviewed Nov 26, 2020
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
VictorVelarde
VictorVelarde reviewed Nov 26, 2020
View reviewed changes
template-skeleton/template/src/components/views/Main.js
</Grid>
<Grid container direction='row' alignItems='stretch' item xs>
<nav className={classes.drawer}>
<Portal container={isMobile ? mobileContainer.current : desktopContainer.current}>
Copy link
Contributor

@VictorVelarde VictorVelarde Nov 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting use of Portal here in combination with Outlet... 👍

@neokore
Copy link
Contributor Author

neokore commented Nov 26, 2020

The space for the carto username is a bit short
image, also it appears as a link, with no pointer, that I'm not sure about.

Maybe we could just move the username into the dialog itself, more like in
image, but simplified

Yesterday I undertood that the design team want to create a better design, so by now let's keep this text from wrapping and then let's apply their new design when ready.

@neokore @VictorVelarde
Update CHANGELOG.md
6c4b0f1 
Co-authored-by: Víctor Velarde <victor.velarde@gmail.com>
VictorVelarde
VictorVelarde approved these changes Nov 26, 2020
View reviewed changes
Copy link
Contributor

@VictorVelarde VictorVelarde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to merge after login username fix

@neokore neokore merged commit 32c9091 into master Nov 26, 2020
@neokore neokore deleted the feature/ch121312/mobile-support branch November 26, 2020 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VictorVelarde VictorVelarde VictorVelarde approved these changes

@alasarr alasarr Awaiting requested review from alasarr

@Josmorsot Josmorsot Awaiting requested review from Josmorsot

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@neokore @VictorVelarde