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

[Icon] unknown string icon name renders blank icon #2928

Merged
merged 2 commits into from Sep 12, 2018
Merged

Conversation

giladgray
Copy link
Contributor

Fixes #2925

Changes proposed in this pull request:

  • unknown icon name renders markup but no path elements, resulting in blank icon.

@blueprint-bot
Copy link

unknown string icon name renders blank icon

Previews: documentation | landing | table

Copy link
Contributor

@zerovox zerovox left a comment

Choose a reason for hiding this comment

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

Thank you :D

Copy link
Contributor

@themadcreator themadcreator left a comment

Choose a reason for hiding this comment

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

comment about wording, otherwise cool

const paths = this.renderSvgPaths(pixelGridSize, icon);
if (paths == null) {
return null;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

so now it will always have an empty <svg> instead of nothing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if you say <Icon icon="blah" /> then yes. <Icon icon={null} /> renders null.

* nothing.
* - If given an `IconName` (a string literal union of all icon names), that
* icon will be rendered as an `<svg>` with `<path>` tags. Unknown strings
* will render a blank icon.
Copy link
Contributor

Choose a reason for hiding this comment

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

is a "blank icon" different than an empty <svg>? if so, how? if not, how is it an "icon" at all?

the wording makes it sound like there is some sort of stand in icon for a "blank" situation, kind of like the broken <img> graphic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image
😂

@blueprint-bot
Copy link

Update icon.tsx

Previews: documentation | landing | table

@giladgray giladgray merged commit 4a40332 into develop Sep 12, 2018
@giladgray giladgray deleted the gg/icon-blank branch September 12, 2018 22:12
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

Successfully merging this pull request may close these issues.

None yet

4 participants