Search Block: Add border color support

[ramonjd]

Description

This PR add border color support to the search block pursuant to #22071

We're skipping serialization since the targets for border classnames and custom styles change according to the search block style, that is, whether the button appear inside the border or not.

How has this been tested?

Manually for now.

To test, enable customColor in the (experimental-)theme.json:

		"border": {
			"customColor": true,
			"customRadius": false,
			"customStyle": false,
			"customWidth": false
		},

Please check that the editor and frontend match for:

1. Button inside with border swatch color and custom color
2. Button outside with border swatch color and custom color
3. Icon as button
4. Enable border.customRadius in the (experimental-)theme.json and ensure there are no regressions.

Screenshots

Button inside

Button outside

Button icon

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• I've tested my changes with keyboard and screen readers.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.
• I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

[ramonjd]

@stacimc I fixed the border regression in b35d1c1ae3c942bf28155b9c8d1bb9e09296ec2e
Thank you for spotting that!

[aaronrobertshaw]

Thanks for putting this one together @ramonjd 👍

This PR tests as advertised for me.

It might be good though to get a couple of sets of eyes on it regarding forming a consensus on exactly what should get the border color under what conditions.

I don't pretend to be a designer however it looks a bit odd to me to be applying the border color to the button when positioned inside but not the input. I'd be inclined to omit the button border color in that situation. It would also simplify a lot of the changes required to add border color to the block.

Also, when checking this out I see a number of PHP linting errors for = alignment in index.php.

That aside, this is looking good ✨

[ramonjd]

I don't pretend to be a designer however it looks a bit odd to me to be applying the border color to the button when positioned inside but not the input. I'd be inclined to omit the button border color in that situation. It would also simplify a lot of the changes required to add border color to the block.

Thanks @aaronrobertshaw. That's a fair point. I wasn't sure of it myself. I think my logic was that the border radius was being applied to the button as well, so I thought it might be odd to have the color as an exception.

Happy to go in either direction if we get some extra feedback.

Also, when checking this out I see a number of PHP linting errors for = alignment in index.php.

That's just me trying to annoy the linter as much as it annoys me. 😆 Fixed

[aaronrobertshaw]

That's a fair point. I wasn't sure of it myself. I think my logic was that the border radius was being applied to the button as well, so I thought it might be odd to have the color as an exception.

We could go the other way and apply the border color to the input as well. Same degree of simplification in the code.

My personal preference would be to leave it only on the wrapper when the button is inside. It gets pretty busy looking otherwise.

[stacimc]

This tests well for me as well!

My personal preference would be to leave it only on the wrapper when the button is inside. It gets pretty busy looking otherwise.

+1 now that you point it out -- I'm thinking this would be exacerbated if border width or style options are added, too. 🤔

[ramonjd]

@aaronrobertshaw @stacimc Thanks for the feedback!

I've adjusted things so that the border color is applied to the input and button elements only when the button is outside.

When the button is inside, only the wrapper gets the color.

[ramonjd]

I think this one is ready for some more 👀

@jasmussen or @aaronrobertshaw Do you see any issues with the controls in light of the UI updates? Should we hold off?

This introduces the border color control only.

[aaronrobertshaw]

Thanks for all your work on this @ramonjd 👍

While this does only introduce the border color control, there is also the existing border radius control.

