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

Components: Set a default for the ComboboxControl onFilterValueChange #28492

Merged
merged 3 commits into from
Feb 10, 2021

Conversation

pablinos
Copy link
Member

@pablinos pablinos commented Jan 26, 2021

Description

While using the the ComboboxControl I noticed that there were errors stating that onFilterValueChange was undefined. Looking at the example in Storybook, it seems that this was probably required in the past, and was a way for the logic of how the suggestions are filtered, to be handled outside of the combobox.

This can safely be set to a no-op function and the control works as expected. This PR sets that as the default and updates the Storybook example so that it shows the simplified usage.

How has this been tested?

This has been manually tested, in the block I'm developing and using Storybook. The frontend functionality remains unchanged.

Types of changes

This should probably be categorised as a bug fix. The onFilterValueChange prop can be optional, but would cause an error if omitted.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • 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.

@gziolo gziolo added the [Package] Components /packages/components label Feb 4, 2021
@gziolo gziolo requested a review from ItsJonQ February 4, 2021 11:08
@gziolo gziolo added the [Type] Bug An existing feature does not function as intended label Feb 4, 2021
@gziolo gziolo requested a review from sarayourfriend February 4, 2021 11:09
@sarayourfriend sarayourfriend changed the title Components: Set a default for the ComboboxControl onFieldValueChange Components: Set a default for the ComboboxControl onFilterValueChange Feb 4, 2021
Copy link
Contributor

@sarayourfriend sarayourfriend left a comment

Choose a reason for hiding this comment

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

LGTM, just one change request and it'll be all set 😁

packages/components/src/combobox-control/index.js Outdated Show resolved Hide resolved
Copy link
Contributor

@sarayourfriend sarayourfriend left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

@retrofox
Copy link
Contributor

@sarayourfriend could you take a look at this super small one, please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants