Skip to content
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

add svg-sprite-loader #10

Merged
merged 2 commits into from
Apr 3, 2018

Conversation

formatlos
Copy link
Contributor

I've added a ?sprite resource query to be able to create a SVG sprite instead of including the whole SVG markup for every use of the icon.

@codecov
Copy link

codecov bot commented Mar 29, 2018

Codecov Report

Merging #10 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #10   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines          42     42           
  Branches       13     13           
=====================================
  Hits           42     42
Impacted Files Coverage Δ
index.js 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b3b89c3...63b24e0. Read the comment docs.

@cyrilwanner
Copy link
Owner

Thank you very much for your contributions @formatlos! :)
Thats cool, I didn't know the sprite loader yet, but it is definitely better than the ?inline option when the same icon is used multiple times.

I'm currently on vacation but I'll merge your PRs and publish them to npm as soon as I am back home again on tuesday.

@cyrilwanner cyrilwanner merged commit 64228af into cyrilwanner:master Apr 3, 2018
@formatlos
Copy link
Contributor Author

thanks for the quick merge @cyrilwanner , could you also publish a new version to npm?

@cyrilwanner
Copy link
Owner

Sorry for the delay, as I saw some options of the svg-sprite-loader, I wanted to add some other smaller improvements (in #13) but I didn't have time for it yesterday.
These are basically:

  • Add a notice for SSR to the readme
  • Also optimize svg icons used in sprites in production (removing comments etc)
  • Use a runtime generator for easier usage and so we won't need the verbose <svg> and <use> elements in every usage (viewBox, id, etc is still exposed and your example will also still work):
import React from 'react';
import MyIcon from './icons/my-icon.svg?sprite';

export default () => (
  <div>
    my page..
    <MyIcon />
  </div>
);

Everything is now published as version 1.3.0 at npm 🎉

Thank you again very much for your contribution and for improving this plugin!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants