Navigation: Small fixes #36298
Merged
Navigation: Small fixes #36298
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jasmussen
added
[Type] Bug
7 tasks
|
Size Change: +102 B (0%) Total Size: 1.09 MB
ℹ️ View Unchanged
|
tellthemachines
force-pushed
the
polish/navigation-submenu-colors
branch
from
ff2dc0a
to
5a3d46c
Compare
There was a problem hiding this comment.
Thanks for these improvements! The overflow fix is working well with the keyboard in both click and hover modes. The box-shadow is acting a bit funny for me though - see comment below.
(I also rebased this PR to make sure it worked with #36292 which I just merged)
| @@ -290,18 +290,25 @@ | |||
| } | |||
|
|
|||
| // Default background and font color. | |||
| .wp-block-navigation:not(.has-background) { | |||
There was a problem hiding this comment.
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:
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.
There was a problem hiding this comment.
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.
|
I have reverted the border to box shadow change, as it wasn't working as intended. The issue is also minor and shouldn't cause these improvements to not land. |
jasmussen
added
the
Backport to WP Beta/RC
jasmussen
changed the title
andrewserong
pushed a commit
that referenced
this pull request
Nov 10, 2021
* Navigation: Extract small fixes, and refine border rules. * Revert the box shadow change.
|
Cherry picked into the Gutenberg 11.9 release in: 1b0b445 |
noisysocks
removed
the
Backport to WP Beta/RC
noisysocks
pushed a commit
that referenced
this pull request
Nov 11, 2021
* Navigation: Extract small fixes, and refine border rules. * Revert the box shadow change.
jasmussen
added
Backport to WP Beta/RC
|
Uh, sorry about the readding the backport label, I thought i had forgotten to add it in the first place. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR extracts a few fixes from #36292 (the text alignment rule) and from #36215 (the overflow fixes), to make for a small mostly visual bugfix oriented PR that should be easily to rebase the other two on top of.
When submenus have customized background colors, the border around them should not show. This didn't work as intended, because on the frontend, the
has-backgroundclass is not present for submenu items:The bug with the Submenu block not having its color set is separate.
This PR fixes it so that the
has-backgroundisn't needed at all, which simplifies things and also reduces specificity a bit:It does so by moving to an inset box-shadow which just gets covered by the explicitly set color.
It also ports over this small line-height fix. Click button sizes before:
After:
Finally it ports over the fix where submenus could create a horizontal scrollbar even when closed, because their contents would take up space.
How has this been tested?
Test the navigation block with nested submenus, test with click and hover options, test with enough submenus that when opened, they cause a horizontal scrollbar, and then verify that they don't when the menus are closed.
Checklist:
*.native.jsfiles for terms that need renaming or removal).