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

fix(TableToolbarSearch): allow default values to be set #5089

Merged

Conversation

tw15egan
Copy link
Member

Closes #3170

The DefaultValue prop was being ignored. This checks for the presence of a DefaultValue, and if it exists, it is used as the default state of value. On the initial load, it sets the expanded state to true and runs the onChangeProp to filter the table. Modified the handleOnInputValueChange in DataTable.js to account for non-event based sorting (table loads with a default value).

Screen Shot 2020-01-17 at 2 10 40 PM

Changelog

New

  • DefaultValue prop works as intended

Changed

  • handleOnInputValueChange now has a defaultValue parameter

Removed

  • DefaultValue being passed directly down to Search. If DefaultValue exists, it is immediately set to value which is then passed down.

Testing / Reviewing

  • Open src/components/DataTable/stories/with-toolbar.js
  • Add defaultValue="1" to TableToolbarSearch
  • Open the With Toolbar Storybook example, verify only 1 result shows up.
  • Remove the value, verify all results show up.
  • Add new value and verify the correct results show up.

@tw15egan tw15egan requested a review from a team as a code owner January 17, 2020 22:13
@ghost ghost requested review from abbeyhrt and joshblack January 17, 2020 22:13
@tw15egan tw15egan changed the title Table toolbar search value fix fix(TableToolbarSearch): allow default values to be set Jan 17, 2020
@netlify
Copy link

netlify bot commented Jan 17, 2020

Deploy preview for the-carbon-components ready!

Built with commit abb08ec

https://deploy-preview-5089--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented Jan 17, 2020

Deploy preview for carbon-components-react failed.

Built with commit abb08ec

https://app.netlify.com/sites/carbon-components-react/deploys/5e2b627e74fb1f0008826468

@netlify
Copy link

netlify bot commented Jan 17, 2020

Deploy preview for carbon-elements ready!

Built with commit abb08ec

https://deploy-preview-5089--carbon-elements.netlify.com

Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

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

Thank you for jumping in @tw15egan!

@tw15egan tw15egan force-pushed the table-toolbar-search-value-fix branch from 1cd2e5d to baca15e Compare January 18, 2020 00:33
Copy link
Contributor

@abbeyhrt abbeyhrt left a comment

Choose a reason for hiding this comment

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

LGTM! 🎉

@tw15egan
Copy link
Member Author

@joshblack updated

@tw15egan
Copy link
Member Author

@joshblack are we good with this?

@tw15egan tw15egan merged commit e97a7ed into carbon-design-system:master Jan 24, 2020
@tw15egan tw15egan deleted the table-toolbar-search-value-fix branch January 24, 2020 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TableToolbarSearch] value overwritten by empty state
4 participants