VUE-223 KvIcon: dynamically inline most icons, sprite common icons#1555
Conversation
ryan-ludwig
requested review from
BoulderBrains,
eddieferrer,
emuvente and
mcstover
| "webpack-hot-middleware": "^2.25.0", | ||
| "webpack-node-externals": "^1.7.2", | ||
| "webpack-svgstore-plugin": "^4.1.0" | ||
| "webpack-svgstore-plugin": "ryan-ludwig/webpack-svgstore-plugin#develop" |
There was a problem hiding this comment.
This is unfortunate, but the only way I could get the sprite plugin to work with our magnifying glass icon was to fork it and update it's SVGO dependency. I have a PR up with them.
I believe @BoulderBrains has had problems adding SVGs to the sprite in the past, I believe this is the same issue.
There was a problem hiding this comment.
Can you link to that PR?
There was a problem hiding this comment.
There was a problem hiding this comment.
I think webpack-svgstore-plugin is practically abandoned. We should try to replace it with something else
There was a problem hiding this comment.
Yeah, I've wondered if we shouldn't just do the sprite manually since it has so few items now.
There was a problem hiding this comment.
I have my doubts that webpack-svgstore-plugin is going to merge my PR. Don't really love that this is pointing to my personal github for something we're wanting to get into prod.
2 options I think:
- Fork webpack-svgstore-plugin to the kiva github, make the same change, point package.json to that.
- Ditch webpack-svgstore-plugin, roll the sprite by hand (easy), and ajax it in from TheHeader.vue or something similar.
Both have upsides and downsides. @emuvente @mcstover thoughts?
|
Lets also please make sure to squash these commits when we merge this in. Thats a lot of commits... |
Good call, I'll rebase after reviews. |
Run all icons through SVGO to tidy things up Move thanks page social share icons into kv-icon Update print and question mark icons on thanks page Move credit card icons for BraintreeCheckout Move lock to an inline-icon Allow KvIcon2 to pull form the sprite or inline depending on prop. Move close x to sprited icon Move notice icon over to kvicon2 Move all of the lending stats icons out of the sprite Tweak how icons fill their space Create a logo folder, move Kiva logo into it. Move dedicate-heart icon out of the sprite Remove unused checkbox icons Move KvTipMessage icons out of sprite Update Stats Section completed icon Add a max width to icons Move confirmation checkbox icon over to kv-icon2 Move DonationItem icons to kv-icon2 Pull the x icon from sprite in kv-tip-message Use close and magnify glass from sprite in header KvFacebookButton use kv-icon2 More triangle icon conversions Magnify glass from sprite Triangles from sprite in kv-pagination Convert viewtoggle icons to inline Use star from sprite Fix height Rename confirmation icon to checkmark-in-circle Update checkoutnow button, remove checkmark-in-circle icon. Couple of small-x icon updates Convert order totals icon. Change from a tags to button tags for semantics. Update loan card close icons Update specific promo banners (although they could likely be deleted) Move over chevrons Reorganize icon folders. Remove unused icons. Convert promo banner icons Update genericpromobanner icons to match contentful. Rename a few icons. Rename KvIcon2 to KvIcon Color dedicate-heart using scss Add max-height Fix icon location Change tags to buttons for accessibility
ryan-ludwig
force-pushed
the
VUE-223_kv-icon-slimsprite
branch
from
4e372b7 to
298d1e0
Compare
|
@ryan-ludwig @emuvente Please hold on merging this until next week. |
5 participants
Follow up from #1541
This PR moves the majority of icon files out of the global icon sprite and into a folder where we dynamically import them only in the components which use them. This dramatically reduces the DOM nodes on all pages from ~465 to ~25 from the sprite.
To test:
pull down,
run npm install,
browse to some pages you've worked on, make sure icons look correct.
npm run storybook to see the available icons