-
Notifications
You must be signed in to change notification settings - Fork 376
Breaking/alert #4190
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
Breaking/alert #4190
Conversation
| import avatarImg from './examples/avatarImg.svg'; | ||
|
|
||
| <Avatar src={avatarImg} alt="avatar"></Avatar> | ||
| class AvatarExample extends React.Component { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made this change because i noticed the codesandbox wasn't working before. should probably go through and update all the examples that use the previous syntax @redallen ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All you need is to name the example like this @jschuler :
AvatarExample = () => <Avatar src={avatarImg} alt="avatar"></Avatar>There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
codesandbox would work (after adding const in front), but our docs would not
SyntaxError: Unexpected token (1:8)
1 : return (const AvatarExample = () => <Avatar src={avatarImg} alt="avatar"></Avatar>)
|
PF4 preview: https://patternfly-react-pr-4190.surge.sh |
dlabrecq
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests failed for actionClose, but otherwise LGTM
mcoker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
What: Closes #4082
Alerts can now contain both the close button and one or more action links.
Breaking changes
actionprop for the close button, use theactionCloseprop insteadactionprop for a link button, use theactionLinksprop instead