-
-
Notifications
You must be signed in to change notification settings - Fork 79.1k
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: fix StackBlitz icon link examples #39799
base: main
Are you sure you want to change the base?
Conversation
632d3e0
to
0db1983
Compare
This beats the point of using the SVG sprite, though. And makes the situation even worse for maintenance, see #39809. We should streamline our icons usage and generally not try to fix StackBlitz if we are going to duplicate stuff and make maintenance harder. |
Yes, considering the overall architecture of the documentation: the SVG sprite is better. However, it's probably less understandable to the end-user point of view for these examples in terms of autonomy and reusability (copy/pasting the code).
I get your point about maintenance. For these use cases, maybe we don't necessary need these icons to be exactly the same as the final version of the ones in Bootstrap icons. Or maybe in Hugo we can make some of them reusable with partials (or anything else).
There's something to consider regarding the usage of the documentation by the users; the ease to understand, integrate into their projects and to create reproducible use cases when they create issues. I'm not saying I'm against your points, just that everything needs to be taken into account, not only docs' maintenance. Since I don't have a strong opinion on the topic and I just would like these StackBlitz examples to be fixed, I let you decide with @mdo and @louismaximepiton which version would be better between this one and #39505, or any other strategies. Note that if this one is closed, there's probably something to update in our Alerts documentation, because it's basically what's already done there. |
It's one thing trying to make docs better and another make it harder for maintainers to maintain the docs. The icons at this point are a mess. We have numerous icons everywhere, with no easy way to see what's used and what's not. So, I'd rather have a few broken icons at the moment or just drop the StackBlitz examples from those than make the situation worse right now. We need to figure out a simpler way to handle the icons before scattering more icons across the docs. Using the font in the examples is one easy way for us regardless the downside of people having to load the font. If someone wants to improve loading times later they could use the icons they only need to but that's covered in our icons documentation. |
Description
This PR takes into account the issue reported in #39499 (the issue has been closed by OP but is still there in our documentation).
This PR supersedes #39505 based on @louismaximepiton and @mdo suggestions (#39505 (comment) and #39505 (comment))
Contrary to what's been done in #39505, this PR modifies the Icon Link page to embed the needed SVGs instead of using their references used elsewhere and not accessible from StackBlitz.
@louismaximepiton suggested using the second example in Alerts Icons. It can be challenged obviously, but I've rather used the first example which seemed closer to what I would do in a real project by default (of course it also depends on the global usage of these SVGs on the website, if they're used at different places, etc.).
The only drawback that I can see would when we'd want to change the "box-seam", "arrow-right" and "clipboard" globally in the documentation, it might be difficult to spot these usages here.
Type of changes
Checklist
npm run lint
)Live previews
Related issues
Closes #39499
Closes #41134