CSS variable files should differentiate between variables and variables that load assets about theming-base-content HOT 8 CLOSED

As I described earlier, I don't see the problem on theming-base-content side but in the configuration of the bundler.
The css_variables.css is just a single CSS file, it does neither know nor care about the existence of bundlers and their attempt to inline assets.

I envision an improvement like this as part of a larger restructuring of the theming-content that makes it more easily consumable. Especially I think of something like

@import "@sap-theming/theming-content/sap_horizon.css";
/* or */
@import "@sap-theming/theming-content/sap_horizon-parameters.css";
@import "@sap-theming/theming-content/sap_horizon-assets.css";

instead of

 @import "@sap-theming/theming-content/content/Base/baseLib/sap_horizon/css_variables.css";

This is however just an early thought that is not even part of the current UI theme designer backlog.

from theming-base-content.

Hi Greg,

Sorry for the late reply.

Is this issue already solved for you by the linked change?

from theming-base-content.

@uwe-klinger It is a method for us to move forward, but ultimately it would be better coming from this library.

from theming-base-content.

Hi @greg-a-smith (cc @droshev)!

This issue is stale since nearly two years. I suspect your custom postcss plugin works good enough.

To me the whole issue reads like a misconfiguration of your bundler, which, I assume, is exactly what you target with your postcss plugin. The theming-base-content does not force the bundler to download (I assume to inline) the fonts. In my eyes you could get away with postcss-plugin-context or postcss-use and only apply your downloading/inlining to your own sources (or specifically not the theming-base-content sources).

As the issue is stale for so long, and as it should be solved in your bundler configuration anyways, I'll close it.

Best
Dominik

from theming-base-content.

@dominikschreiber this issue is/will be present to anyone who is using this library and trying to include the css-variable.css file. One solution is to offer this relative import as an optional file parallel to the css-variables one.

from theming-base-content.

@dominikschreiber that would be great. I hope we can see that after more than 2y of raising that issue

from theming-base-content.

The Luigi Framework relies on Fundamental Styles and @sap-themeing. With recent changes, we have to include the entire css_variable.css file as part of our framework, and it caused us to double-bundle size, which is extremely a lot.

Considering that we don't need all fonts from the css_variables.css, we would like to ask you to separate fonts and CSS variables into different files. Would that be possible?

from theming-base-content.

I've spent the morning verifying that my claim is correct that, if assets get inlined that you don't want to be inlined, the problem is not in the css_variables.css but in the bundler configuration, and wrote a blog post about it: How to import files from theming-base-content without inlining all urls using postcss. The gist:

// file: postcss.config.js
module.exports = {
  plugins: [
    require('postcss-import')(), // inlines @import statements
    require('postcss-url')([ // inlines url() statements
      {filter: 'things/you/want/to/be/inlined/**/*', url: 'inline'}, // but only here
      {filter: '**/content/Base/baseLib/**/*', url: ({url}) => url} // and not here
      // you could also omit the 2nd filter, or use it to rewrite the URLs to assets
      // in the theming-base-content to a CDN or wherever you serve them from
    ])
  ]
}
// I called that with `npx postcss src/main.css -o target/main.css` but YMMV

I understand that it would be nice to have the split out-of-the-box, so that you can import e.g. css_variables_only.css but not css_variables_urls.css. But this is neither a regression in the theming-base-content nor a problem of css_variables.css (which, again, does not know or care that bundlers exist and that you can misconfigure them).

Therefore we will prioritize this requirement against other requirements to the UI theme designer (the product theming-base-content is part of) and see if or when we will be able to deliver such a split.

from theming-base-content.