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

FIX: Navbar margin on mobile #541

Merged
merged 1 commit into from Jan 10, 2022
Merged

FIX: Navbar margin on mobile #541

merged 1 commit into from Jan 10, 2022

Conversation

choldgraf
Copy link
Collaborator

@choldgraf choldgraf commented Jan 7, 2022

This applies the same fix as #526 but in a more generic fashion. I realized that the problem was not unique to having a text-based site title. On mobile this is how it would look:

(note the mis-alignment with the content block below for the logo, and how the dropdown button is right up against the screen edge on the right)

chrome_WpBTFeVW53

With this fix, we add margin to the navbar-start and navbar-end div elements, so that no matter what is inside them, they'll have nicer padding. Here's how it looks on mobile now:

chrome_OZpxIkvEm3

Copy link
Collaborator

@damianavila damianavila left a comment

Choose a reason for hiding this comment

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

This change looks good to me on mobile but I see a small linear and horizontal movement in the logo when I go below 960 px width (from a wider view). Probably not a blocker... just something I saw at the time to test this one.

@choldgraf
Copy link
Collaborator Author

I think that it's OK to have the padding slightly more on wide screens - but happy to discuss that in a follow up issue. Thanks @damianavila for the review!

@choldgraf choldgraf merged commit 6950bcd into pydata:master Jan 10, 2022
@choldgraf choldgraf deleted the padding branch January 10, 2022 21:32
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.

None yet

2 participants