Latest update to @blueprintjs/icons breaks static icon rendering when icons CSS not imported

[nerdstep]

When using only static icon imports from Blueprint there is no need to import blueprint-icons.css, however with the latest change the icons are broken if the CSS is not imported.

Environment

• Package version(s): core@5.8.0 | icons@5.6.0
• Operating System: Windows 10
• Browser name and version: Chrome 120

Code Sandbox

Steps to reproduce

1. Do not import blueprint-icons.css
2. Use a static icon, i.e. <Download size={16} />

Actual behavior

Icon is not rendered correctly due to missing transform-origin: center; styles.

Expected behavior

Expected the icons to render correctly without needing to import blueprint-icons.css

Possible solution

Move the new styles to blueprint.css

[adidahiya]

Thanks for pointing this out, I see that this might be a regression if you were not previously importing blueprint-icons.css. However, I don't think that was ever endorsed as a supported usage pattern for anyone rendering Blueprint icons, even if you're only using static components. The Getting started docs instruct users to import blueprint-icons.css.

We could fix the problem by moving these styles to @blueprintjs/core, but that breaches the abstraction layer of the @blueprintjs/icons module.

Are there any downsides to importing blueprint-icons.css in your app?

[nerdstep]

I think this issue probably raises some good questions with respect to Blueprint usage given the new static icon import paradigm.

We stopped importing blueprint-icons.css to avoid including the icon font in our bundle. I suppose one could argue that it doesn't matter if it's never used, but generally speaking I think it's best practice to only bundle resources that are actually used.

As for breaching the abstraction layer of the icons module, it seems to me that @blueprintjs/core already does this by specifying all of the font icon classes:

.bp5-icon-add::before{
  content:"\f109";
}

...

I do wonder why this exists in the core CSS file instead of the icons CSS? It adds quite a bit of extra heft to our CSS file that is never used.

[adidahiya]

As for breaching the abstraction layer of the icons module, it seems to me that @blueprintjs/core already does this by specifying all of the font icon classes

That's a good point... we should move that CSS to @blueprintjs/icons and keep all the icon font related code in one place which can be optionally loaded. I suppose that was not considered carefully in #4505 and #4513 - the focus of those PRs was on JS bundle modularity rather than CSS modularity.

To be extra safe, I think the change to move the icon font CSS to @blueprintjs/icons should happen in v6.0.

In the meantime, I can fix your reported regression by moving the .bp5-icon-svg path { transform-origin: center } style to @blueprintjs/core as you suggested. I'll go ahead and create a PR and push a patch for that fix.

[nerdstep]

we should move that CSS to @blueprintjs/icons and keep all the icon font related code in one place which can be optionally loaded

Yep sounds like a good change and makes sense to do it in a new major version.

[adidahiya]

I ended up with a different solution which reverts to applying transformOrigin directly in the static icon component template, see #6639