-
Notifications
You must be signed in to change notification settings - Fork 54
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
Alerts: Change warning icon #2240
Conversation
✅ Deploy Preview for boosted ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Haven't checked the code yet but something indirectly linked: https://deploy-preview-2240--boosted.netlify.app/docs/5.3/extend/icons/#sprite is using the previous icon, maybe we need to change it to use the new one OR find another icon example. It could mean removing |
This comment was marked as resolved.
This comment was marked as resolved.
ea0707e
to
e800175
Compare
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.
Tested with combinations of old browsers/OSs and everything seems to be rendered correctly.
TBH I'm not a huge fan of the "warning": ("icon": escape-svg($warning-icon)),
syntax but it's lighter compared to a clean one where for each $alert-icons
we would have to define this icon
value and then another one to true
for instance. So let's go with it.
IMO https://deploy-preview-2240--boosted.netlify.app/docs/5.3/utilities/colors/#oranges-colors and https://deploy-preview-2240--boosted.netlify.app/docs/5.3/extend/icons/ can work temporarily if this code would happen to be released without #2075.
Just two minor comments to tackle and we're good to go.
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
Note: Please transform
- [ ]
into- (NA)
in the description when things are not applicableRelated issues
Close one item of #1761.
Description
Change the
$warning-icon
that is only used for.alert-warning
Motivation & Context
Be conform to UI kit v5.
Types of change
Live previews
Checklist
Contribution
Accessibility
Design
Development
Documentation
Checklist (for Core Team only)
After the merge