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

Local navigation block: Hide page title before collapsing menu #641

Merged
merged 3 commits into from
Jul 22, 2024

Conversation

ryelle
Copy link
Contributor

@ryelle ryelle commented Jul 18, 2024

Fixes #640, see See WordPress/Learn#2622. When elements (site title, page title, navigation) are too long, they can overlap or cause wrapping issues. In #573, the navigation menu collapsed when they would wrap, but in some cases this can cause the navigation to be hidden on more screen-sizes than is ideal. Instead, the navigation menu should be prioritized, and the page title can be hidden first.

This PR updates the logic, so that the page title is hidden first, extending the screen sizes that will see the correct desktop version of the nav bar. If the menu will still collide with the site title, it does collapse down into the chevron dropdown, but this is only at some screen sizes, since most site titles are short.

Screenshots

Screen recording of shrinking the screen from 1440px, with an exaggerated page title and menu length. The page title disappears around 1300px, and while the menu stays inline until around 970px (where it would otherwise run into the site title). Previously, the menu would collapse at that 1300 width. (Note that these breakpoints are relative to the page content, not universal)

local-nav-collapse.mp4
Before After
1310px Screen Shot 2024-07-18 at 17 14 13 same
1300px Screen Shot 2024-07-18 at 17 19 44 Screen Shot 2024-07-18 at 17 14 17

On a real page, the Learn menu is now visible here, where previously it collapsed on a larger screen size.

Screen Shot 2024-07-18 at 17 42 50

To test:

  • View a site with a "L2B" page header — has the site title and a page title.
  • Scroll down so you can see the page title (easier to test)
  • Shrink your screen down, the page title should disappear before the menu collapses into a chevron
  • Nothing should wrap to a second line
  • The mobile menu kicks in at 600px, and there should be no change there

Copy link
Collaborator

@fcoveram fcoveram left a comment

Choose a reason for hiding this comment

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

From the screenshots, it looks great to me 🚀

Copy link
Contributor

@adamwoodnz adamwoodnz left a comment

Choose a reason for hiding this comment

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

LGTM

I did find one odd point between 550px and 599px where the site title is covered by the mobile button, but this appears to be existing, and I guess only happens with this really long title. It is fairly graceful.

Screenshot 2024-07-22 at 9 30 43 PM

@ryelle
Copy link
Contributor Author

ryelle commented Jul 22, 2024

I did find one odd point between 550px and 599px where the site title is covered by the mobile button, but this appears to be existing, and I guess only happens with this really long title. It is fairly graceful.

Yeah, I avoided changing anything under 600px, but it is a little strange that the title appears for just that 49px width breakpoint. I've bumped it up to 599 so the page title stays hidden.

@ryelle ryelle merged commit 85cde2d into trunk Jul 22, 2024
2 checks passed
@ryelle ryelle deleted the update/local-nav-block-responsive branch July 22, 2024 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Local Navigation: Prevent menu from collapsing when page title is long
3 participants