Add stylesheet configuration to widgets

[hubsif]

Hi @ghys,

I addressed something I think many powerusers have requested for: custom css stylesheets.

This first draft allows to define a stylesheet for a page, like this:

config:
  layoutType: responsive
  label: stylesheet_test
  sidebar: true
  stylesheet: |
    .myoneclass {
      background-color: red;
    }
    .myotherclass {
      font-size: 40px;
    }
blocks:
...

The stylesheet gets added globally on pageIn and removed on pageOut (it's not scoped to the page, so it can generally also affect things like the panel, but I think powerusers can deal with that).

Please let me know if something like this is desired and if I should complete it (e.g. add to layouting). Please also share if a stylesheet per page is a good approach in your view.
Some users might also want to add a global stylesheet, so it affects all pages at once. I'm also planning on adding this (if ok by you), though the approach will definitely be different (requires a global config or a static include or similar).

[relativeci]

Job #52: Bundle Size — 10.9MB (0%).

dab56cb vs dab56cb

Changed metrics (1/8)

Metric	Current	Baseline
Cache Invalidation	0.18%	20.86%

Changed assets by type (0/7)

No changes

---

View Job #52 report on app.relative-ci.com

[ghys]

I'm not opposed to it.

To be complete, normally I'm rather wary of giving so much latitude for security reasons and the fact that you could completely break the UI, but for stylesheets I'm not opposed to this approach as I believe it's less of a risk than allowing inline JavaScript (#626).
Still, it could be exploited to deceive and manipulate users, if abused. I believe it would still be wise to add a well-crafted Content Security Policy (like in HABPanel) to at least prevent "@importing" CSS from external files - i.e. all stylesheets should either be inline or loaded from the same origin i.e. server-relative URLs such as /static/... - that is including frameworks like Tailwind or Bootstrap, so they would have to be copied over locally and not loaded from a CDN. This gives the admin a chance to review what they're including and rest assured that the remote stylesheet will not change and silently include malicious code without notice.

I get the argument that adding the ability to override global styles is a powerful feature, but maybe it's also a bit dangerous. Scoping to the page would ensure that the sidebar remains unaffected and predictable so it's possible to bail out of a page with broken stylesheet overrides. I'm ready to be convinced that there are legitimate use cases where it would not be desired. On the contrary, it could be argued that it would be desirable to have the ability to add scoped stylesheets to every "root" UI component -not only pages but also widgets; so you could define classes that you use inside your widget that wouldn't "spill" to other widgets.

For adding global CSS we don't have a standard way of loading options for the entire UI, as I didn't find a need for them yet and it spares one additional request to the API before it can be rendered. I'm open to all ideas!

[hubsif]

On the contrary, it could be argued that it would be desirable to have the ability to add scoped stylesheets to every "root" UI component -not only pages but also widgets

I'm glad you said that, since the idea is actually as follows:
Extend general widget (widget-mixin maybe) and page configuration to support stylesheet definitions like above. These then get scoped (by some css scoping library) and are added when mounted and removed when unmounted.

This would allow for e.g. custom widgets to define their own stylesheet, which I think could drastically reduce repetitions and make them cleaner. And due to scoping they're limited to their component/widget. Being part of the widget YAML they're also included when shared.

As for security, I think a Content-Security-Policy is a simple and effective way to limit resources to the openHAB instance. I even don't think external imports are actually required and @import statements could simply be prohibitted.

Regarding global CSS my current suggestion would be to follow the simplest approach and have a predefined static style include added to the app, e.g. /static/ui_custom.css. If users want to add global css they can create that file and add their stylings.

[hubsif]

So, originally I wanted to wait for your ok, but couldn't wait trying 😉.

I "moved" the stylesheet configuration to the widget mixin and added scoping. Surprisingly, this works smoothly for all I've tested so far, also in layout-edit and widget developer (latest at redraw). Though, it surely requires some more, thorough testing.
Also, I limited CSS to 'self' by extending the CSP.

Simple custom widget sample:

uid: label_card_stylesheet
component: f7-card
config:
  stylesheet: |
    .testclass {
      color: red;
    }
slots:
  content:
    - component: Label
      config:
        class: testclass
        text: Test

When I first attended to adding this feature, I didn't think it was actually that simple to implement 😄.

[ghys]

Works pretty well!
Tested with this example:

component: f7-card
config:
  style:
    display: flex
    justify-content: center
  stylesheet: >
    .mylink {
      color: blue !important;
    } .mylink:hover {
      text-decoration: underline !important;
      transform: scale(1.5);
      transition: 0.3s all;
    }
slots:
  default:
    - component: oh-link
      config:
        text: Test
        class:
          - margin
          - mylink

A small bug, the stylesheet doesn't get initialized when you enter the widget editor; until you force a redraw or change the split orientation. On pages it works normally.
This seems to be fixed by setting ready to false (don't know why it is initialized to true) here:

openhab-webui/bundles/org.openhab.ui/web/src/pages/developer/widgets/widget-edit.vue

Line 123 in e9e5506

ready: true,

[ghys]

• this.css_uid should be initialized (to null) in data () and also be camelCase instead of snake_case (i.e. cssUid).
• to generate a random ID this.$f7.utils.id() is used throughout the app, maybe use that here as well?

[hubsif]

• right, sorry, been sick last week (is that a valid excuse for all these "consistency faults"? 😆 )
• hm, I should have looked deeper into this. I thought that id() generates a uuid, which I thought is unnecessarily long in this case (well, eventually it wouldn't have mattered either). Checking the docs I now realize that by default it creates a 10 chars hex hash, which is about the same. So, will adjust, of course!

[hubsif]

ah, and regarding data ():
I meanwhile read on multiple websites that it's not best practice to put variables in data() that don't need to be reactive, and that one should avoid simply initializing all "component variables" there. That's actually the reason I didn't put it there.
It's up to you how to handle this. Eventually, I think it doesn't hurt much, it just creates some getters and setters that are never called...

[hubsif]

• this.css_uid should be initialized (to null) in data () and also be camelCase instead of snake_case (i.e. cssUid).

• to generate a random ID this.$f7.utils.id() is used throughout the app, maybe use that here as well?

fixed. what about data () ?

[ghys]

what about data () ?

Your argument above seems fair enough to me.

[hubsif]

This seems to be fixed by setting ready to false (don't know why it is initialized to true) here:

Hm, git blame: 13 months ago ... ghys! 😜
Let me see if I can find a reason for this ...

I found another "blemish": At least when used in custom widgets, the stylesheet is applied as html attribute to the element. I don't think that breaks anything, as it should be ignored. Yet, it's an ugly spot, especially with longer stylesheets...

[hubsif]

I found another "blemish": At least when used in custom widgets, the stylesheet is applied as html attribute to the element. I don't think that breaks anything, as it should be ignored. Yet, it's an ugly spot, especially with longer stylesheets...

fixed.

This seems to be fixed by setting ready to false (don't know why it is initialized to true) here:

Did some basic tests and couldn't see a reason to init it to true. Actually, false seems much better suited, I adjusted it.

[ghys]

Nice addition, now rather safe to add with the scoping and CSP, and which I suspect will boost the creativity of widget authors, can't wait to see the results!

I'd rather leave that out of the widget definitions (so it's not documented nor displayed on config GUIs), but neither are visible, visibleTo, style, class etc.
It'd be nice to make all of these at least appear in the YAML editors' autocompletion where relevant, eventually. This could be done in one go and directly in the hints file for components.

It would be nice to amend the documentation article to mention the new stylesheet parameter, though.

[altaroca]

I just noticed that the stylesheet parameter doesn't work on the floor plan page. After some investigation I think there is an error on this line:
this.$el.classList.add(this.cssUid)
If classList is empty this will throw. It seems that vue optimizes components such that v-if results in an empty comment if not true. In the case of floor plans that leads to this.$el being just such a comment and classList being empty.
I do not have a good solution since it is difficult to find the widget's root element in this situation. Maybe scan the dom tree upwards to a known class? Thanks for your support.