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

Lettre: Fix styling issues for top nav menu #7167

Merged
merged 1 commit into from
Jun 29, 2023

Conversation

edanzer
Copy link
Contributor

@edanzer edanzer commented Jun 28, 2023

Changes proposed in this Pull Request:

  • Reduces the font size of items in the nave menu. Among other things this fixes an issue with text for medium-to-longer page titles wrapping on small screens.
  • Ensure wrapping text doesn't align left.
  • Remove underline from :focus items. This was producing a situation where the first item in the menu was always underlined, even when it was not current.
  • Add underline to current item. The current item in the menu will now be underlined.

Before
lettre nav before

After
lettre-nav-after

Related issue(s):

#7071
#7072

Testing instructions

  1. Checkout this branch to your local WordPress instance.
  2. Ensure you have multiple items on your menu, including one page with a long page title.
  3. Confirm:
    • First item isn't always underlined by default
    • Current page is underlined
    • Font size is smaller
    • If longer item does wrap, it is still aligned right.

@edanzer edanzer requested a review from a team June 28, 2023 16:57
@edanzer edanzer self-assigned this Jun 28, 2023
@lezama lezama merged commit f6588b8 into trunk Jun 29, 2023
@lezama lezama deleted the fix/lettre-navigation-styles branch June 29, 2023 17:57
.wp-block-pages-list__item:focus.wp-block-navigation a:where(:not(.wp-element-button)):focus {
text-decoration: none;
}
Copy link
Member

Choose a reason for hiding this comment

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

Removing focus-underline so that wrong link isn't highlighted as "current" element could impact browsing the navigation with keyboard? Also weird how first element was focused when the navigation opens?

cc @scruffian hi! Is this something that should be improved in Gutenberg? (See PR description for screenshots)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants