Skip to content

Conversation

@sharvit
Copy link
Contributor

@sharvit sharvit commented Dec 17, 2017

Export DropdownButton and SplitButton from react-bootstrap

re #11

What:

@@ -0,0 +1 @@
export { DropdownButton as default } from 'react-bootstrap';
Copy link
Member

Choose a reason for hiding this comment

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

does it make sense to add snapshots? this will ensure markup doesnt change on a newer upgrade of react-bootstrap?

@sharvit sharvit force-pushed the feature/dropdown-buttons branch from c29e36b to d84cc9a Compare December 17, 2017 13:20
@sharvit
Copy link
Contributor Author

sharvit commented Dec 17, 2017

@ohadlevy - I change it to use the constants.js and added some tests


const SplitButton = props => <BsSplitButton {...props} />;

SplitButton.propTypes = Object.assign(BsSplitButton.propTypes, {
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as above


DropdownButton.propTypes = Object.assign(BsDropdownButton.propTypes, {
bsStyle: PropTypes.oneOf(BUTTON_BS_STYLES),
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

This modifies also the BsDropdownButton.propTypes. To make it correct you either need to use

DropdownButton.propTypes = Object.assign({}, BsDropdownButton.propTypes, {
  bsStyle: PropTypes.oneOf(BUTTON_BS_STYLES),
});

or much simpler:

DropdownButton.propTypes = {
  ...BsDropdownButton.propTypes,
  bsStyle: PropTypes.oneOf(BUTTON_BS_STYLES),
});

Copy link
Member

Choose a reason for hiding this comment

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

@jtomasek @sharvit works for me. thanks.

@sharvit sharvit force-pushed the feature/dropdown-buttons branch from d84cc9a to bcc6e12 Compare December 18, 2017 13:41
@sharvit
Copy link
Contributor Author

sharvit commented Dec 18, 2017

Nice catch @jtomasek!
I have copied this code from Button which is suffer from the same problem together with Modal.

Update all of them in the last commit.

jtomasek
jtomasek previously approved these changes Dec 18, 2017
@jgiardino
Copy link
Contributor

Thanks for adding these! They look good to me. :-)

@priley86
Copy link
Member

Can you rebase this PR first @sharvit ?

Export DropdownButton and SplitButton from react-bootstrap

re #11
@sharvit sharvit dismissed stale reviews from jeff-phillips-18 and jtomasek via 9896cdb December 19, 2017 09:12
@sharvit sharvit force-pushed the feature/dropdown-buttons branch from bcc6e12 to 9896cdb Compare December 19, 2017 09:12
@priley86 priley86 merged commit e4cdf7d into patternfly:master Dec 19, 2017
@sharvit sharvit deleted the feature/dropdown-buttons branch December 20, 2017 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants