nfroidure/svgicons2svgfont

fixedWidth option does not fit icons' width

Closed this issue · 10 comments

mnpenner commented

Issue

I'm a gentledev i:

  • fully read the README recently
  • searched for existing issues
  • checked i'm up to date with the latest version of the project

Expected behavior

Widths should fit the icons.

Actual behavior

Hopefully you can see the light grey background behind the icons. That's the width of the icon. It's too narrow to fit the icons. When I turned fixedWidth off, the icons fit nicely but aren't a uniform width which doesn't work so well for my website design.

The icons look fine when I preview them in Chrome. I'm no expert on SVGs but I guess some of the meta-data that determines the bounding box isn't coming out right. Or it could be a bug in the SVG->TTF->WOFF conversion but it seems to work fine without fixed width, so I'm not sure.

Steps to reproduce the behavior

Here's the settings I'm using:

const fontStream = new SVGIcons2SVGFontStream({
    fontName: fontName,
    normalize: true,
    fontHeight: 5000,
    fixedWidth: true,
    centerHorizontally: true,
    log: () => {
    },
});

Or you can view the whole script.

Debugging informations

  • node -v result:
v8.9.4
  • npm -v result:
5.6.0

If the result is lower than 6.9.5, there is
poor chances i even have a look to it. Please,
use the last NodeJS LTS version.

NaridaL commented

Can you link the icons you are using in your example?

NaridaL commented

Yeah there's a bug, the width isn't getting correctly scaled with the fontHeight... which font viewer are you using which is showing the background rectangles?

mnpenner commented
NaridaL commented

@mnpenner There is currently odd behavior: when normalize is off, "fontHeight" is basically ignored except as the "units-per-em" attribute in the output. When normalize is on, fontHeight is used as the value to scale all icons to, instead of to the "max icon height" as stated in the docs. This incongruity seems like a bug.

What exactly are you trying to achieve by setting fontHeight? Isn't it basically just used as a relative value? Would just letting the "units-per-em" value default to max icon height, and specifying the font size where you use it be an option?

mnpenner commented

What exactly are you trying to achieve by setting fontHeight?

It's recommended that fontHeight be at least 1000, no?

Would just letting the "units-per-em" value default to max icon height, and specifying the font size where you use it be an option?

Uhh... I don't know. Will the icons still come out accurate and normalized? I can try this a bit later I guess.

I managed to fix my particular problem by 'cropping' the source SVGs in Illustrator, so this isn't a big deal for me anymore -- I was just trying to get all the icons to align nicely in my sidebar. They were slightly different widths which was misaligning the text.

NaridaL commented

Yeah, you're right, looks like having the actual values too small leads to problems down the line (at least with woff font format). Possibly something is rounding values too much. In any case, the fontHeight should then also increase all values even when normalize is off. I will investigate further.

  • 👍1
NaridaL commented

@mnpenner I published a new version (9.0.0). It would be great if you could test your script with it.

mnpenner commented

fixedWidth=true fixedWidth=false

Looks like it worked perfectly! Thank you so much.

NaridaL commented

You're welcome!