Try using coloured overlay instead of border for grid visualiser #61390
Conversation
tellthemachines
added
[Type] Enhancement
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +73 B (0%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
Hmm. Maybe it makes sense to show a more subdued (borders only) version of the overlay when an item is selected and then the full overlay when the grid is selected? That's my lame idea. Maybe @jasmussen has a better one. |
| border: $border-width dashed $gray-300; | ||
| background-color: color-mix(in srgb, currentColor 10%, transparent); | ||
| outline: 1px solid color-mix(in srgb, currentColor 20%, transparent); | ||
| border-radius: 2px; |
There was a problem hiding this comment.
Probably should use $border-width for the 1px and $radius-block-ui for the 2px.
|
Nice, thanks for trying this! Can we use That would also mean the 2px One aspect that gets a bit more visible when this is applied, is the fact that these cell visualizations are an overlay:
Would it be possible to move this visualization below? I.e. if a cell has a background color, it should ideally cover the cells. |
We're dealing with the iframed editor here so I don't think it's possible without putting styles on the block itself which we try not to do as they can conflict with theme styles. |
Another thing to consider is that if we move it below, it's no longer possible to tell at a glance how many columns/rows a block spans, which I think is nice to have. |
|
Ok, updated to use |
There was a problem hiding this comment.
This is looking nice!
Something I found interesting in testing is that the contrast appears to be greater visually when the current color is a dark, than if it's light. For example, here's how it looks with a wrapping Group block with white background and black text color:
Whereas if using a dark background color and light text color for a section, then the grid visualiser appears to be very subtle or nearly invisible:
I'm not sure how essential it is that we make it more visible for dark background / light text, but just thought I'd mention it in case it's worth tweaking the percentages.
Edit: this is just an observation, not a blocker to landing!
Hmm that's a good point. Should we be looking at the background color of the group and assigning the grid a constrasting color (and if it's transparent, we fall back to a mid-grey sort of thing)? |
I've noticed this as well, but it's important to find a middle-ground here, so one mode of lightness doesn't suffer over the other. Hue-value wise this will also likely shift in perceived contrast where white on blue is likely to have a very high contrast, but shades of green or yellow to have low contrast. Finally, if we can't move the grid visualization under the blocks, it's extra important that this remains decorative, lest any increased opacity will actually reduce the legibility of the content it overlays. |
|
Thanks for the extra context! 👍
Excellent point. Don't let my testing here block anything, erring on the side of decorative to ensure legibility of content sounds like a good way to go. |
|
Thanks for all the feedback and testing folks! I think everything has been addressed, so this should be ready for a final review! |
There was a problem hiding this comment.
Just gave this another test, and I think there might be another detail to this approach that we haven't considered. By applying an overlay over the whole item, it's no longer possible to have a WYSIWYG representation of the background color of a child of the grid when adjusting colors. It's a bit hard to explain, so hopefully a video will make it clearer.
Essentially, because we're displaying a darkening overlay when the grid is selected, then if you go to make changes to either the background color of the Grid or any of its children, then you're not seeing the "real" background color on the editor canvas until you click outside of the grid to deselect it:
2024-05-13.14.11.35.mp4
The main issue here is that if you're going to select a very specific background color, I could imagine the presence of the overlay being a hindrance.
I'll defer to @jasmussen on this, but IMO this feels like a blocker in that it impacts the usability of other features.
One potential option if we think the overlay is the right way to go — could we limit when the overlay is shown, say only while hovering over the grid in the editor canvas? Or would it be better to stick with what we have on trunk?
|
On this fine Monday morning, I had a solution to this branch in my head. Dive in, remove the background, keep just the borders for now, and this'll be good. I got as far as removing the background, which works well in the usual themes I'd test. And then I realized: These outlines should be red. Because the text-color is red, and that the outlines were meant to inherit that red color so that there would always be contrast. Why aren't they? The moment of insight was that the outlines are popovered, so they exist outside of the canvas. Therefore they do not inherit the red color, they inherit the text-color of wp-admin. That also explains why the color kept being so dark on dark backgrounds. Sorry for not realizing this sooner. Are there any easy fixes we can do here? Can the visualization be moved into the canvas? Or can the color of its in-canvas-inherited-color be copied to the popover? Any other options? Borders, backgrounds, radius, dashes, we can always tweak and refine. The main motivation for this PR was to use currentColor to ensure contrast. Red on green, white on black, black on white, orange on blue. The only color that works on all of those is the text-color, because it will always be designed to work against a background. |
noisysocks
force-pushed
the
try/grid-places-opacity
branch
from
4a16d3e
to
3db9172
Compare
I did this 😊 |
|
Thanks @noisysocks, LGTM! |
|
Holy guacamole, this is working great!
|
…dPress#61390) Co-authored-by: tellthemachines <isabel_brison@git.wordpress.org> Co-authored-by: noisysocks <noisysocks@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org>
What?
Addresses some of the feedback from this comment, specifically about improving the look of the grid visualiser.
Testing Instructions
My main question at this point is whether it makes sense to show the overlay across all blocks, or if perhaps the selected block doesn't need to show it? It does help to see the shape of the grid when there are items spanning multiple columns/rows:
Whereas on trunk it can be almost invisible:
Testing Instructions for Keyboard
Screenshots or screencast