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

Svg did not rendered as expected #523

Open
sayjeyhi opened this issue Sep 11, 2020 · 9 comments
Open

Svg did not rendered as expected #523

sayjeyhi opened this issue Sep 11, 2020 · 9 comments

Comments

@sayjeyhi
Copy link

sayjeyhi commented Sep 11, 2020

Reporting a bug or issue.

Expected behavior:
I have an SVG file that contains just a path and I want to render it. it renders well in the browser and even in the simulator of react-native.

Observed behavior:
render like it was rendered at snack.

How to reproduce:
just put this SVG in basic SVG example.

Sketch version: 68.2

the svg file:

<Svg viewBox="0 0 24 24" fill="currentColor" focusable="false" width="1em" height="1em">
      <Svg.Path d="M14.33 20h-.21a2 2 0 01-1.76-1.58L9.68 6l-2.76 6.4A1 1 0 016 13H3a1 1 0 010-2h2.34l2.51-5.79a2 2 0 013.79.38L14.32 18l2.76-6.38A1 1 0 0118 11h3a1 1 0 010 2h-2.34l-2.51 5.79A2 2 0 0114.33 20z" />
    </Svg>

regular render:
Screen Shot 1399-06-21 at 17 24 27

render with react-sketchapp:
Screen Shot 1399-06-21 at 17 47 16

@macintoshhelper
Copy link
Contributor

macintoshhelper commented Sep 13, 2020

Hi, this an issue with Sketch's SVG path rendering, not with react-sketchapp. I had trouble importing it into Illustrator, so the SVG itself appears to be problematic; I would try recreating the vector path manually in Sketch, then export it as SVG.

If you paste the raw SVG into Sketch, it renders in the same way:

<svg viewBox="0 0 24 24" fill="currentColor" focusable="false" width="24" height="24">
  <path d="M14.33 20h-.21a2 2 0 01-1.76-1.58L9.68 6l-2.76 6.4A1 1 0 016 13H3a1 1 0 010-2h2.34l2.51-5.79a2 2 0 013.79.38L14.32 18l2.76-6.38A1 1 0 0118 11h3a1 1 0 010 2h-2.34l-2.51 5.79A2 2 0 0114.33 20z" />
</svg>

Also, the width/height of 1em should be replaced with 24, as non-browser environments don't support em/rem.

@macintoshhelper
Copy link
Contributor

macintoshhelper commented Sep 13, 2020

I've fixed the SVG for you, using https://svg-path-visualizer.netlify.app/ :

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="#000000" width="24" height="24">
  <path d="M 14.33,20 h -0.21 a 2,2 0 0 1 -1.76,-1.58 L 9.68,6 l -2.76,6.4 A 1,1 0 0 1 6,13 H 3 a 1,1 0 0 1 0,-2 h 2.34 l2.51,-5.79 a 2,2 0 0 1 3.79.38 L 14.32 18 l 2.76,-6.38 A 1,1 0 0 1 18,11 h 3 a 1,1 0 0 1 0,2 h -2.34 l -2.51,5.79 A 2,2 0 0 1 14.33,20 Z"></path>
</svg>

Chrome and react-native-svg are applying corrections to the vector path data, whereas Sketch.app isn't – there are missing commas and spaces, which I added back in.

You can test Sketch's SVG implementation by replacing <Svg> with <svg>, <Svg.Path> with <path>, etc, and paste it into Sketch document manually (cmd + V), in my experience SVG issues have been with Sketch itself, not with react-sketchapp.

@sayjeyhi
Copy link
Author

sayjeyhi commented Sep 13, 2020

Thanks, @macintoshhelper, actually my problem is a little bit bigger. I have a big repo of SVG icons, and I convert them to react native svg icons using svgr, I have this bug in a lot of my icons and I don't know is there a programmatically way to prevent this bug or not!

you can clone this repo:
https://github.com/iconsbox/sketch-integration

then install packages and run yarn makeSvgNativeFiles this will call scripts/makeSvgNativeFiles.js and will create all IconBox icons in a native friendly format. then you can make a plugin and see that 40% of icons get damaged...

All svg files are working in https://iconbox.space and all of them are official ones.
I don't know if a regex or something could fix these spacings in SVG convert or no I should find another way.

@macintoshhelper
Copy link
Contributor

macintoshhelper commented Sep 13, 2020

Ah, yes, I've run into the same issue with svgr and react-sketchapp. This is an issue with svgo “optimising” the path data for web browsers. Try creating a .svgo.yml file in your root directory (where svgr is run from) with this in the contents:

- convertPathData: false

ref: gregberge/svgr#334

