Block highlight: Fix radius issue. #49864
Merged
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
|
Sorry for the novella of a PR, but I feel it's important to document these details for posterity if this gets refactored in the future! |
|
Size Change: +132 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
richtabor
approved these changes
Apr 17, 2023
|
Cool, thank you, let's try this! Let's also be mindful of any focus/highlight/multi-select issues in the coming releases, in case there are any side effects here. |
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.
What?
Alternative to #49766. Fixes an issue where an improper radius was applied to social logos and more.
Blocks can be styled to have rounded corners, whether through a block style variation, or through the Border Radius tool.
We also use border radius in block UI, such as the block highlight (hover the drag handle), selection styles (select several blocks), or block focus (select an image block).
For the block UI, we've generally tried to avoid using pseudo selectors (::before/::after) as much as possible, to allow the theme itself to leverage those selectors. An older refactor removed one such selector in favor of painting the highlight style on the block itself. Unfortunately that meant the border radius overriding the radius set by the block. As an example:
Shown here:
This PR fixes that.
How?
This PR restores the pseudo selector (we still only "reserve" the ::after, so ::before is still unstyled and ready for themes to use). This ensures all those cases above work, as shown in this GIF:
I did have a different instinct to try as well, just reducing the specificity of the radius (as I do in this PR for the multi select style), but this had unintended side effects:
Shown in this GIF is a rounded site logo that is simultaneously multi selected, and highlighted. It shows duplicate highlight/focus states. This is in part due to how the site logo is built — the radius is applied to the block wrapper itself, and then inherited all the way down to the img inside, meaning that the radius would be applied to a random child div inside. That's perhaps not ideal behavior for the site logo, but nevertheless demonstrated to me why after all, it seems, we need the pseudo element approach.
Testing Instructions
Please test content that features a diversity of blocks, but most specifically include social logos (with background colors under the icons), together with the Site Logo, and ideally also any other blocks that support border radius (such as Image). With those in mind, please test:
In all cases, the UI (focus style and blue overlay) should have 2px radius, and in no cases should the "natural" radius of each block be affected.