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

prependUnicode option should NOT be suggested #191

Open
crowmagnumb opened this issue Jan 19, 2021 · 4 comments
Open

prependUnicode option should NOT be suggested #191

crowmagnumb opened this issue Jan 19, 2021 · 4 comments

Comments

@crowmagnumb
Copy link

The README contains the line ...

prependUnicode: true, // recommended option

...so I went ahead and just blindly added that. Then when I couldn't figure out why my source SVG fyiles had changed name, I started looking back at the code and that jumped out at me. Could it be that? Of course it was. I have no idea why you would want your source files names to be changed but fine, if you want that option use it. But to claim it as a "recommended" option without any explanation on the page as to what it does is very confusing, and in my mind leads to unwelcome (and unexpected) results.

Cheers.

@sabberworm
Copy link
Collaborator

I agree the documentation on this should be improved. I think the reason this is a recommended option is that it’s the only way to guarantee that code points of previous icons don’t change when new icons are added to the font. If the file name contains the code point, it’s fixed and will be used for the next run, too.

@crowmagnumb
Copy link
Author

crowmagnumb commented Jan 20, 2021

Ah, didn't know it would do that. Wish I had known. :) I used the on glyphs event to write out a json file that serves as a map and that my code slurps up to convert from strings to codes. I certainly see now why it might be "recommended". :) Cheers.

            return gulp
                .src(SVG_ICONS)
                .pipe(
                    iconfont({
                        fontName: "myfont",
                        formats: ["ttf", "svg", "woff"], // 'woff2' and 'eot' also available
                    })
                )
                .on("glyphs", function (glyphs, options) {
                    const map = {};
                    for (const glyph of glyphs) {
                        map[glyph.name] = glyph.unicode[0].charCodeAt(0);
                    }
                    fs.writeFile(
                        path.join(
                            CORE_ASSETS_DIR,
                            `iconmap.${ICONSET_VERSION}.json`
                        ),
                        JSON.stringify(map),
                        {
                            encoding: "UTF-8",
                        },
                        (err) => {
                            if (err) {
                                console.log(err);
                            }
                        }
                    );
                })
                .pipe(
                    rename((path) => {
                        path.basename += `.${ICONSET_VERSION}`;
                    })
                )
                .pipe(gulp.dest(path.join(CORE_ASSETS_DIR, "fonts")));

@nfroidure
Copy link
Owner

@crowmagnumb would you mind to PR the README.md so that no one ever fall into that trap again?

@crowmagnumb
Copy link
Author

Yeah, was going to see if I could do that anyway. It's on my TODO list. :)

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