Color palette settings button hover tootlip fix.#14693
Color palette settings button hover tootlip fix.#14693gziolo merged 1 commit intoWordPress:masterfrom v9ai:14685/color-settings-hover-fix
Conversation
|
@NicolaD yes, it fixes it. Thanks! I don't have the powers to approve PRs though. |
Everyone can approve :) |
gziolo
left a comment
There was a problem hiding this comment.
It looks like a valid fix. There is one remaining code maintainability related question to be answered before merging.
| box-shadow: inset 0 0 0 4px; | ||
| border: $border-width solid $dark-gray-400; | ||
| position: relative; | ||
| z-index: 1; |
There was a problem hiding this comment.
@jasmussen - should there be special handling for z-index used?
There was a problem hiding this comment.
My impression is that's always good. But I'm honestly not confident, sometimes a z index of 1 is just to elevate a child in a context that isn't subject to the mess it can otherwise create. I know @aduth has thoughts.
There was a problem hiding this comment.
My impression has been to always use _z-index function for any z-index, with the mapping serving as a centralized authority to serve in proactively avoiding aggressively and endlessly increasing a z-index value to "combat" others.
https://github.com/WordPress/gutenberg/blob/master/assets/stylesheets/_z-index.scss
But:
- For a case like this, where the value is
1, perhaps it's reasonable to make exceptions - In any case, we don't codify this guideline one way or the other. Whatever we decide, it ought to be written down, e.g.
coding-guidelines.mddocument
|
Note: if using |
I guess |
|
Yep, z-index is fine. And if this PR is semi urgent or can make the cut by being merged now, feel free to and if Andrew objects to the lack of the z-index helper later on, I volunteer to fix that after the fact. |
|
@NicolaD - thanks for the fix. Let's wait for feedback and take action only if necessary. |
Sure, I can add a follow-up PR based on feedback, if it will be the case. |
Description
Fixes #14685
Adding
z-indexfixes the issue.How has this been tested?
Has been tested locally, more details in gif from below
Screenshots
Types of changes
Had added few styles to color palette settings button in active state.
Checklist: