add announcement banner #751
Conversation
Signed-off-by: Abdelsalem <[email protected]>
Signed-off-by: Abdelsalem <[email protected]>
Signed-off-by: Abdelsalem <[email protected]>
AbdelHedhili
force-pushed
the
add_announcement_banner
branch
from
f6b4b8c to
24286de
Compare
Signed-off-by: Abdelsalem <[email protected]>
Signed-off-by: Abdelsalem <[email protected]>
| </Box> | ||
| <Box sx={mergeSx(styles.banner, isInfo ? styles.infoBg : styles.warningBg)}> | ||
| {isInfo && ( | ||
| <Box sx={mergeSx(styles.icon, styles.infoIcon)}> |
There was a problem hiding this comment.
we don't use mergesx for static styles
There was a problem hiding this comment.
why ? I don't get the problem here, I made it this way so I don't duplicate the common style between the infoIcon and warningIcon. Did I miss something ?
| </Box> | ||
| )} | ||
| {!isInfo && ( | ||
| <Box sx={mergeSx(styles.icon, styles.warningIcon)}> |
There was a problem hiding this comment.
we don't use mergesx for static styles
There was a problem hiding this comment.
Why not using Alert component instead ? It have all we need.
There was a problem hiding this comment.
Because it's supposed to be the same thing as the banner we already use so it makes more sense to just reuse it and not try to copy it using another component
TheMaskedTurtle
force-pushed
the
add_announcement_banner
branch
from
46eaf43 to
202c72e
Compare
This reverts commit a28344e.
|
No description provided.