If you're using @svgr/webpack, you can use this in the webpack.skpm.config.js export function:

  config.module.rules[1].test = /^(?!.*\.(jsx?|tsx?|json|svg|md|nib|xib|framework|xcodeproj|xcworkspace|xcworkspacedata|pbxproj)$).*/;

  config.module.rules.push({
    test: /\.svg$/,
    use: [
      {
        loader: 'babel-loader',
        options: {
          presets: ['@skpm/babel-preset'],
        },
      },
      {
        loader: '@svgr/webpack',
        options: {
          native: true,
          svgoConfig: {
            plugins: [{ convertPathData: false }],
          },
          babel: false,
        },
      },
    ],
  });

(or can pass this object to the @svgr/core options)

svgr(svgFileContent, {
  native: true,
  svgoConfig: {
    plugins: [{ convertPathData: false }],
  },
}, ...)

@sayjeyhi
Copy link
Author

sayjeyhi commented Sep 14, 2020

@macintoshhelper Thanks a lot. I disabled svgo for svgr and get a better result, but still, there are some bugs and they come from the source svg which already has an applied svgo.

Screen Shot 1399-06-24 at 11 52 15

for e.g we have second svg which is AirPlayIcon:

import * as React from "react";
import Svg, { Path } from "react-sketchapp/lib/components/Svg";

function AirplayIcon(props) {
  return <Svg fill="none" stroke="currentColor" strokeWidth={2} strokeLinecap="round" strokeLinejoin="round" viewBox="0 0 24 24" focusable="false" {...props}>
    <Path d="M5 17H4a2 2 0 01-2-2V5a2 2 0 012-2h16a2 2 0 012 2v10a2 2 0 01-2 2h-1" />
    <Path d="M12 15l5 6H7l5-6z" />
  </Svg>;
}

export default AirplayIcon;

could we make a function out there to work on path data and make them Sketch friendly like a reverse work of what svgo did? maybe like what Mathieu did here.

@macintoshhelper
Copy link
Contributor

macintoshhelper commented Sep 14, 2020

Can try some svg path parsing packages:

e.g.

import svgpath from 'svgpath';

const transformed = svgpath('M14.33 20h-.21a2 2 0 01-1.76-1.58L9.68 6l-2.76 6.4A1 1 0 016 13H3a1 1 0 010-2h2.34l2.51-5.79a2 2 0 013.79.38L14.32 18l2.76-6.38A1 1 0 0118 11h3a1 1 0 010 2h-2.34l-2.51 5.79A2 2 0 0114.33 20z')
  .toString();

svgo is open-source, so you can reverse the convertPathData plugin: https://github.com/svg/svgo/blob/master/plugins/convertPathData.js

It appears that Sketch isn't following the W3 spec. Btw, there's another issue about this: #478

@mathieudutour
Copy link
Collaborator

I'm wondering if the nodejs version of react-sketchapp manage to generate those properly (it parses the svg path and build the points manually without relying on Sketch's parsing). If so we could try to do the same in the Sketch version as well

@macintoshhelper
Copy link
Contributor

macintoshhelper commented Sep 14, 2020

I'm wondering if the nodejs version of react-sketchapp manage to generate those properly (it parses the svg path and build the points manually without relying on Sketch's parsing). If so we could try to do the same in the Sketch version as well

Just tested (used renderToJSON in Node env, and patched the JSON into a Sketch file) and it seems to work 🎉 . Hmm, that makes sense. I'm wondering whether there'd be any side-effects with diverging from the Sketch implementation, would the Node-based SVG renderer still handle d3, etc (i.e. API parity with Sketch implementation)?

(For reference: https://github.com/airbnb/react-sketchapp/blob/master/__tests__/jest/components/nodeImpl/Svg.tsx#L28 – produces Sketch JSON with the points parsed correctly & https://github.com/airbnb/react-sketchapp/blob/master/src/jsonUtils/makeSvgLayer/index.ts#L86 )

@sayjeyhi
Copy link
Author

@thanks @macintoshhelper, I used svgpath and it worked great.

For anyone else figuring this issue. my code is:

let svgFileContent = await fse.readFile(icon, "utf-8");
svgFileContent = svgFileContent.replace(
  /(<path(.*)d=["']?((?:.(?!["']?\s*?[>"']))+.)["']?(.*)>)/gm,
  (wholes, p1, p2, p3, p4) => {
    const path = svgpath(p3).toString();
    return `<path${p2} d="${path}"${p4}>`;
  },
);

Screen Shot 1399-06-27 at 14 56 38

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

No branches or pull requests

3 participants