From a5c4adaf567dba0e55eb591a5cdc05669246f494 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Mon, 25 Oct 2021 15:45:32 -0400 Subject: [PATCH] [v4] [icons] fix: icon paths are right-side up (#4984) --- packages/core/src/components/icon/_icon.scss | 3 - packages/core/src/components/icon/icon.tsx | 8 +- .../core-examples/common/iconNames.ts | 29 +++++ .../core-examples/common/iconSelect.tsx | 15 ++- packages/icons/package.json | 2 +- packages/icons/scripts/common.js | 42 +++++++ packages/icons/scripts/generate-icon-fonts.js | 6 +- packages/icons/scripts/generate-icon-paths.js | 103 +++++++++--------- packages/node-build-scripts/package.json | 1 - yarn.lock | 5 - 10 files changed, 137 insertions(+), 77 deletions(-) create mode 100644 packages/docs-app/src/examples/core-examples/common/iconNames.ts create mode 100644 packages/icons/scripts/common.js diff --git a/packages/core/src/components/icon/_icon.scss b/packages/core/src/components/icon/_icon.scss index cd62c3842b..feff3fdca6 100644 --- a/packages/core/src/components/icon/_icon.scss +++ b/packages/core/src/components/icon/_icon.scss @@ -31,9 +31,6 @@ $icon-classes: ( > svg { // prevent extra vertical whitespace display: block; - // paths parsed by generate-icon-paths.js are mirrored vertically, so we need - // to flip them upright here - transform: scaleY(-1); // inherit text color unless explicit fill is set &:not([fill]) { diff --git a/packages/core/src/components/icon/icon.tsx b/packages/core/src/components/icon/icon.tsx index 455017d8a2..441473510e 100644 --- a/packages/core/src/components/icon/icon.tsx +++ b/packages/core/src/components/icon/icon.tsx @@ -150,12 +150,12 @@ export class Icon extends AbstractPureComponent2` elements for the given icon name. Returns `null` if name is unknown. */ - private renderSvgPaths(pathsSize: number, iconName: IconName): JSX.Element | null { + private renderSvgPaths(pathsSize: number, iconName: IconName): JSX.Element[] | null { const svgPathsRecord = pathsSize === IconSize.STANDARD ? IconSvgPaths16 : IconSvgPaths20; - const pathString = svgPathsRecord[iconNameToPathsRecordKey(iconName)]; - if (pathString == null) { + const paths = svgPathsRecord[iconNameToPathsRecordKey(iconName)]; + if (paths == null) { return null; } - return ; + return paths.map((path, i) => ); } } diff --git a/packages/docs-app/src/examples/core-examples/common/iconNames.ts b/packages/docs-app/src/examples/core-examples/common/iconNames.ts new file mode 100644 index 0000000000..384be736f0 --- /dev/null +++ b/packages/docs-app/src/examples/core-examples/common/iconNames.ts @@ -0,0 +1,29 @@ +/* + * Copyright 2021 Palantir Technologies, Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { IconName, IconNames } from "@blueprintjs/icons"; + +export const NONE = "(none)"; +export type IconNameOrNone = IconName | typeof NONE; + +export function getIconNames(): IconNameOrNone[] { + const iconNames = new Set(); + for (const [, name] of Object.entries(IconNames)) { + iconNames.add(name); + } + iconNames.add(NONE); + return Array.from(iconNames.values()); +} diff --git a/packages/docs-app/src/examples/core-examples/common/iconSelect.tsx b/packages/docs-app/src/examples/core-examples/common/iconSelect.tsx index 8da17e1e4c..f0a9540c18 100644 --- a/packages/docs-app/src/examples/core-examples/common/iconSelect.tsx +++ b/packages/docs-app/src/examples/core-examples/common/iconSelect.tsx @@ -17,20 +17,19 @@ import * as React from "react"; import { Alignment, Button, Classes, MenuItem } from "@blueprintjs/core"; -import { IconName, IconNames } from "@blueprintjs/icons"; +import { IconName } from "@blueprintjs/icons"; import { ItemRenderer, Select } from "@blueprintjs/select"; +import { getIconNames, IconNameOrNone, NONE } from "./iconNames"; + +const ICON_NAMES = getIconNames(); + export interface IIconSelectProps { iconName?: IconName; onChange: (iconName?: IconName) => void; } -const NONE = "(none)"; -type IconType = IconName | typeof NONE; -const ICON_NAMES = Object.keys(IconNames).map((name: string) => IconNames[name as keyof typeof IconNames]); -ICON_NAMES.push(NONE); - -const TypedSelect = Select.ofType(); +const TypedSelect = Select.ofType(); export class IconSelect extends React.PureComponent { public render() { @@ -84,5 +83,5 @@ export class IconSelect extends React.PureComponent { return iconName.toLowerCase().indexOf(query.toLowerCase()) >= 0; }; - private handleIconChange = (icon: IconType) => this.props.onChange(icon === NONE ? undefined : icon); + private handleIconChange = (icon: IconNameOrNone) => this.props.onChange(icon === NONE ? undefined : icon); } diff --git a/packages/icons/package.json b/packages/icons/package.json index 5d00e00dd4..2acf8c9c5d 100644 --- a/packages/icons/package.json +++ b/packages/icons/package.json @@ -50,7 +50,7 @@ "react": "^16.14.0", "react-dom": "^16.14.0", "react-test-renderer": "^16.14.0", - "svg-parser": "^2.0.4", + "svgo": "^1.3.2", "typescript": "~4.1.2", "webpack-cli": "^3.3.12" }, diff --git a/packages/icons/scripts/common.js b/packages/icons/scripts/common.js new file mode 100644 index 0000000000..71bfe7a69e --- /dev/null +++ b/packages/icons/scripts/common.js @@ -0,0 +1,42 @@ +/* + * Copyright 2021 Palantir Technologies, Inc. All rights reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +const fs = require("fs"); +const path = require("path"); + +const COPYRIGHT_HEADER = "/*\n * Copyright 2021 Palantir Technologies, Inc. All rights reserved.\n */\n"; +const RESOURCES_DIR = path.resolve(__dirname, "../../../resources/icons"); +const GENERATED_SRC_DIR = path.resolve(__dirname, "../src/generated"); +const NS = "bp4"; + +/** + * Writes lines to given filename in GENERATED_SRC_DIR. + * + * @param {string} filename + * @param {Array} lines + */ +function writeLinesToFile(filename, ...lines) { + const outputPath = path.join(GENERATED_SRC_DIR, filename); + const contents = [COPYRIGHT_HEADER, ...lines, ""].join("\n"); + fs.writeFileSync(outputPath, contents); +} + +module.exports = { + COPYRIGHT_HEADER, + RESOURCES_DIR, + GENERATED_SRC_DIR, + NS, + writeLinesToFile, +}; diff --git a/packages/icons/scripts/generate-icon-fonts.js b/packages/icons/scripts/generate-icon-fonts.js index 5a52c87097..d4cc2fdfd0 100644 --- a/packages/icons/scripts/generate-icon-fonts.js +++ b/packages/icons/scripts/generate-icon-fonts.js @@ -22,11 +22,9 @@ const { getLogger } = require("fantasticon/lib/cli/logger"); const fs = require("fs"); const path = require("path"); -const RESOURCES_DIR = path.resolve(__dirname, "../../../resources/icons"); -const GENERATED_SRC_DIR = path.resolve(__dirname, "../src/generated"); -const logger = getLogger(); -const NS = "bp4"; +const { RESOURCES_DIR, GENERATED_SRC_DIR, NS } = require("./common"); +const logger = getLogger(); logger.start(); fs.mkdirSync(path.join(GENERATED_SRC_DIR, `16px/paths`), { recursive: true }); diff --git a/packages/icons/scripts/generate-icon-paths.js b/packages/icons/scripts/generate-icon-paths.js index 569a9a08ba..d43fd24e03 100644 --- a/packages/icons/scripts/generate-icon-paths.js +++ b/packages/icons/scripts/generate-icon-paths.js @@ -21,67 +21,68 @@ const { camelCase } = require("change-case"); const fs = require("fs"); const path = require("path"); -const { parse } = require("svg-parser"); - -const GENERATED_SRC_DIR = path.resolve(__dirname, "../src/generated"); -const COPYRIGHT_HEADER = "/*\n * Copyright 2021 Palantir Technologies, Inc. All rights reserved.\n */\n"; - -for (const iconSize of [16, 20]) { - const iconFontSvgDocument = fs.readFileSync( - path.join(GENERATED_SRC_DIR, `${iconSize}px/blueprint-icons-${iconSize}.svg`), - "utf8", - ); - - const icons = []; - console.info(`Parsing SVG glyphs from generated ${iconSize}px SVG icon font...`); - parseIconGlyphs(iconFontSvgDocument, (iconName, iconPath) => { - icons.push(iconName); - writeLinesToFile(`${iconSize}px/paths/${iconName}.ts`, `const path = "${iconPath}"`, "export default path;"); - }); - console.info(`Parsed ${icons.length} icons.`); - - console.info(`Writing index file for ${iconSize}px icon kit paths...`); - writeLinesToFile( - `${iconSize}px/paths/index.ts`, - ...icons.map(iconName => `export { default as ${camelCase(iconName)} } from "./${iconName}";`), - ); - console.info("Done."); -} +// Note: we had issues with this approach using svgo v2.x, so for now we stick with v1.x +// With v2.x, some shapes within the icon SVGs would not get converted to paths correctly, +// resulting in invalid d="..." attributes rendered by the component. +const SVGO = require("svgo"); /** - * Parse all icons of a given size from the SVG font generated by fantasticon. - * At this point we've already optimized the icon SVGs through svgo (via fantasticon), so - * we avoid duplicating that work by reading the generated glyphs here. - * - * @param {string} iconFontSvgDocument - * @param {(iconName: string, iconPath: string) => void} cb iterator for each icon path + * @typedef {Object} IconMetadata + * @property {string} displayName - "Icon name" for display + * @property {string} iconName - `icon-name` for IconName and CSS class + * @property {string} tags - comma separated list of tags describing this icon + * @property {string} group - group to which this icon belongs + * @property {string} content - unicode character for icon glyph in font */ -function parseIconGlyphs(iconFontSvgDocument, cb) { - const rootNode = parse(iconFontSvgDocument); - const defs = rootNode.children[0].children[0]; - const glyphs = defs.children[0].children.filter(node => node.tagName === "glyph"); - for (const glyph of glyphs) { - const name = glyph.properties["glyph-name"]; +/** @type {IconMetadata[]} */ +const ICONS_METADATA = require("../icons.json").sort((a, b) => a.iconName.localeCompare(b.iconName)); +const { RESOURCES_DIR, writeLinesToFile } = require("./common"); + +const svgo = new SVGO({ plugins: [{ convertShapeToPath: { convertArcs: true } }] }); +const ICON_NAMES = ICONS_METADATA.map(icon => icon.iconName); + +(async () => { + for (const iconSize of [16, 20]) { + const iconPaths = await getIconPaths(iconSize); - // HACKHACK: for some reason, there are duplicates with the suffix "-1", so we ignore those - if (name.endsWith("-1")) { - continue; + for (const [iconName, pathStrings] of Object.entries(iconPaths)) { + writeLinesToFile( + `${iconSize}px/paths/${iconName}.ts`, + `const paths: string[] = [${pathStrings.join(", ")}];`, + "export default paths;", + ); } - const path = glyph.properties["d"]; - cb(name, path); + console.info(`Writing index file for ${iconSize}px icon kit paths...`); + writeLinesToFile( + `${iconSize}px/paths/index.ts`, + ...ICON_NAMES.map(iconName => `export { default as ${camelCase(iconName)} } from "./${iconName}";`), + ); + console.info("Done."); } -} +})(); /** - * Writes lines to given filename in GENERATED_SRC_DIR. + * Loads SVG file for each icon, extracts path strings `d="path-string"`, + * and constructs map of icon name to array of path strings. * - * @param {string} filename - * @param {Array} lines + * @param {16 | 20} iconSize */ -function writeLinesToFile(filename, ...lines) { - const outputPath = path.join(GENERATED_SRC_DIR, filename); - const contents = [COPYRIGHT_HEADER, ...lines, ""].join("\n"); - fs.writeFileSync(outputPath, contents); +async function getIconPaths(iconSize) { + /** @type Record */ + const iconPaths = {}; + for (const iconName of ICON_NAMES) { + const filepath = path.join(RESOURCES_DIR, `${iconSize}px/${iconName}.svg`); + const svg = fs.readFileSync(filepath, "utf-8"); + const optimizedSvg = await svgo.optimize(svg, { path: filepath }); + const pathStrings = (optimizedSvg.data.match(/ d="[^"]+"/g) || []) + // strip off leading 'd="' + .map(s => s.slice(3)) + // strip out newlines and tabs, but keep other whitespace + .map(s => s.replace(/[\n\t]/g, "")); + iconPaths[iconName] = pathStrings; + } + console.info(`Parsed ${Object.keys(iconPaths).length} ${iconSize}px icons.`); + return iconPaths; } diff --git a/packages/node-build-scripts/package.json b/packages/node-build-scripts/package.json index 353d74d4aa..c904e84978 100644 --- a/packages/node-build-scripts/package.json +++ b/packages/node-build-scripts/package.json @@ -28,7 +28,6 @@ "strip-css-comments": "^4.1.0", "stylelint": "~13.8.0", "stylelint-junit-formatter": "^0.2.2", - "svgo": "^1.3.2", "tslint": "~6.1.3", "typescript": "~4.1.2", "yargs": "^17.1.1" diff --git a/yarn.lock b/yarn.lock index 8092873943..b83dd9a374 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12492,11 +12492,6 @@ supports-color@^6.1.0: dependencies: has-flag "^3.0.0" -svg-parser@^2.0.4: - version "2.0.4" - resolved "https://registry.yarnpkg.com/svg-parser/-/svg-parser-2.0.4.tgz#fdc2e29e13951736140b76cb122c8ee6630eb6b5" - integrity sha512-e4hG1hRwoOdRb37cIMSgzNsxyzKfayW6VOflrwvR+/bzrkyxY/31WkbgnQpgtrNp1SdpJvpUAGTa/ZoiPNDuRQ== - svg-pathdata@^5.0.0: version "5.0.5" resolved "https://registry.yarnpkg.com/svg-pathdata/-/svg-pathdata-5.0.5.tgz#65e8d765642ba15fe15434444087d082bc526b29"