feat(Alert): add size props; add css-api; refactor component #2146
Conversation
|
Preview is ready. |
|
Visual Tests Report is ready. |
benax-se
force-pushed
the
feat/alert-sizes-and-css-api
branch
6 times, most recently
from
f08a52c to
353afc1
Compare
| color: var(--g-alert-title-color, var(--g-color-text-primary)); | ||
| font-size: var(--g-alert-title-font-size, var(--_--title-font-size)); | ||
| font-weight: var(--g-alert-title-font-weight, var(--_--title-font-weight)); | ||
| line-height: var(--g-alert-title-line-height, var(--_--title-line-height)); |
There was a problem hiding this comment.
There is no need of font weight CSS vars, user can do the following:
<Alert title={<MyBoldTitle/>} />Let's keep only font-size and line-height
src/components/Alert/Alert.scss
Outdated
| } | ||
|
|
||
| &__title { | ||
| color: var(--g-alert-title-color, var(--g-color-text-primary)); |
There was a problem hiding this comment.
| color: var(--g-alert-title-color, var(--g-color-text-primary)); | |
| color: var(--g-alert-title-text-color, var(--g-color-text-primary)); |
src/components/Alert/Alert.scss
Outdated
| } | ||
|
|
||
| &__message { | ||
| color: var(--g-alert-message-color, var(--g-color-text-complementary)); |
There was a problem hiding this comment.
| color: var(--g-alert-message-color, var(--g-color-text-complementary)); | |
| color: var(--g-alert-message-text-color, var(--g-color-text-complementary)); |
src/components/Alert/Alert.scss
Outdated
| @@ -17,5 +134,6 @@ $block: '.#{variables.$ns}alert'; | |||
|
|
|||
| &__close-btn { | |||
| flex-shrink: 0; | |||
| margin-inline-start: var(--g-alert-close-btn-indent, var(--_--close-btn-indent)); | |||
There was a problem hiding this comment.
What's the purpose of manipulating both margin and relative position? There are already usage of --_--close-btn-offset above.
There was a problem hiding this comment.
after discussion with the designer we decided to remove additional offset
src/components/Alert/Alert.scss
Outdated
| &__main { | ||
| flex-grow: 1; | ||
| display: flex; | ||
| gap: var(--g-alert-content-gap, var(--_--content-gap)); |
There was a problem hiding this comment.
We should specify in CSS API only the parts we publically declare. The content name is the internal structure. This should be variables like: g-alert-message-margin, g-alert-actions-margin and so on
src/components/Alert/Alert.tsx
Outdated
| layout = 'vertical', | ||
| closeBtnSize = 'm', |
There was a problem hiding this comment.
| closeBtnSize = 'm', | |
| closeButttonSize = 'm', |
| } | ||
|
|
||
| &__icon { | ||
| vertical-align: text-bottom; |
There was a problem hiding this comment.
Icon is now not aligned properly with the title. Icon should be a bit under the text baseline
benax-se
force-pushed
the
feat/alert-sizes-and-css-api
branch
from
353afc1 to
2710b80
Compare
benax-se
force-pushed
the
feat/alert-sizes-and-css-api
branch
from
06c4405 to
3854a73
Compare
No description provided.