Skip to content

feat(About): add ability to use product title image#198

Merged
cdcabrera merged 1 commit intopatternfly:masterfrom
jeff-phillips-18:about-modal
Jan 31, 2018
Merged

feat(About): add ability to use product title image#198
cdcabrera merged 1 commit intopatternfly:masterfrom
jeff-phillips-18:about-modal

Conversation

@jeff-phillips-18
Copy link
Member

What:
Adds the ability to pass an image (or other node) for the product title in the about modal.

Link to Storybook:
https://jeff-phillips-18.github.io/patternfly-react/

Additional issues:
Fixes #197

Copy link
Member

@serenamarie125 serenamarie125 left a comment

Choose a reason for hiding this comment

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

@jeff-phillips-18 this looks good. Now there is a choice to either use text or an SVG right?

/** Text to show for the product title */
productTitle: PropTypes.string,
/** Text or Element to show for the product title */
productTitle: PropTypes.oneOfType([PropTypes.string, PropTypes.node]),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it can be as simple as:

productTitle: PropTypes.node,

Since node can be a string.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. Updated.

Copy link
Member

@cdcabrera cdcabrera left a comment

Choose a reason for hiding this comment

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

And of course I didn't catch this until now... Thinking the commit needs to be amended to fix(About) ... or something related to enhancement

@priley86
Copy link
Member

+1... for fix() ... and happy to see it merge after...

@priley86
Copy link
Member

validated in my semantic-release playground that fix() and perf() do a patch level release, and that refactor() and revert() does no release. feat() will do a minor release.

@cdcabrera
Copy link
Member

@priley86 @jeff-phillips-18

For the most part we tend to keep our use of semantic release types limited. Going to go ahead and look like I'm flip-flopping, and merge this in.

There is a bit of subjective to these, and this is one of those in-between updates that's kind of enhancing the existing component and not breaking previous behavior. Where fix seems like an odd choice, since the component isn't actually broken. For future reference, based on the Commitizen types, per @priley86 perf and, of course, fix appear to increment as patches.

@cdcabrera cdcabrera merged commit 9542015 into patternfly:master Jan 31, 2018
@jeff-phillips-18 jeff-phillips-18 deleted the about-modal branch March 13, 2018 11:35
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.

5 participants