Update: Allow color gradient popover to be above the color toggle #37430
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
jorgefilipecosta
added
[Type] Enhancement
|
Size Change: +82 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
|
This is cool, it solved the issue for me for the first level of popover but I still see the same issue for the second level (custom color picker) |
jorgefilipecosta
force-pushed
the
update/allow-color-gradient-popover-to-be-above-the-color-buttons
branch
from
c7db6e2 to
01b6ced
Compare
A fix was applied for the second level popovers 👍 |
|
This is very cool. There's only one left issue with the custom color picker, if the size of the window is very small, its height is adapted but the scrollbars are not shown causing a weird issue.
|
jorgefilipecosta
force-pushed
the
update/allow-color-gradient-popover-to-be-above-the-color-buttons
branch
from
01b6ced to
c34aea9
Compare
I made a CSS update to include the scroll. |
|
Sometimes I get vertical scrollbars too But this PR is in a very good state I think. I'd love some testing from folks here @jasmussen maybe |
youknowriad
approved these changes
Dec 17, 2021
|
Generally nice work, and it's probably good to land as an improvement. Longer term, I'd love to see some of the CSS reduced and simplified to rely less on media queries and positioning, but I also recognize that's a larger endeavour. Just to say, I look forward to the day we can get to that!
There are a number of smaller issues when testing this thoroughly. Because those issues are almost certainly present in trunk already, they probably shouldn't block this PR.
|
jorgefilipecosta
force-pushed
the
update/allow-color-gradient-popover-to-be-above-the-color-buttons
branch
from
c34aea9 to
800816a
Compare
|
The solution for the scroll caused other issues and seemed to do more harm than good. I removed the scroll fix but added a CSS rule that keeps the popover rendered as expected although if the screen is very small some parts may be hidden (in practice I think it will not be replicated unless one intentionally make the window very very small). Thank you for the reviews @youknowriad and @jasmussen. |
jorgefilipecosta
deleted the
update/allow-color-gradient-popover-to-be-above-the-color-buttons
branch
7 tasks
|
Cherry-picked for 5.9.1 |
Mamaduka
removed
the
Backport to WP Minor Release
Mamaduka
pushed a commit
that referenced
this pull request
Feb 16, 2022
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.
This PR allows the color popover to be above the color buttons if there is no space below. Disables the position forcing and allows us to take advantage of the popover automatic position mechanism.
How has this been tested?
I made the window height equal to 700px.
I added a group block.
I opened the background color picker and verified the popover appears above and is completely visible as the screenshot shows.
Screenshots