Skip to content
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

Feat: Update textControl to searchControl in taxonomy search. #64605

Merged
merged 2 commits into from
Aug 21, 2024

Conversation

vipul0425
Copy link
Contributor

What?

Fixes: #64593

Why?

The "Search categories" field in the Post sidebar uses a normal TextControl even though it's a search field.

How?

This PR updates the TextControl component to SearchControl in sidebar taxonomy search.

Testing Instructions

  • Create any post.
  • View the post settings in Sidebar and in categories it should be SearchControl now instead of normal TextControl.

@vipul0425 vipul0425 marked this pull request as ready for review August 19, 2024 09:25
Copy link

github-actions bot commented Aug 19, 2024

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: vipul0425 <[email protected]>
Co-authored-by: t-hamano <[email protected]>
Co-authored-by: mirka <[email protected]>
Co-authored-by: jasmussen <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

I think everything about the code is correct, let's get some design feedback.

Before After
image image

There are visual changes to the field, but I think this is acceptable.

If changes are necessary, I think it would be better to improve it at the component level, as discussed in #56388.

@t-hamano t-hamano added Needs Design Feedback Needs general design feedback. [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Editor /packages/editor labels Aug 20, 2024
@t-hamano t-hamano requested a review from a team August 20, 2024 03:44
@t-hamano t-hamano added the [Type] Enhancement A suggestion for improvement. label Aug 20, 2024
@jasmussen
Copy link
Contributor

I'd tend to agree with Aki 👍 👍

Copy link
Member

@mirka mirka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me too 👍 The label is hidden by default on SearchControl, but it's still accessibly associated to the input. And with the placeholder and search icon, it's a lot visually clearer that this is a search input, even without the visual label. So I think we're good.

@t-hamano
Copy link
Contributor

Thanks everyone for your feedback. Let's merge this PR now.

@vipul0425 Thanks for the great work!

@t-hamano t-hamano merged commit 2719bc8 into WordPress:trunk Aug 21, 2024
69 of 73 checks passed
@github-actions github-actions bot added this to the Gutenberg 19.1 milestone Aug 21, 2024
bph pushed a commit to bph/gutenberg that referenced this pull request Aug 31, 2024
…rdPress#64605)

Co-authored-by: vipul0425 <[email protected]>
Co-authored-by: t-hamano <[email protected]>
Co-authored-by: mirka <[email protected]>
Co-authored-by: jasmussen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Feedback Needs general design feedback. [Package] Editor /packages/editor [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should sidebar "Search categories" filter be a SearchControl?
4 participants