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

Navigation: Small fixes #36298

Merged
merged 2 commits into from
Nov 10, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/block-library/src/navigation-link/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
min-width: 200px !important;
height: auto !important;
width: auto !important;
overflow: visible !important;
}
}
}
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/navigation-submenu/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ button.wp-block-navigation-item__content {
color: currentColor;
font-size: inherit;
font-family: inherit;
line-height: inherit;

// Buttons default to center alignment. This becomes visible
// when a menu item label is long enough to wrap.
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/navigation/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
.wp-block-navigation__container.is-parent-of-selected-block {
visibility: visible;
opacity: 1;
overflow: visible;
}

// Low specificity default to ensure background color applies to submenus.
Expand Down
37 changes: 22 additions & 15 deletions packages/block-library/src/navigation/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -119,17 +119,14 @@
// Don't take up space when the menu is collapsed.
width: 0;
height: 0;
overflow: hidden; // Overflow is necessary to set, otherwise submenu items will take up space.

// Submenu items.
> .wp-block-navigation-item {
> .wp-block-navigation-item__content {
display: flex;
flex-grow: 1;

// Without this, the changing to zero on hover-out can cause wrapping and
// result in an invinite hover/hover-out loop.
white-space: nowrap;

// Right-align the chevron in submenus.
.wp-block-navigation__submenu-icon {
margin-right: 0;
Expand Down Expand Up @@ -178,6 +175,7 @@
// Show submenus on hover unless they open on click.
&:where(:not(.open-on-click)):hover > .wp-block-navigation__submenu-container {
visibility: visible;
overflow: visible;
opacity: 1;
width: auto;
height: auto;
Expand All @@ -187,6 +185,7 @@
// Keep submenus open when focus is within.
&:where(:not(.open-on-click):not(.open-on-hover-click)):focus-within > .wp-block-navigation__submenu-container {
visibility: visible;
overflow: visible;
opacity: 1;
width: auto;
height: auto;
Expand All @@ -196,6 +195,7 @@
// Show submenus on click.
.wp-block-navigation-submenu__toggle[aria-expanded="true"] + .wp-block-navigation__submenu-container {
visibility: visible;
overflow: visible;
opacity: 1;
width: auto;
height: auto;
Expand Down Expand Up @@ -290,18 +290,25 @@
}

// Default background and font color.
.wp-block-navigation:not(.has-background) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't reproduce the issue with has-background not being applied on the front end. I'm seeing whether the background color is custom, theme defined or a default core one. Which theme are you using?

Regardless, the change is working well on the front end. But in the editor, I'm seeing this weird border at the bottom of each dropdown when nested 2 or more deep:

Screen Shot 2021-11-10 at 4 20 35 pm

I think it's happening because the block appender doesn't get background-color set inline, like the navigation items do, so the box-shadow becomes visible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for testing. It does look like those borders need a little more work, I'll dive in when I get my dev environment working again.

In the mean time, to clarify: has-background is applied correctly when the Background color is set. But it is not applied when only the Submenu & overlay background color is set. I don't even think it should be, at least not on the main navigation block, possibly on submenus.

.wp-block-navigation__submenu-container {
// Set a background color for submenus so that they're not transparent.
// NOTE TO DEVS - if refactoring this code, please double-check that
// submenus have a default background color, this feature has regressed
// several times, so care needs to be taken.
background-color: #fff;
color: #000;
border: 1px solid rgba(0, 0, 0, 0.15);
}
.wp-block-navigation .wp-block-navigation__submenu-container {
// Set a background color for submenus so that they're not transparent.
// NOTE TO DEVS - if refactoring this code, please double-check that
// submenus have a default background color, this feature has regressed
// several times, so care needs to be taken.
background-color: #fff;
color: #000;

// Apply a border using an inset box-shadow instead of a border.
// That way, if an explicit background color is applied, it will be hidden
// behind the background color, so you don't end up with border and background.
box-shadow: inset 0 0 0 1px rgba(0, 0, 0, 0.15);

// Box-shadows are removed in Windows High Contrast mode, but a transparent
// outline will show up as an opaque border.
outline: 1px solid transparent;
}


// Navigation block inner container.
.wp-block-navigation__container {
display: flex;
Expand Down Expand Up @@ -396,7 +403,7 @@
border: none;
padding-left: 32px;
padding-right: 32px;

box-shadow: none;
}

// Space unfolded items using gap and padding for submenus.
Expand Down