Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign uprefactor(demos): improve icons usage with svg symbols #3354
Conversation
benouat
force-pushed the
benouat:feat/demos/icon-svg-refs
branch
from
8611a13
to
d2b43ad
Codecov Report
@@ Coverage Diff @@
## master #3354 +/- ##
=======================================
Coverage 90.88% 90.88%
=======================================
Files 95 95
Lines 2743 2743
Branches 510 510
=======================================
Hits 2493 2493
Misses 190 190
Partials 60 60
Continue to review full report at Codecov.
|
benouat
force-pushed the
benouat:feat/demos/icon-svg-refs
branch
from
d2b43ad
to
81077d3
|
Thanks for the PR @benouat ! LGTM. I have however one remark about SVG and symbol (with no impact for this PR though, but it can feed some future improvements): I would have preferred implement something with an external link instead of having them inline in the page. It would have taken the advantage of the browser cache, and the svg containing the symbols would have been in the public directory instead of being in the icons component (this would have been more generic by working with the svg you want, no need to embed everything in icons.component.html). But unfortunately, it comes with a drawback: it doesn't work with IE, so a workaround is necessary, rendering the solution more complex. So for now, your PR is fine for me, as it improves the SVG usage in the demo ! |
|
|
Inspired by #3315 I refactored a bit the way we display icons in demos skeleton structure by introducing the usage of svg symbols. (in the demo website, not in the demos themselves)
Unfortunately we just can't use that in the demos themselves, as it would simply not work in Stackblitz.