Work being done on improving the border support UI ( #31585 ) though it needs a little more time before it lands. It might also be desirable that the border support UI be updated with a new progressive disclosure approach similar to #32392.

I'll defer to @jasmussen regarding what's acceptable in terms of UI and this landing. On the code front, I'll give it a proper review later today.

[jasmussen]

Thanks for the ping. This is what I see:

These are some nice features, excited for them to land. Aaron also makes some good points on using the progressive discovery panel, adopting Jay's style picker and unifying on a "Border" panel. Here's a quick doodle that includes some of Jay's work, but for the Search block:

While that mockup is a bit forward looking (it's also a work in progress, and CC @javierarce as I know he's been working on Search block improvements as well) — there are a few details and principles I think are important:

• The fewer panels in the inspector, the better. If the color is related to the border, put it in the border panel. I'm even thinking we could have an "Appearance" panel that includes both border and other properties in the future — do we ever add box shadow and blur features?
• No reason to include the word "Settings", the sidebar is already settings so it's implied.
• Title case in all panels. It's "Status & visibility", not "Status & Visibility". It's an older decision that can be revisited holistically if important enough to change, but we should keep it consistent.

Landing #31585 first would also be of huge value. While the progressive discovery panel would also benefit things, one of the aspects of the Search block currently is that there aren't that many properties to configure, so it's okay to just show all of them by default. Progressive discovery will enable that to scale better, of course, but I don't think it a blocker here.

So what do we need to do?

• Make sure this PR is aware of the work in Block Support: Update border support UI #31585, perhaps land that first. That would also let this employ the new radius picker.
• Change "Border Settings" to "Border", and remove the word "border" from the other labels inside.
• Keep the panel open by default.

It's unrelated to this PR, but the "Display Settings" panel is just a "Dimensions" panel.

[aaronrobertshaw]

Great feedback as always @jasmussen, thank you 🙇

No reason to include the word "Settings", the sidebar is already settings so it's implied.

I appreciate the "Display Settings" panel wasn't added in this PR but there's no harm in removing the word settings which also then removes one title case inconsistency. The "settings" in the border panel comes from the current border block support UI which is cleaned up as part of #31585.

Landing #31585 first would also be of huge value.

#31585 is getting much closer to landing as the PR it depends on should get merged some time in the next week, or few days, if we're lucky.

While the progressive discovery panel would also benefit things

The new progressive discovery panel is close however will need a little additional work once it lands to update the border block support to use it.

Make sure this PR is aware of the work in #31585, perhaps land that first. That would also let this employ the new radius picker.

Both the new progressive discovery panel and the refinements in #31585 will be automatically picked up by the search block once they land. No further tweaks should be required.

Change "Border Settings" to "Border", and remove the word "border" from the other labels inside.

This is included in #31585.

Keep the panel open by default.

At present, the default open/closed state of the border support panel is a simple boolean value. It is either open by default for all blocks or none. I don't think we want it open by default for all blocks for the time being.

When the progressive discovery panel lands we'll get the ability to display different controls by default for different blocks.

[jasmussen]

At present, the default open/closed state of the border support panel is a simple boolean value. It is either open by default for all blocks or none. I don't think we want it open by default for all blocks for the time being.

I mean this "Border settings" one:

to be like this:

Or did you mean that if that panel is set to be open by default, it's open by default for any block that includes the border support? Like the group?

In that case, yes, let's keep it collapsed until it's progressive and we can choose some good defaults. Perhaps with a new color picker as well.

[aaronrobertshaw]

Or did you mean that if that panel is set to be open by default, it's open by default for any block that includes the border support? Like the group?

This one. If we open that panel by default, all blocks that opt into border support, such as the group block, will also have the panel open by default.

That panel comes straight from the border block support. No UI has been directly "added" as part of this PR. This is also why as soon as #31585 that refines that panel lands, this search block immediately benefits from it. Likewise, once the border block support gets the progressive discovery panel, it will show immediately for the search block.

In that case, yes, let's keep it collapsed until it's progressive and we can choose some good defaults.

Great. That's one less tweak required to this PR. It now appears everything else on the earlier "todo list" is covered by #31585.

As the search block already has the border support panel displaying the border radius control, could this PR land now? Or can we hold off until #31585 merges?

[aaronrobertshaw]

Thanks again @ramonjd for getting this PR together.

The code side is looking pretty good ✨

I have a couple of minor thoughts that might help ourselves in the future.

Could we make what's happening in the edit.js a little clearer by replacing the repeated ternary statements with named variables instead?

Also, there could be an opportunity to simply reuse the borderProps.style rather than rebuild the { borderRadius, borderColor } style object a few times. It would also then pick up other border support styles such as width and border style when/if they get adopted.

I don't have strong feelings so I'll leave that one with you.

[jasmussen]

As the search block already has the border support panel displaying the border radius control, could this PR land now? Or can we hold off until #31585 merges?

Ideally we can address the items spread across a few todos before the next plugin release, and if that's the case, it definitely seems fine to merge with the pending PRs landing after.

[ramonjd]

Thanks a lot for the feedback and context surrounding block controls UI progress @aaronrobertshaw and @jasmussen, Very helpful!

Ideally we can address the items spread across a few todos before the next plugin release, and if that's the case, it definitely seems fine to merge with the pending PRs landing after.

🚀

Could we make what's happening in the edit.js a little clearer by replacing the repeated ternary statements with named variables instead?

Also, there could be an opportunity to simply reuse the borderProps.style rather than rebuild the { borderRadius, borderColor } style object a few times. It would also then pick up other border support styles such as width and border style when/if they get adopted.

Thanks for the nudge. I'll refactor in line with your suggestions 👍

[ramonjd]

Rebased on trunk to include changes in Search: Update search block to handle per corner border radii #33023

[ramonjd]

Rebased to include #33376

@aaronrobertshaw I think this is good to go. What do you think?

[aaronrobertshaw]

This is working as advertised for me 👍

✅ Code looks good
✅ Search block tests well in the editor, applying styles and classes as appropriate
✅ Search block displays correctly on the frontend
✅ No changes to previous styles or markup requiring deprecations
✅ No errors when loading deprecated versions of the search block

I think we're good to merge this. Nice work @ramonjd!