-
Notifications
You must be signed in to change notification settings - Fork 317
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
AttentionBox with icon on compact mode support & Docs fixes in attention box and typography #463
Conversation
hadasfa
commented
Jan 19, 2022
•
edited
Loading
edited
- AttentionBox with icon on compact mode support
- Fix spacing between attention box header and content
- Fix attention box story ad the assets in typography story
@@ -74,11 +74,7 @@ addParameters({ | |||
"*", | |||
"Accessibility", | |||
"Hooks" | |||
], | |||
includeName: false, |
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.
that's not work and therefore deleted
@@ -13,6 +13,8 @@ const AttentionBox = ({ | |||
className, | |||
// Backward compatibility for props naming | |||
componentClassName, | |||
// Will remove when releasing version 2 as BREAKING CHANGES | |||
withIconWithoutHeader, |
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.
because until now we did not support displaying icon without title.
we still don't support it in regular mode but in compact mode it is looking good and needed. (example in screenshot in the pr)
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.
that is a breaking change and that's why we add the flag
@@ -62,11 +66,9 @@ | |||
display: flex; | |||
align-items: center; | |||
margin-top: 0; | |||
margin-bottom: $spacing-medium; | |||
margin-bottom: var(--spacing-small); |
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.
evigany request
"@storybook/addon-links": "6.5.0-alpha.18", | ||
"@storybook/addon-storyshots": "6.5.0-alpha.18", | ||
"@storybook/addons": "6.5.0-alpha.18", | ||
"@storybook/builder-webpack5": "6.5.0-alpha.18", |
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.
we want to upgrade the storybook version to the latest and check how if it fix few storybook bugs
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.
do we have a list of bugs we'd like to get fixed?
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.
its mainly a bug with the order of the stories - the stories displayed by alphabet instead according to their load order even we did not defined it in the preview