Latest update to @blueprintjs/icons breaks static icon rendering when icons CSS not imported about blueprint HOT 5 CLOSED

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.

from blueprint.

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

from blueprint.

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?

from blueprint.

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.

from blueprint.

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.

from blueprint.