Add neutral option to BccAlert#215
Conversation
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://brave-coast-028fe6203-215.westeurope.3.azurestaticapps.net |
laurensgroeneveld
left a comment
laurensgroeneveld
left a comment
There was a problem hiding this comment.
Looking good, just two small things I noticed.
| class: "bcc-alert-danger", | ||
| icon: ErrorFillIcon, | ||
| }, | ||
| notice: { |
There was a problem hiding this comment.
In the Badge/Pin we've named it neutral. I see here it uses the brand colors, so could brand be an appropriate option?
I'm mostly thinking about this because the other options are "contexts", which is a specific term in the design system related to the (mostly) semantic colors. Perhaps check this naming with the designers with this in mind if it hasn't been discussed already (sorry if this was already hashed out behind the scenes 😅).
There was a problem hiding this comment.
Fixed! Renamed to neutral
| type Props = { | ||
| context?: keyof typeof contexts; | ||
| icon?: boolean; | ||
| icon?: boolean | Component; |
There was a problem hiding this comment.
The type we use for icon in other components like the button is string | Component | Function, so it could be nice to add those other two as well to keep it similar.
There was a problem hiding this comment.
I'm actually not sure if string works on icon haven't tested it.
There was a problem hiding this comment.
Fair, and I think I ran into that when accidentally passing an icon as a string it would not throw an error because string is allowed (but would render <searchicon> as an HTML element), so maybe we should just remove it everywhere.
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://brave-coast-028fe6203-215.westeurope.3.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://brave-coast-028fe6203-215.westeurope.3.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://brave-coast-028fe6203-215.westeurope.3.azurestaticapps.net |
3 participants
Change summary
Also made the
iconprop customizable.Change type