Block inserter: Improve alignment of block inserter search and close icons

[juanfra]

What?

Improve alignment of block inserter search and close icons.

Fixes: #49756

Why?

When using the block inserter search and clearing the search it's pretty much noticeable that the icons are not aligned and it creates a strange experience.

Testing Instructions

1. Open a post or page.
2. Open the block inserter, and check the search icon position.
3. Type in your search, and confirm that the search and close icons are aligned.

Screenshots or screencast

Screen.Recording.2023-05-08.at.18.14.18.mov

[jasmussen]

Seems good to me, thanks for the PR, happy to approve. Just a quick ping by @ciampo in case there's a better way.

[ciampo]

Thank you @juanfra for working on this, and thank you for the ping, @jasmussen !

The currently proposed solution has a few issues:

• the bug that we're trying to solve affects the SearchControl component, and not just this specific instance in the block inserter. We should therefore apply a solution to the component directly if possible.
• we should not be using .components-* class names for style overrides, especially outside the @wordpress/components package
• in general, we should avoid style overrides when possible (like the padding rule here), since they make the UI much more prone to breakage any time there's a change upstream
• changing the min-width also affect the focus style of the close icon button:

trunk	this PR

A better solution could be to:

• apply a fix directly to SearchControl
• centering the search button via flex layout, regardless of the button's width and padding (thus avoiding style overrides on the button component)

Here's how those code changes would look like

diff --git a/packages/block-editor/src/components/inserter/style.scss b/packages/block-editor/src/components/inserter/style.scss
index 5579e17bc9..d6cf148724 100644
--- a/packages/block-editor/src/components/inserter/style.scss
+++ b/packages/block-editor/src/components/inserter/style.scss
@@ -109,11 +109,6 @@ $block-inserter-tabs-height: 44px;
 
 	.components-search-control__icon {
 		right: $grid-unit-10 + ($grid-unit-60 - $icon-size) * 0.5;
-
-		.components-button.has-icon {
-			min-width: initial;
-			padding: 0;
-		}
 	}
 }
 
diff --git a/packages/components/src/search-control/style.scss b/packages/components/src/search-control/style.scss
index c02250de4f..5acc4fcc24 100644
--- a/packages/components/src/search-control/style.scss
+++ b/packages/components/src/search-control/style.scss
@@ -43,8 +43,10 @@
 	top: 0;
 	right: ( $grid-unit-60 - $icon-size ) * 0.5;
 	bottom: 0;
+	width: $icon-size;
 	display: flex;
 	align-items: center;
+	justify-content: center;
 
 	> svg {
 		margin: $grid-unit-10 0;

[juanfra]

Thanks @ciampo for the thoughtful review. I'm new to the codebase, this is really helpful. I've adjusted the PR to address the changes with the proposed approach.

[ciampo]

Thanks @ciampo for the thoughtful review. I'm new to the codebase, this is really helpful. I've adjusted the PR to address the changes with the proposed approach.

Thank you, @juanfra ! It's my pleasure to help — feel free to ping me again next time you need a review, especially if related to the editor's UI and @wordpress/components specifically.

Before merging, could you also add an entry to the CHANGELOG, under the "Bug Fix" sub-section?

Otherwise, code changes LGTM 🚀

[ciampo]

Thank you! Will merge once all checks have passed.

@chad1008 you may want to take a look into the CI failure for the CHANGELOG check — the task is having some issues fetching trunk, it may have to do with the fact that this PR originates from a fork of the repo?

[juanfra]

Grazie a te, Marco! I appreciate it 😄

[ciampo]

Es un placer!

[chad1008]

you may want to take a look into the CI failure for the CHANGELOG check — the task is having some issues fetching trunk, it may have to do with the fact that this PR originates from a fork of the repo?

Thanks for the ping. I'll take a look! Forked repos should be fine, as we fixed that a while back, but I'll look into what's happening here. Maybe there's something more that needs to be added to cover all forks.

[chad1008]

🤔 I'm not sure why the CI check wasn't able to properly fetch trunk here. I've tested from a fork of my own with no issues. I also looked through a handful of other Components-related PRs from forked repos, and they similarly appear to be running without issue. It looks like this was either an odd edge case that we'll need more examples of try and find a pattern, or perhaps something unique to this fork/PR.

@juanfra, please do let me know if you see any more fatal: couldn't find remote ref trunk failures on this check. Happy to dig in more if it becomes a pattern!