Fix colour rendering in Navigation overlay#36479
Merged
tellthemachines merged 4 commits intotrunkfrom Nov 17, 2021
Merged
Conversation
|
Size Change: +77 B (0%) Total Size: 1.09 MB
ℹ️ View Unchanged
|
Contributor
|
Thanks for taking a stab at this! Up front I see the overlay color working now:
I can also confirm that when the burger menu is set to off, in the editor the submenu color is correctly applied: There's still that issue in the frontend with the submenu block not grabbing the color, but I think that's separate:
There's a problem when the menu is set to "Mobile", as now the color is applied also to the navigation block:
Any thoughts on whether we can get that to work so only the submenu and overlays are colored? Major progress in a fast time. Thank you! |
Contributor
Author
|
Thanks for the thorough testing!
I had to use the |
jasmussen
approved these changes
Nov 16, 2021
Contributor
jasmussen
left a comment
There was a problem hiding this comment.
Nice one, thanks so much. If you'll forgive my terrible colors, this is working as it should for me:
| // If the responsive wrapper is present but overlay is not open, | ||
| // overlay styles shouldn't apply. | ||
| &:not(.is-menu-open.is-menu-open) { | ||
| color: inherit !important; |
Contributor
There was a problem hiding this comment.
The !important's feel appropriate enough for the way the feature works at the moment. It's possible in the future we could explore editing the contents of the modal in isolation (like template parts), at which point these would be revisited anyway.
noisysocks
pushed a commit
that referenced
this pull request
Nov 22, 2021
* Fix colour rendering in Navigation overlay * Add overlay styles in the editor * Overlay styles shouldn't apply when overlay is closed. * Fix php lint errors.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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
Fixes #36429.
Setting Submenu & overlay text and background wasn't having any effect on the Navigation overlay because the overlay colours aren't added to the Navigation block outer wrapper (unlike regular text and background colours). This PR adds the relevant overlay classnames and/or inline styles to the responsive wrapper so the overlay can render in the correct colours.
How has this been tested?
Screenshots
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist:
*.native.jsfiles for terms that need renaming or removal).