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

SearchControl: Deprecate onClose prop #65334

Closed
mirka opened this issue Sep 13, 2024 · 5 comments
Closed

SearchControl: Deprecate onClose prop #65334

mirka opened this issue Sep 13, 2024 · 5 comments
Labels
[Package] Components /packages/components [Type] Code Quality Issues or PRs that relate to code quality

Comments

@mirka
Copy link
Member

mirka commented Sep 13, 2024

What problem does this address?

The onClose prop is not used anymore in the Gutenberg app. It was originally intended to render a close button to "trigger your own logic to close the search field entirely, rather than just clearing the input value."

The prop itself is causing confusion to consumers (#64921), and I think the UI pattern of a single button that doubles as a reset and a close is questionable from a UX standpoint.

What is your proposed solution?

I think we can soft deprecate the feature. For consumers that really want to maintain the original behavior, however unrecommended, we can issue guidance to use the suffix prop.

@mirka mirka added [Type] Code Quality Issues or PRs that relate to code quality [Package] Components /packages/components labels Sep 13, 2024
@ciampo
Copy link
Contributor

ciampo commented Sep 20, 2024

we can issue guidance to use the suffix prop.

Agreed. We could add a dedicated Storybook example too

@Vrishabhsk
Copy link
Contributor

Hi @mirka 👋

  • I have raised a PR addressing this issue : SearchControl: Deprecate onClose prop #65988
  • I was initially going to add context to use suffix instead of onClose but didn't due to it being unrecommended.
  • Let me know If I should extend the docs and deprecation to suggest the use of suffix. Thanks

@mirka
Copy link
Member Author

mirka commented Oct 9, 2024

  • I was initially going to add context to use suffix instead of onClose but didn't due to it being unrecommended.

Yes, I don't think we need to document it officially. At most, I'll maybe add it to a Dev Note.

@Vrishabhsk
Copy link
Contributor

Sounds good. I have addressed the feedback in my latest commit : PR.
Thanks

@mirka
Copy link
Member Author

mirka commented Oct 10, 2024

Closed by #65988

@mirka mirka closed this as completed Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

No branches or pull requests

3 participants