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

Dropdown: Pass 'options' objects as props to Dropdown.Item #418

Merged

Conversation

jeffcarbs
Copy link
Member

@jeffcarbs jeffcarbs commented Aug 22, 2016

When specifying options on a dropdown, previously only 'text' and 'value' were being passed to <Dropdown.Item>. This makes it so the entire object is passed as props.

cc #416

@codecov-io
Copy link

codecov-io commented Aug 22, 2016

Current coverage is 95.15% (diff: 100%)

Merging #418 into master will not change coverage

@@             master       #418   diff @@
==========================================
  Files            91         91          
  Lines          1177       1177          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           1120       1120          
  Misses           57         57          
  Partials          0          0          

Powered by Codecov. Last update 1620db3...46cea61

value: PropTypes.oneOfType([
PropTypes.string,
PropTypes.number,
]).isRequired,
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: This isRequired got dropped in this change.

On the one hand, PropTypes.arrayOf(PropTypes.shape(DropdownItem.propTypes)) is much DRYer and less brittle.

On the other, there's a slight difference in terms of what props are required that we're now ignoring.

Open to suggestions here.

Copy link
Member

Choose a reason for hiding this comment

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

Seems reasonable for now. The text/value props can't reliably be required at the DropdownItem level as there is no guarantee they are being used in a selection Dropdown.

Previously only 'text' and 'value' were being passed to <Dropdown.Item> for each
object in `options`. This makes it so the entire objects are passed as props.
@jeffcarbs jeffcarbs force-pushed the feature/dropdown-options-props branch from 260d069 to 46cea61 Compare August 22, 2016 05:21
@jeffcarbs
Copy link
Member Author

Updated based on comments 👍

@levithomason levithomason merged commit 2afe318 into Semantic-Org:master Aug 22, 2016
@levithomason
Copy link
Member

Released in v0.36.3, thanks!

@jeffcarbs jeffcarbs deleted the feature/dropdown-options-props branch August 22, 2016 05:56
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.

3 participants