-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[docs-infra] Add icon to callouts #38525
Conversation
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.
Very happy with this one! Thanks for kicking it off @alexfauquette 🙏
['info', 'success', 'warning', 'error'].includes(token.severity) | ||
? [ | ||
'<svg focusable="false" aria-hidden="true" viewBox="0 0 24 24" data-testid="ContentCopyRoundedIcon">', | ||
`<use class="MuiCode-copied-icon" xlink:href="#${token.severity}-icon" />`, |
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.
Now on any page, these SVG get loaded. This can't be tree-shaked. e.g. view-source:https://master--material-ui.netlify.app/blog/mui-x-mid-v6-features/.
I wonder what's best. Per https://cloudfour.com/thinks/svg-icon-stress-test/, we could use inline SVG, it doesn't see much different than the symbol sprite for performance, so maybe better.
There are a few duplicated icons, e.g. ![]() #38689 created. |
Closes #38519
This PR adds icons to the documentation callouts! We also took advantage of the opportunity to clean instances of it that were using emojis instead of the built-in icons. There might be more spread out throughout the docs ⎯ it was just a quick check.
https://deploy-preview-38525--material-ui.netlify.app/experiments/docs/callouts/
To-dos:
<symbols/>
ids are already correct)