Proposal: Make icons multipath for more advanced CSS styling

vladimir-djokic commented 6 years ago

It would be great if icons that consist of multiple elements would have multiple paths, each with class name that could be used to style it (CSS).

For example: cloud-download would have 2 paths, one for cloud, one for down arrow. Each path would have it's own class name accessible using CSS for styling purposes.

reiv commented 6 years ago

If I'm understanding you correctly, couldn't you layer a second Icon component on top of another (using position: absolute) and then apply whatever additional CSS classes are desired? Sure, you'd get a bit more markup, but visually it should be the same? JSFiddle here. But I don't think the icons were designed with this kind of layering in mind.

giladgray commented 6 years ago

@vladeck while this is a very nice idea, we deliberately did not design the icons this way and it is far too expensive to make such a change at this point (do you want to redraw 700+ images?).

vladimir-djokic commented 6 years ago

What was the reason to deliberately design icons this way?

I wasn't talking about redrawing, more about editing existing icons and I meant this as a proposal for some future version (3.0?) not right-away[TM] :)

For example, cloud-download.svg would become (I'm no SVG expert):

<svg version="1.1" id="Layer_1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" x="0px" y="0px"
    viewbox="0 0 16 16" enable-background="new 0 0 16 16" xml:space="preserve">
    <g id="cloud_download">
        <g>
            <path class="pt-cloud-download-cloud" fill-rule="evenodd" clip-rule="evenodd" d="M 7,0 C 4.24,0 2,2.24 2,5 2,5.11 2.0095313,5.2200781 2.0195312,5.3300781 0.82953125,5.8900781 0,7.1 0,8.5 0,9.91 0.83929687,11.109922 2.0292969,11.669922 2.1992969,10.169922 3.46,9 5,9 5.06,9 5.1294531,9.0195312 5.1894531,9.0195312 5.0694531,8.6995312 5,8.36 5,8 5,6.34 6.34,5 8,5 9.66,5 11,6.34 11,8 11,8.36 10.930547,8.6995312 10.810547,9.0195312 10.870547,9.0195312 10.94,9 11,9 c 1.48,0 2.699219,1.070703 2.949219,2.470703 C 15.169219,10.790703 16,9.5 16,8 16,5.79 14.21,4 12,4 11.97,4 11.930391,3.9997656 11.900391,4.0097656 11.440391,1.7197656 9.42,0 7,0 Z"/>
            <path class="pt-cloud-download-down-arrow" fill-rule="evenodd" clip-rule="evenodd" d="m 8,6.9980469 c -0.55,0 -1,0.45 -1,1 V 12.587891 L 5.7109375,11.298828 C 5.5309375,11.108828 5.28,10.998047 5,10.998047 c -0.55,0 -1,0.45 -1,1 0,0.28 0.1090625,0.530937 0.2890625,0.710937 l 3,3 c 0.18,0.18 0.4309375,0.289063 0.7109375,0.289063 0.28,0 0.5309375,-0.109063 0.7109375,-0.289063 l 3.0000005,-3 C 11.890935,12.528984 12,12.278047 12,11.998047 c 0,-0.55 -0.45,-1 -1,-1 -0.28,0 -0.530932,0.111016 -0.710938,0.291015 L 9,12.587891 V 7.9980469 c 0,-0.55 -0.45,-1 -1,-1 z"/>
        </g>
    </g>
</svg>

And example.html:

<!DOCTYPE html>
<html>
<head>
    <meta charset="utf-8" />
    <title>SVG Icon Styling Example</title>
</head>
<body>

    <style>
        svg {
            width: 16px;
            height: 16px;
        }

        .pt-cloud-download-cloud {
            fill: gray;
        }

        .pt-cloud-download-down-arrow {
            fill: dodgerblue;
        }
    </style>

    <svg version="1.1" id="Layer_1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" x="0px" y="0px"
        viewbox="0 0 16 16" enable-background="new 0 0 16 16" xml:space="preserve">
        <g id="cloud_download">
            <g>
                <path class="pt-cloud-download-cloud" fill-rule="evenodd" clip-rule="evenodd" d="M 7,0 C 4.24,0 2,2.24 2,5 2,5.11 2.0095313,5.2200781 2.0195312,5.3300781 0.82953125,5.8900781 0,7.1 0,8.5 0,9.91 0.83929687,11.109922 2.0292969,11.669922 2.1992969,10.169922 3.46,9 5,9 5.06,9 5.1294531,9.0195312 5.1894531,9.0195312 5.0694531,8.6995312 5,8.36 5,8 5,6.34 6.34,5 8,5 9.66,5 11,6.34 11,8 11,8.36 10.930547,8.6995312 10.810547,9.0195312 10.870547,9.0195312 10.94,9 11,9 c 1.48,0 2.699219,1.070703 2.949219,2.470703 C 15.169219,10.790703 16,9.5 16,8 16,5.79 14.21,4 12,4 11.97,4 11.930391,3.9997656 11.900391,4.0097656 11.440391,1.7197656 9.42,0 7,0 Z"/>
                <path class="pt-cloud-download-down-arrow" fill-rule="evenodd" clip-rule="evenodd" d="m 8,6.9980469 c -0.55,0 -1,0.45 -1,1 V 12.587891 L 5.7109375,11.298828 C 5.5309375,11.108828 5.28,10.998047 5,10.998047 c -0.55,0 -1,0.45 -1,1 0,0.28 0.1090625,0.530937 0.2890625,0.710937 l 3,3 c 0.18,0.18 0.4309375,0.289063 0.7109375,0.289063 0.28,0 0.5309375,-0.109063 0.7109375,-0.289063 l 3.0000005,-3 C 11.890935,12.528984 12,12.278047 12,11.998047 c 0,-0.55 -0.45,-1 -1,-1 -0.28,0 -0.530932,0.111016 -0.710938,0.291015 L 9,12.587891 V 7.9980469 c 0,-0.55 -0.45,-1 -1,-1 z"/>
            </g>
        </g>
    </svg>

</body>
</html>

And the result:

cloud-download

reiv commented 6 years ago

@vladeck Ohhh. I thought you were talking about composing existing icons with each other, but this is pretty neat. I think it would be possible to extract individual paths using the generate-icons-source script, without having to touch the icon source files¹. My suggestion would be to use a path:nth-of-type or nth-child selector rather than hand-tagging all those individual classes (they'd increase the bundle size too)... even though that's not as nice and semantic.

[1] E.g. by splitting the path string on Z (close path).

llorca commented 6 years ago

Adding a class to every path seems quite overkill. As Gilad mentioned, Blueprint hasn't had a use case for this, we've only needed simple shape, unicolor UI icons.

Now, if it is possible to automatically optimize the SVG markup of each icon (as done in generate-icons-source) without combining every path into one (i.e. preserving each <path>), and in a way that (almost) doesn't increase the bundle size, I'd be okay with it.

vladimir-djokic commented 6 years ago

I'm "down" with changing generate-icons-source so that the icons are generated with multiple paths (split on Z) and then styled (for those who want fine-grained coloring) with path:nth-.... I'll play around with the idea when the time permits and investigate the implications on bundle size and overall usage.

giladgray commented 6 years ago

@vladeck I would happily review a submission for this 🔼!

vladimir-djokic commented 6 years ago

I tried, but is hard to develop/test when project is not capable of building on Windows (followed instructions in README.md and yarn verify fails complaining about missing bash). I managed to isolate scripts/svg files and did some prototyping:

async function buildPathsObject(objectName, size) {
    return Promise.all(
        ICONS_METADATA.map(async icon => {
            const filepath = path.resolve(__dirname, `../../resources/icons/${size}px/${icon.iconName}.svg`);
            const svg = fs.readFileSync(filepath, "utf-8");

            // Get d="...".
            const pathsDees = svg.match(/ d="[^"]+"/g) || [];

            // Get actual value of d="..." -> ...
            const pathsDeesMapped = pathsDees.map(s => s.slice(4, s.length - 1));

            const splitPathsDees = [];

            // Split on z or Z, rejecting empty splits.
            pathsDeesMapped.forEach(p => {
                const split = p.split(/[zZ]/).filter(s => s);
                split.forEach(s => splitPathsDees.push(`${s}z`));
            })

            const newPaths = [];

            // Construct array of <path /> entries for each split.
            splitPathsDees.forEach(p => {
                newPaths.push(`<path fill-rule="evenodd" clip-rule="evenodd" d="${p}"/>`)
            });

            // Construct new svg file content, replacing single <path /> with (potentially) multiple <path /> entries.
            const newSvg = svg.replace(/<path.*\/>|<path.*\/path>/gs, newPaths.join("\n"));

            const pathStrings = await svgo
                .optimize(newSvg)
                .then(({ data }) => data.match(/ d="[^"]+"/g) || [])
                .then(paths => paths.map(s => s.slice(3)));
            return `    "${icon.iconName}": [${pathStrings.join(",\n")}],`;
        }),
    );
}

However, this fails at some point (for one of zoom 20x20 icons) with: Error in parsing SVG: Unexpected close tag. Again, I'm no SVG expert and do not understand the inner-workings of SVGO so maybe someone who has working dev environment can take a look at this?

Note: trying to split on Z after optimization does not yield correctly looking icons.

giladgray commented 6 years ago

@vladeck do not generate the SVG elements in the script. inspect the existing output: it's just an array of strings (the content of d="...") and the SVG elements are created in the Icon component. i imagine you could simply add .split("Z") after this line.

vladimir-djokic commented 6 years ago

Wish that it was that easy: splitting path on z or Z after optimize(...) from SVGO will not produce icons that are correctly rendered (as I noted in my previous comment).

giladgray commented 6 years ago

@vladeck ok i'm not at all surprised by that. this confirms my original comment: the icons were not drawn with this feature in mind, so it's likely that supporting this request would require redrawing most of the images to split the paths as desired.

I'm going to close this again as it seems quite impractical at this time.

llorca commented 6 years ago

@giladgray what about turning off mergePaths in SVGO?

giladgray commented 6 years ago

@llorca i went down that road too, but the cruel fact is that the SVG images themselves only contain a single <path> element, so there's nothing to merge. 😢

llorca commented 6 years ago

gotcha, then that's a @pkwi question, probably an Illustrator export setting that does not merge paths or something

vladimir-djokic commented 6 years ago

@giladgray @llorca

How about if we create script that will take current single-path .svg files and convert them to multi-path .svg files (by splitting on z or Z)? Those converted icons will then become basis for all future releases? That way we would not need to re-edit all current icons by hand, just make sure that all future ones are multi-path, if needed?

vladimir-djokic commented 6 years ago

/**
 * Script that can convert any single-path to multi-path `.svg` file by
 * splitting the path on `z` or `Z`.
 */
const fs = require("fs");
const path = require("path");
const process = require("process");
const program = require("commander");

/**
 * Reads (asynchronous) contents of the file.
 *
 * @param filename Filename to read from.
 *
 * @returns `Promise` resolved to the contents of the file or rejected in case
 * of error.
 */
const readFileAsync = async (filename) => {
    return new Promise((resolve, reject) => {
        fs.readFile(
            filename,
            "utf-8",
            (error, data) => {
                if (error) {
                    reject(error);
                    return;
                }

                resolve(data);
            }
        );
    });
}

/**
 * Writes (asynchronous) contents to the file.
 *
 * @param filename Filename to write to.
 *
 * @returns `Promise` resolved upon successfull completion of write operations
 * or rejected in case of error.
 */
const writeFileAsync = async (filename, content) => {
    return new Promise((resolve, reject) => {
        fs.writeFile(
            filename,
            content,
            (error) => {
                if (error) {
                    reject(error);
                    return;
                }

                resolve();
            }
        )
    });
}

/**
 * Retrieves (asynchronous) all files in a directory matching provided extension.
 *
 * @param dirname Directory to retrieve files from.
 * @param extension Extension to match files against.
 *
 * @returns `Promise` resolved to array of files or rejected in case of error.
 */
const readDirAsync = async (dirname, extension) => {
    return new Promise((resolve, reject) => {
        fs.readdir(
            dirname,
            "utf-8",
            (error, files) => {
                if (error) {
                    reject(error);
                    return;
                }

                resolve(
                    files
                        .filter(f => path.extname(f) === extension)
                        .map(f => path.join(dirname, f))
                );
            }
        );
    });
}

/**
 * Copies (asynchronous) file from `source` to `destination`.
 *
 * @param source Source file to copy.
 * @param destination Destination file to copy to.
 *
 * @returns `Promise` resolved or rejected in case of error.
 */
const copyFileAsync = async (source, destination) => {
    return new Promise((resolve, reject) => {
        fs.copyFile(
            source,
            destination,
            0,
            (error) => {
                if (error) {
                    reject(error);
                    return;
                }

                resolve();
            }
        );
    });
}

const convert = async () => {
    try {
        program
            .version("0.0.1")
            .option("-s, --sourceDir <value>", "Source directory")
            .parse(process.argv);

        if (program.sourceDir === undefined)
            program.help(() => "Error: 'sourceDir' must be provided.");

        await convertImp(program.sourceDir);
    }
    catch (error) {
        process.stdout.write(`Error: ${error}`);
    }
}

const convertImp = async (sourceDir) => {
    // Get paths of source `.svg` files.
    sourcePaths = await readDirAsync(sourceDir, ".svg");

    // Create backups.
    await Promise.all(
        sourcePaths.map(
            p => copyFileAsync(p, `${p}.backup`)
        )
    );

    // Do the conversion.
    await Promise.all(
        sourcePaths.map(
            p => convertSvg(p)
        )
    )
}

const convertSvg = async (filename) => {
    // Get original `.svg` content.
    const svgContent = await readFileAsync(filename);

    const pathDMatch = svgContent.match(/\sd="([^"]+)"/);

    if (!pathDMatch) {
        return;
    }

    const pathDContent = pathDMatch[1];

    const splitPathContent = pathDContent.split(/[zZ]/).filter(s => s).map(s => `${s.trim()}z`);

    const newPaths = splitPathContent.map(p => `<path fill-rule="evenodd" clip-rule="evenodd" d="${p}"/>`);
    const newSvgContent = svgContent.replace(/<path.*\/>|<path.*\/path>/gs, newPaths.join("\n"));

    // Save converted `.svg` content.
    await writeFileAsync(filename, newSvgContent);
}

convert();

pkwi commented 6 years ago

@llorca currently most (almost all) icons are single-path svgs so re-exporting (re-drawing) all as multi-path would be extremely time consuming.

giladgray commented 6 years ago

@vladeck if you can prove that it doesn't break any of the images then i'll happily review a contribution! that said, i'm not confident you can simply split all paths on z without breaking the resulting image.

vladimir-djokic commented 6 years ago

This was my first run of the script:

convert_first_run

There are some issue where cut-outs in one-color (original) versions are considered as separate paths in converted versions while some of them being in the wrong order making them not show (because they are "under" larger path).

I'll keep working on this in my spare time and if I crack how to correctly determine path overlapping I believe that will pave the way for correct conversion.

giladgray commented 6 years ago

ok that's pretty neat! i'll reopen this so it appears in the main list.

llorca commented 6 years ago

that's looking pretty good. so is it like the reverse algorithm of SVGO's mergePaths? anybody recommend some good reading on merging/unmerging SVG paths?

vladimir-djokic commented 6 years ago

This is my current progress (still WIP). I have re-edited 436 16px icons (not including blank.svg in the count). Icons that would benefit from multi-path design are now "split" into 2-paths. In order to better visualize changes, I have colored one path in blue and another in red.

Before going any further (still a lot of work to do on these), I have a few questions for the team:

How should I go about in submitting a PR if I'm unable to build Blueprintjs on Windows? Do build scripts work on Linux (I can install a Linux under VM)? I want to make sure that all existing scripts are compatible with my changes.
Is it OK to do PR in chunks? For example, first 16px and then 20px icons or sould PR be done in "one go"?
I would really like to cleanup all SVG files, as most of them have a lot of "junk" left in them - unneeded groups, IDs introduced by editing tools, ... Does anyone object?
Do you feel like 2-path design is enough, or should we talk about introducing more paths for icons that would benefit from it (edit.svg, layers.svg, ...)
I would like to re-edit some Icons by doing more than just "splitting" paths. For example, lock.svg and unlock.svg do not align (at least the base) and arcs are not consistent. There are more examples for these kind of visual inconsistencies... Does anyone object?

blueprintjs_icons_16px_issues

Full preview of work so far:

blueprintjs_icons_16px_rework

reiv commented 6 years ago

I have re-edited 436 16px icons

By hand? How long did that take you? :open_mouth:

Anyway, it should be possible to build on Windows as long as the right tools are installed. You mentioned that bash is missing on your system -- if you installed Git for Windows you should already have it (check in C:\Program Files\Git\bin) but it might not be in your PATH variable; in that case it should be safe to add it there to make it available globally.

Alternatively, if you're on Windows 10 you can look into setting up WSL (Windows Subsystem for Linux).

pkwi commented 6 years ago

I would like to re-edit some Icons by doing more than just "splitting" paths. For example, lock.svg and unlock.svg do not align (at least the base) and arcs are not consistent. There are more examples for these kind of visual inconsistencies... Does anyone object?

Not sure I'm ok with this. It makes sense to align both at the base if you use these icons to toggle lock/unlocked states. However, to make this work, we would have to align the lock icon fully to the left which will cause alignment issues when this icon is used in Menus.

reiv commented 6 years ago

@pkwi The icon is used as a toggle in this docs example. Rather than align to the left, why not center the lock in both versions, leaving the same amount of space for padding in the closed state as is taken up by the shackle in the open state?

pkwi commented 6 years ago

@reiv yeah this would align the icons nicely. Although, we tried this before and the feedback we have received pointed out that it's hard to distinguish between the open and close states, especially with smaller (16px) versions.

llorca commented 6 years ago

Whoa, this is very cool @vladeck. Can you share rough stats on SVG file sizes before and after?

Is it OK to do PR in chunks? For example, first 16px and then 20px icons or sould PR be done in "one go"?

Both would be better, though that's fine to split out if you can create a feature branch.

I would really like to cleanup all SVG files, as most of them have a lot of "junk" left in them - unneeded groups, IDs introduced by editing tools, ... Does anyone object?

I'm surprised to hear that. I thought SVGO would do that clean up automatically. Instead of doing this work manually, is there a way to do it automatically? Maybe we're missing some kind of SVGO config? I don't think it's viable to do this manually now or over time.

Do you feel like 2-path design is enough, or should we talk about introducing more paths for icons that would benefit from it (edit.svg, layers.svg, ...)

2-path is already looking great for most icons, but I'm sure some other icons would benefit from additional paths. @pkwi is the best person to help you here 👍

I would like to re-edit some Icons by doing more than just "splitting" paths. For example, lock.svg and unlock.svg do not align (at least the base) and arcs are not consistent. There are more examples for these kind of visual inconsistencies... Does anyone object?

As Piotr mentioned, this is intentional or else we'd end up with lots of misalignments in lists and menus, when such icons are used in a standalone way. I would prefer holding off on this, for now at least.

vladimir-djokic commented 6 years ago

@reiv

By hand? How long did that take you?

I used my script to automate the "splitting" of all paths, and then "by hand" edit (few hours) so that there are only 2 important paths left. Trial version of Affinity Designer helped me a lot!

As for building the codebase: I'll try what you proposed by installing WSL and see how it fares for building.

@llorca

Can you share rough stats on SVG file sizes before and after?

Sure. Here are the sizes:

before 417 KB after 436 KB

I'm surprised to hear that. I thought SVGO would do that clean up automatically.

The "junk" I mentioned is in the "raw" .svg files itself, before being processed by SVGO (and optimized) for Blueprintjs's consumption (if I understand how it all falls into place).

giladgray commented 6 years ago

@vladeck don't bother cleaning up the raw SVG files. you'll notice they're not published to NPM (they don't live in the packages/ folder) and we run them through SVGO before publishing only the d="" path strings in the icons package. so all the extra cruft like groups and IDs don't even make it to the final published package.

vladimir-djokic commented 6 years ago

As for the lock icon, I would do something like this:

blueprintjs_icons_16px_issues_proposal

Both icons (at least the base) remain centered. I think this would still look good in the menus and would look great when having the transition from 'locked' to 'unlocked' and vise-versa.

The bigger issue for me are miss-alignments present in some of the icons:

blueprintjs_icons_16px_issues_002

The next one also introduces additional SVG path data; by resolving these, files would become smaller in size - only by little, sure, but smaller nonetheless :)

blueprintjs_icons_16px_issues_003

pkwi commented 6 years ago

@vladeck I like the lock icon, I think this can work.

Yeah, those misalignments are annoying. I discovered that a lot of it happens when exporting the icon. Both lock and unlock icons use the same rectangular base shape. However, somehow the lock icon's bottom radius gets distorted. Thanks for your help!

vladimir-djokic commented 5 years ago

Since I will not be able to dedicate time to work on this, I'm uploading all icons that I edited for 16x16 grid. It might prove usefull to someone that wants to tackle this problem.

Having two-tone icons would be incredible, we just need someone who can push through and edit all of them :)

16px.zip

palantir / blueprint

Proposal: Make icons multipath for more advanced CSS styling #2389