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 Icons! #2028

Merged
merged 50 commits into from
Feb 1, 2018
Merged
Show file tree
Hide file tree
Changes from 45 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
414032e
add svgo to node-build-scripts
Jan 23, 2018
dd72980
add type information and @ts-check to generate-icons-source
Jan 23, 2018
a26afdb
iconContents.tsx contains `export const ICON_NAME = <svg>...</svg>` f…
Jan 23, 2018
85e41b3
generate and export IconSvgs
Jan 23, 2018
50c460b
refactor existing Icon component to render SVGs
Jan 23, 2018
3b2ff4b
fix usages (no more iconSize)
Jan 24, 2018
36159e8
svg.pt-icon style fixes, LegacyIconName extends IconName
Jan 24, 2018
ea5fbf8
revert documentalist usage
Jan 24, 2018
739e643
svgo v1 (0.7 is still used by css-loader :sad:)
Jan 24, 2018
87e8f09
iconSvgs => iconSvgPaths, map of icon name to array of path strings (…
Jan 24, 2018
eb79ca2
update DocsIcon
Jan 24, 2018
e242f50
remove style tag from endorsed icon (the only one that had it)
Jan 24, 2018
a82d096
fix tree test
Jan 24, 2018
718ff50
IconSvgPaths16 and IconSvgPaths20 objects
Jan 24, 2018
5e042e5
[WIP] choose 16 or 20 based on iconSize + example
Jan 24, 2018
a0f70da
back to iconSize
Jan 24, 2018
fe13ed7
one more, InputGroup needs a css fix
Jan 24, 2018
9ef12bb
requires, remove tslints
Jan 24, 2018
2fd23d6
iconSize <= 16 ? use 16 : use 20
Jan 24, 2018
f99ee70
color + style props, spread all svg props
Jan 24, 2018
fc5dd11
fix React prop warnings, remove invalid clipRule (not inside clipPath)
Jan 24, 2018
3f900a7
fix icons in input groups - padding only!
Jan 24, 2018
99b11e9
improve IconExample with IconSuggest
Jan 24, 2018
fb3a8fc
scripts: dev:core watches labs, icons clean rm's src/generated
Jan 24, 2018
84dc720
Merge branch 'develop' of github.com:palantir/blueprint into gg/svg-i…
Jan 26, 2018
dd37942
Button uses Icon instead of class, Dialog close button Icon
Jan 26, 2018
063a648
IconSuggest new syntax
Jan 26, 2018
a9feff4
MenuItem renders Icon instead of class
Jan 26, 2018
728288a
magic negative margin aligns SVG and DOM
Jan 27, 2018
647d608
fix a bunch of tests (one offending Popover test)
Jan 27, 2018
1089e6e
fix that popover test
Jan 27, 2018
9ef3861
Merge branch 'develop' of github.com:palantir/blueprint into gg/svg-i…
Jan 31, 2018
5049f57
comments
Jan 31, 2018
de0df31
Merge branch 'develop' into gg/svg-icons
adidahiya Jan 31, 2018
84ef214
Merge branch 'develop' of github.com:palantir/blueprint into gg/svg-i…
Jan 31, 2018
394df5c
revert popoverTests, delete iconSize=inherit test
Jan 31, 2018
0d2c391
ensure itemPredicate has default value in QueryList. fixes Icon compo…
Jan 31, 2018
08bb074
Merge branch 'develop' of github.com:palantir/blueprint into gg/svg-i…
Feb 1, 2018
e10ea0c
fix tree example
Feb 1, 2018
1069282
move icon class to data-icon attribute
Feb 1, 2018
e47e2a6
update docs
Feb 1, 2018
daa8c56
put short IconName in data-icon attribute
Feb 1, 2018
de3013b
fix tests
Feb 1, 2018
ab45534
fix Icon props spread
Feb 1, 2018
a2ab952
examples/core-examples/common/IconSelect
Feb 1, 2018
8fa9b92
docs language, normalizedIconName
Feb 1, 2018
3ddec4c
fix MenuItem icon color and dark submenu shadow
Feb 1, 2018
0115861
fix input-group button icon size
Feb 1, 2018
b871252
Callout renders SVG Icon component (#2060)
giladgray Feb 1, 2018
2a8caae
clarify callout icon logic
Feb 1, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"copy-landing-app": "cp -rf packages/landing-app/dist/* docs/.",
"deploy": "gh-pages -d docs -b master",
"dev:all": "lerna run dev --parallel --scope '!@blueprintjs/{landing-app,table-dev-app}'",
"dev:core": "lerna run dev --parallel --scope '@blueprintjs/{core,docs-app}'",
"dev:core": "lerna run dev --parallel --scope '@blueprintjs/{core,icons,docs-app}'",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change is unnecessary. we rarely work on icons source. you can run lerna run dev --parallel --scope '@blueprintjs/icons' in a separate console.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in my experience, this change has been very helpful. especially cuz data files come from icons package but component comes from core. all other dev aliases include package deps, so what's the harm here?

"dev:docs": "lerna run dev --parallel --scope '@blueprintjs/{docs-app,docs-theme}'",
"dev:datetime": "lerna run dev --parallel --scope '@blueprintjs/{core,datetime,docs-app}'",
"dev:labs": "lerna run dev --parallel --scope '@blueprintjs/{core,labs,select,docs-app}'",
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/components/alert/alert.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export class Alert extends AbstractPureComponent<IAlertProps, {}> {
return (
<Dialog className={classNames(Classes.ALERT, className)} isOpen={isOpen} style={style}>
<div className={Classes.ALERT_BODY}>
<Icon iconName={iconName} iconSize="inherit" intent={Intent.DANGER} />
<Icon iconName={iconName} iconSize={40} intent={Intent.DANGER} />
<div className={Classes.ALERT_CONTENTS}>{children}</div>
</div>
<div className={Classes.ALERT_FOOTER}>
Expand Down
8 changes: 8 additions & 0 deletions packages/core/src/components/button/_button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,14 @@ Styleguide pt-button
color: $pt-icon-color;
}

// icon-only button
.pt-icon:first-child:last-child {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎩

// center icon horizontally. this works for large buttons too.
$icon-margin-offset: -($pt-button-height - $pt-icon-size-standard) / 2;
margin-right: $icon-margin-offset;
margin-left: $icon-margin-offset;
}

/*
Advanced icon usage

Expand Down
18 changes: 10 additions & 8 deletions packages/core/src/components/button/abstractButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ export abstract class AbstractButton<T> extends React.Component<React.HTMLProps<
[Classes.DISABLED]: disabled,
[Classes.LOADING]: this.props.loading,
},
Classes.iconClass(this.props.iconName),
Classes.intentClass(this.props.intent),
this.props.className,
);
Expand Down Expand Up @@ -114,7 +113,7 @@ export abstract class AbstractButton<T> extends React.Component<React.HTMLProps<
};

protected renderChildren(): React.ReactNode {
const { loading, rightIconName, text } = this.props;
const { iconName, loading, rightIconName, text } = this.props;

const children = React.Children.map(this.props.children, (child, index) => {
if (child === "") {
Expand All @@ -127,12 +126,15 @@ export abstract class AbstractButton<T> extends React.Component<React.HTMLProps<
return child;
});

return [
loading ? <Spinner className="pt-small pt-button-spinner" key="spinner" /> : undefined,
text != null ? <span key="text">{text}</span> : undefined,
...children,
<Icon className={Classes.ALIGN_RIGHT} iconName={rightIconName} key="icon" />,
];
return (
<>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so. nice.

<Icon iconName={iconName} />
{loading && <Spinner className="pt-small pt-button-spinner" />}
{text != null && <span>{text}</span>}
{children}
<Icon className={Classes.ALIGN_RIGHT} iconName={rightIconName} />
</>
);
}
}

Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/components/dialog/_dialog.scss
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ $dialog-padding: $pt-grid-size * 2 !default;
min-height: $pt-icon-size-large + $dialog-padding;
padding-left: $dialog-padding;

> .pt-icon,
.pt-icon-large {
flex: 0 0 auto;
margin-right: $dialog-padding / 2;
Expand Down Expand Up @@ -143,6 +144,7 @@ $dialog-padding: $pt-grid-size * 2 !default;

cursor: pointer;
padding: $pt-grid-size;
padding-right: $dialog-padding;
}

.pt-dialog-body {
Expand Down
11 changes: 7 additions & 4 deletions packages/core/src/components/dialog/dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,14 @@ export class Dialog extends AbstractPureComponent<IDialogProps, {}> {
}

private maybeRenderCloseButton() {
// for now, show close button if prop is undefined or null
// show close button if prop is undefined or null
// this gives us a behavior as if the default value were `true`
if (this.props.isCloseButtonShown !== false) {
const classes = classNames(Classes.DIALOG_CLOSE_BUTTON, Classes.iconClass("small-cross"));
return <button aria-label="Close" className={classes} onClick={this.props.onClose} />;
return (
<button aria-label="Close" className={Classes.DIALOG_CLOSE_BUTTON} onClick={this.props.onClose}>
<Icon iconName="small-cross" iconSize={Icon.SIZE_LARGE} />
</button>
);
} else {
return undefined;
}
Expand All @@ -112,7 +115,7 @@ export class Dialog extends AbstractPureComponent<IDialogProps, {}> {
}
return (
<div className={Classes.DIALOG_HEADER}>
<Icon iconName={iconName} iconSize={20} />
<Icon iconName={iconName} iconSize={Icon.SIZE_LARGE} />
<h5>{title}</h5>
{this.maybeRenderCloseButton()}
</div>
Expand Down
6 changes: 2 additions & 4 deletions packages/core/src/components/forms/_input-group.scss
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,7 @@ $input-button-height-large: $pt-button-height !default;

// bump icon up so it sits above input
z-index: 1;
margin: 0 ($pt-input-height - $pt-icon-size-standard) / 2;
line-height: $pt-input-height;
margin: ($pt-input-height - $pt-icon-size-standard) / 2;
color: $pt-icon-color;
}

Expand Down Expand Up @@ -145,8 +144,7 @@ $input-button-height-large: $pt-button-height !default;
}

.pt-icon {
margin: 0 ($pt-input-height-large - $pt-icon-size-standard) / 2;
line-height: $pt-input-height-large;
margin: ($pt-input-height-large - $pt-icon-size-standard) / 2;
}

.pt-input {
Expand Down
10 changes: 8 additions & 2 deletions packages/core/src/components/forms/_label.scss
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ label.pt-label {
margin: 0 0 ($pt-grid-size * 1.5);

.pt-input,
.pt-select {
.pt-select,
.pt-popover-wrapper {
display: block;
margin-top: $pt-grid-size / 2;
text-transform: none;
Expand All @@ -49,7 +50,8 @@ label.pt-label {

.pt-input,
.pt-input-group,
.pt-select {
.pt-select,
.pt-popover-wrapper {
display: inline-block;
margin: 0 0 0 ($pt-grid-size / 2);
vertical-align: top;
Expand All @@ -64,6 +66,10 @@ label.pt-label {
}
}

&:not(.pt-inline) .pt-popover-target {
display: block;
}

/*
Disabled labels

Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/components/forms/inputGroup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export class InputGroup extends React.PureComponent<HTMLInputProps & IInputGroup

return (
<div className={classes}>
<Icon iconName={leftIconName} iconSize="inherit" />
<Icon iconName={leftIconName} />
<input
type="text"
{...removeNonHTMLProps(this.props)}
Expand Down
11 changes: 11 additions & 0 deletions packages/core/src/components/icon/_icons.scss
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,14 @@ span.pt-icon {
content: $content;
}
}

svg.pt-icon {
// respect dimensions exactly
flex: 0 0 auto;
// SVG and DOM elements don't align perfectly - this magic number seems to fix it.
// https://blog.prototypr.io/align-svg-icons-to-text-and-say-goodbye-to-font-icons-d44b3d7b26b4#6e0c
margin-top: -0.125em;
vertical-align: middle;
// inherit text color by default
fill: currentColor;
}
72 changes: 49 additions & 23 deletions packages/core/src/components/icon/icon.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,62 @@
See [**Icons**](#icons) for a searchable list of all available UI icons.
</div>

<div class="pt-callout pt-intent-primary pt-icon-info-sign">
<h5>SVG icons in 2.0</h5>
Blueprint 2.0 introduced SVG icon support throughout the ecosystem, and moved icon resources to a separate
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: no need for the comma here (I noticed your prose style does this a lot and I generally don't mind for code comments, but I'm more picky about it in actual documentation).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, "throughout the ecosystem" is a bit of a stretch. I would remove that phrase entirely.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sometimes, i use commas to indicate where to pause, for maximal dramatic effect. 🎭

__@blueprintjs/icons__ package. The `Icon` component now renders SVG paths and the icon classes are no longer
used by any Blueprint React component. Icon font support has been preserved but should be considered legacy,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"... but should be considered a legacy feature that will be removed in a future major version."

and we plan to remove the icon fonts in a future major version.
</div>

This section describes two ways of using the UI icon font: via CSS or via React component.

Many Blueprint components provide an `iconName` prop, which supports both the
full name `pt-icon-projects` and the short name `projects`.

@reactExample IconExample

@## JavaScript API

The `Icon` component is available in the __@blueprintjs/core__ package.
Make sure to review the [general usage docs for JS components](#blueprint.usage).

Use the `<Icon>` component to easily render __SVG icons__ in React. The `iconName` prop is typed
such that editors can offer autocomplete for known icon names. The optional `iconSize` prop ensures
you'll never forget a sizing class and clarifies the expected width and height of the icon element.
The component also accepts all valid HTML props for an `<svg>` element.

Data files in the __@blueprintjs/icons__ package provide SVG path information for Blueprint's 300+ icons
for 16px and 20px grids. The `iconName` prop dictates which SVG is rendered, and `iconSize` determines
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another unnecessary comma here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i actually thought long and hard about this one

which pixel grid is used: `iconSize >= 20` will use the 20px grid and smaller icons will use the 16px grid.

```tsx
// string literals are supported through IconName union type
<Icon iconName="cross" />
<Icon iconName="pt-icon-globe" iconSize={20} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably shouldn't encourage the long names with pt- if you want to eventually remove them

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

follow-up PR. still technically supported.

<Icon iconName="graph" iconSize={40} intent={Intent.PRIMARY} />

// can also use IconClasses string enum and Icon.SIZE_* constants
import { IconClasses } from "@blueprintjs/core";
<Icon iconName={IconClasses.ALIGN_LEFT} iconSize={Icon.SIZE_LARGE} />

// can pass all valid HTML props
<Icon iconName="add" onClick={this.handleAdd} onKeyDown={this.handleAddKeys}>
```

@interface IIconProps

@## CSS API

<div class="pt-callout pt-intent-warning pt-icon-warning-sign">
<h5>Icon fonts are legacy in 2.0</h5>
Blueprint's icon fonts should be considered legacy, and we plan to remove them in a future major version.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use the same phrasing I suggested above

The SVGs rendered by the React component do not suffer from the blurriness of icon fonts, and browser
support is equivalent.
</div>

The CSS-only icons API uses the __icon fonts__ from the __@blueprintjs/icons__ package.

To use Blueprint UI icons via CSS, you must apply two classes to a `<span>` element:
- a __sizing class__, either `pt-icon-standard` (16px) or `pt-icon-large` (20px)
- an __icon name class__, such as `pt-icon-projects`
Expand All @@ -30,26 +79,3 @@ Icon classes also support the four `.pt-intent-*` modifiers to color the image.
necessary, set a `font-size` that is whole multiple of 16 or 20 with the relevant size class.
You can instead use the class `pt-icon` to make the icon inherit its size from surrounding text.
</div>

@## JavaScript API

Use the `<Icon>` component to easily render icons in React. The required `iconName` prop is typed
such that editors can offer autocomplete for known icon names. The optional `iconSize` prop ensures
you'll never forget a sizing class and clarifies the expected width and height of the icon element.
The component also accepts all valid HTML props for a `<span>` element.

```tsx
// string literals are supported through IconName union type
<Icon iconName="cross" />
<Icon iconName="pt-icon-globe" iconSize="inherit" />
<Icon iconName="graph" iconSize={20} intent={Intent.PRIMARY} />

// can also use IconClasses string enum and Icon.SIZE_* constants
import { IconClasses } from "@blueprintjs/core";
<Icon iconName={IconClasses.ALIGN_LEFT} iconSize={Icon.SIZE_LARGE} />

// can pass all valid HTML props
<Icon iconName="add" onClick={this.handleAdd} onKeyDown={this.handleAddKeys}>
```

@interface IIconProps
75 changes: 44 additions & 31 deletions packages/core/src/components/icon/icon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,60 +7,73 @@
import * as classNames from "classnames";
import * as React from "react";

import { IconName } from "@blueprintjs/icons";
import { IconName, IconSvgPaths16, IconSvgPaths20, LegacyIconName } from "@blueprintjs/icons";
import { Classes, IIntentProps, IProps } from "../../common";

export { IconName };

export interface IIconProps extends IIntentProps, IProps {
/**
* Color of icon. Equivalent to setting CSS `fill` property.
*/
color?: string;

/**
* Name of the icon (with or without `"pt-icon-"` prefix).
* If `undefined`, this component will render nothing.
* If omitted or `undefined`, this component will render nothing.
*/
iconName: IconName | undefined;
iconName?: LegacyIconName;

/**
* Size of the icon.
* Blueprint provides each icon in two sizes: 16px and 20px. The keyword `"inherit"` will
* render a 20px icon but inherit `font-size` from its parent.
* Constants are exposed for each of these values on the component itself:
* `Icon.SIZE_(STANDARD|LARGE|INHERIT)`,
* @default 16
* Size of the icon, in pixels.
* Blueprint contains 16px and 20px SVG icon images,
* and chooses the appropriate resolution based on this prop.
* @default Icon.SIZE_STANDARD = 16
*/
iconSize?: 16 | 20 | "inherit";
iconSize?: number;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no default?


/** CSS style properties. */
style?: React.CSSProperties;
}

export class Icon extends React.PureComponent<IIconProps & React.HTMLAttributes<HTMLSpanElement>, never> {
export class Icon extends React.PureComponent<IIconProps & React.SVGAttributes<SVGElement>> {
public static displayName = "Blueprint2.Icon";

public static readonly SIZE_STANDARD = 16 as 16;
public static readonly SIZE_LARGE = 20 as 20;
public static readonly SIZE_INHERIT = "inherit" as "inherit";
public static readonly SIZE_STANDARD = 16;
public static readonly SIZE_LARGE = 20;

public render() {
if (this.props.iconName == null) {
return null;
}
const { className, iconName, intent, iconSize = Icon.SIZE_STANDARD, ...restProps } = this.props;
const { className, iconName: iconNameProp, iconSize = Icon.SIZE_STANDARD, intent, ...svgProps } = this.props;
const iconName = iconNameProp.replace("pt-icon-", "") as IconName;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: leave the prop as iconName (also do this destructuring on the first line of the function, so you can use it on L46) and make this variable named normalizedIconName


const classes = classNames(
getSizeClass(iconSize),
Classes.iconClass(iconName),
Classes.intentClass(intent),
className,
// choose which pixel grid is most appropriate for given icon size
const pixelGridSize = iconSize >= Icon.SIZE_LARGE ? Icon.SIZE_LARGE : Icon.SIZE_STANDARD;
const classes = classNames(Classes.ICON, Classes.intentClass(intent), className);
const viewBox = `0 0 ${pixelGridSize} ${pixelGridSize}`;
return (
<svg
{...svgProps}
className={classes}
data-icon={iconName}
width={iconSize}
height={iconSize}
viewBox={viewBox}
>
<title>{iconName}</title>
{this.renderSvgPaths(pixelGridSize, iconName)}
</svg>
);
return <span className={classes} {...restProps} />;
}
}

// NOTE: not using a type alias here so the full union will appear in the interface docs
function getSizeClass(size: 16 | 20 | "inherit") {
switch (size) {
case Icon.SIZE_STANDARD:
return Classes.ICON_STANDARD;
case Icon.SIZE_LARGE:
return Classes.ICON_LARGE;
default:
return Classes.ICON;
private renderSvgPaths(pathsSize: number, iconName: IconName) {
const svgPathsRecord = pathsSize === Icon.SIZE_STANDARD ? IconSvgPaths16 : IconSvgPaths20;
const pathStrings = svgPathsRecord[iconName];
if (pathStrings == null) {
return null;
}
return pathStrings.map((d, i) => <path key={i} d={d} fillRule="evenodd" />);
}
}
Loading