Allow the style tag inside the template tag when the latter is used by a custom HTML tag / Web Component

Hi everyone,

In https://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/XhtmlCssCodeStyle/ we have this rule:

don’t use inline style declarations

Although it’s not very clear, I assume this includes both:

  • using the style attribute, e.g. style="color: red"
  • using the style tag, e.g. <style>p {color: red}</style>

I think it’s a good rule, but I’d like to add an exception. When implementing custom HTML elements or Web Components, it’s convenient to use the <template> HTML tag to define the HTML structure. See Using templates and slots - Web APIs | MDN . If the custom HTML element / Web Component is simple, its CSS styles can be specified inside the same <template> element using the style tag. This resembles a bit how Vue components are defined for instance, see xwiki-platform/xwiki-platform-core/xwiki-platform-livedata/xwiki-platform-livedata-webjar/src/main/vue/footnotes/LivedataFootnotes.vue at master · xwiki/xwiki-platform · GitHub , keeping the component styles in the same place as the component HTML structure (if they are simple enough).

So I’d like to change the rule to:

don’t use inline style declarations (neither the style HTML attribute nor the <style> HTML tab), with one exception:

  • the <style> tag is a direct child of a <template> tag that is used to define a custom HTML element or Web Component

WDYT?

Thanks,
Marius

-0, I don’t have a strong opposition to this, but I don’t think it’s necessary.

IMO the reason why we have this rule is to make sure CSS and HTML are separated properly. This makes it a bit easier to debug CSS (less places a style can come from, you don’t need to search through .vm files for the wrong style…) and also makes your HTML shorter → easier to read.

IMO if the styles are simple enough, they should be attainable by a standard class we already have in our CSS. E.g. right now, styling most buttons is possible without any custom CSS but only using the right combination of classes. If something should be simple but there’s no standard class, this’d mean that we’re missing one and we can complete our styling system, factorizing style code in this standard class.


Sidenote: Until now I understood this rule was for the style attribute (personally I never really needed the style tag without web components).


Thanks for bringing this up, I hope we can find a solution that is convenient to write and maintain :slight_smile:
Lucas C.

Then, based on your interpretation, the 103 Vue templates we currently have don’t follow this rule.

Our best practice is to organize code by functionality / domain. If a module needs some custom CSS it needs to be put in that module, not in a single global skin module. We have of course generic styles that are shared by all modules and which go in the skin, but modules also need custom styles that have to be located in those modules.

Simple is not the same as generic (i.e. that can be shared between modules). Modules often have the need to make small adjustments to the CSS coming from the skin, but these styles, even if simple, are specific to those modules and thus should stay in those modules. I don’t believe it’s possible to rely only on the generic styles from the skin.

Maybe it helps to look at a concrete example. Suppose you want to implement a spinner (loading animation) Web Component that you could use like this:

<xwiki-spinner></xwiki-spinner> Saving...

The goal is to hide the implementation detail: whether this is implemented with an image / SVG / font icon doesn’t matter. This component will need some CSS. I think it’s best to keep that CSS very close to the code that defines that Web Component, not in the skin. Moreover, if that component uses a <template> HTML tag to define its structure, and the style is simple enough, I think it’s fine to keep the style inside the template:

<template id="xwiki-spinner-template">
  <style>
  .spinner {
    ...
  }
  </style>
  <span class="spinner"></span>
</template>

Thanks,
Marius

To me, there are two important aspects:

  1. The ability to customize styles. If we move styles into web components, in the shadow DOM, this basically removes the ability to customize those styles via a skin extension by overriding individual CSS rules.
  2. Limiting the network transfer that is done on every request. A CSS file can easily be cached by the browser (but ideally, it should be a few large CSS files, not many small ones). Putting the CSS code inline in the HTML means that it needs to be loaded with every request.

I don’t know how Live Data handles this currently.

For that part, I think the best practice is to link to external styles instead.
As long as the URL of the css is stable and decently cached, this should solve the issue.

Live Data is based on Vue, using Single-File components. A single-File component (SFC) has three sections: script, template and style.

  • script: the Javascript logic of the component (parameters, internal logic…)
  • template: a mix of html element, sub-components, and custom attributes (for conditional, loops, etc)
  • style: the css, that can be scoped (i.e., only applied on the elements of the current template, even a a {} selector will only be applied on links declared on the current element (see SFC CSS features for more details).

Then, the vite web bundles scans the set of SFC files to produce a set of optimized JavaScript and CSS files.
In addition to classic optimizations such as minification, or inlining. Vite also performs what they call chunking, which is splitting the code in logical groups of files that are:

  1. of a size optimized for the http protocol
  2. made to mix in the same files what needs to be done together (i.e., if two components are always used together, they’ll likely be grouped in the same js/css files)

Hi Michael,

True, but on the other hand, keeping everything public means we have to be very careful whenever we change the HTML structure or CSS, otherwise we break the extensions that have some expectations in that regard. Keeping everything public on the front-end side has lead to the current situation where it is very costly for us to change the design system or refactor the skin. We need to find the right balance, and relying on CSS variables, as you mentioned on the xwiki chat, is probably a good compromise.

Very good point. There is a disadvantage on first load, but afterward, once the CSS file is cached, it will be faster. There is a downside of merging / aggregating CSS files though: you lose the scoping, meaning that: when you write the CSS for your Web Component you need to be careful not to break other Web Components that are bundled together. Without a proper bundler that renames the CSS selectors to target only a given Web Component (making it harder to debug the result), we’ll have to prefix (namespace) all CSS classes / IDs used in a Web Component to avoid conflicts. Maybe it’s not much, but something to keep in mind.

So, to get back to the exception I wanted to add, it’s not clear, but I have the feeling @MichaelHamann and @mleduc don’t like it. Am I right?

Thanks,
Marius

Big +1, I’m in favor of allowing for a better control of how UI components can be customized.

This is not so clear. If you have 10 web components buttons, and they all duplicate the same style, the browser rendering engine will have to parse and render the same CSS 10 times.
If they all refer to the same CSS file, it will be loaded, parsed and cached once.
I’m not sure my mental model of how browsers internal works is correct. Just to say that the benefits and costs are not so clear.

So yes, so far I’m in favor of keeping the existing rule.