-
Notifications
You must be signed in to change notification settings - Fork 56
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
Change icons in Boosted-sprite with real Solaris icons #1929
Conversation
✅ Deploy Preview for boosted ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
# Conflicts: # site/layouts/shortcodes/orange-footer.html
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.
Any need to update migration guide?
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.
Ok for the recent changes but IMO an update of the migration guide is still needed.
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.
As discussed during today's daily, we can go without updating the migration guide so ok.
This comment was marked as resolved.
This comment was marked as resolved.
# Conflicts: # site/static/docs/5.3/assets/img/boosted-sprite.svg
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
There are discrepancies between:
I'm wondering if these success, info, danger, and warning icons are built the same way as the others. Because I've checked all the other ones modified in this PR, and this is the only place I can see such a difference. IDK if it comes from a refresh issue but the rendering of these icons is not incredible (a bit blurry at https://deploy-preview-1929--boosted.netlify.app/docs/5.3/forms/checks-radios/#with-icon): |
Comment after web spec meeting on 22th August:
That description is not quite exact: it remains the same, except inner margins, which will change the rendering size of icons => I update the PR description.
Icon is the same than in library, aliasing is due to browser rendering... PR remains unchanged |
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. There's only the "settings" icon to reintroduce and we're good to merge it.
…nto main-his-solaris-icons-sprite
Note: Please transform
- [ ]
into- (NA)
in the description when things are not applicableRelated issues
PR #2022 changed mobile network coverage icon (which was not the good one) separately.
Description
svgo config:
Motivation & Context
After working on icons, I have seen that they are not formatted exactly the same way (different viewports), which can be confusing. Moreover, they are different from the ones that can be downloaded from DSM. The goal is to make a correct example of sprite file for developers, make it easier and clearer to manipulate.
The look of icons should remain exactly the same, except for inner margins of circled icons: for theses icons, inner margins should be included inside svg and it was not. It will change the rendered size of these icons.
Types of change
Live previews
Checklist
Contribution
Accessibility
Design
Development
Documentation
Checklist (for Core Team only)
After the merge