-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Search Block: Add border radius #27664
Conversation
A CSS based alternative to this approach is available #27991 If it is acceptable, I think this PR can be closed. |
I think #27991 is a solid approach that I've tested and worked, and we can close this if others agree on that PR. |
Hey, I've taken a look at this and these are my notes for my future-self and anyone joining the thread. The goal here is to add support for the borderRadius property in the search block. Under normal circumstances, this would be as easy as adding support for it via block.json. However, the approach in this PR does the following:
If we want to make the search block work with the current supports mechanism as it works today, this is a breakdown of what we'd need to do:
I've also seen the alternative using CSS Custom Properties at #27991. So far, we've avoided using them in the block markup & CSS, and I'd think we want to stay that way (see rationale at #22296). Alternatively, this block could have a custom border control and let themes tweak it via CSS. cc @youknowriad in case he wants to add thoughts to this. |
@nosolosw I don't think I'm following what you are suggesting is the best path forward here. Could you expand on your comment further? In case it helps, the goals of this PR include:
Anything that ticks off that list is fine by me 🙂
Are CSS variables off limits only for block supports and global styles? In other words, could a custom border radius control and attribute for the search block still leverage a CSS variable? It's a pretty clean approach in #27991 not involving heavy changes to the block's code.
Initially this began with (#25553) an ad-hoc border radius control and attribute added to the Search block used the same way as this PR uses the border radius support value. |
👋 It looks like this block could certainly benefit from using ad-hoc UI controls. I'm not sure how to replicate that user experienc with the existing block supports mechanism. I've created #28518 so other people can chime in and offer a perspective on how blocks with ad-hoc controls work with patterns and global styles ― so this can be unblocked. |
The next iteration of the block supports mechanism #28913 is beginning. This PR would benefit greatly from the ability to target which inner elements get the block support style applied to them. As such, I plan to revisit this PR whenever that progresses. |
Closing in favour of #30227 which utilises the skip serialization feature. It was made as a separate PR to allow for easier comparison with this PR. |
Description
This PR contains changes to the search block for it to opt in to the new border radius support added in #25791.
The updates also handle adjusting the border radius when the search button is placed "inside" the search container. This allows the inner border radius to visually match the outer container without appearing to have "fat corners" as illustrated here.
How has this been tested?
Manually.
Testing Instructions.
Screenshots
Types of changes
New feature
Checklist: