Having a class in a generic place is useful to provide information about the state of the UI to different stylesheets.
This is especially useful when we have access in velocity to state info that changes the styles.
For a long time, the body node on XWiki pages have contained information about the state of panels (.hideright, .hideleft), drawer position (.drawer) and the current Skin (.flamingo-skin).
I think it’s an appropriate solution to a few situations, and I personally used it in some more recent developments: panel-left-width-<X> and preference-underlining-only-inline-link. I plan to use this again to convey the “isAdvancedUser” info at https://github.com/xwiki/xwiki-platform/pull/5001 .
Note that another solution for those situations would be to use CSS variables to contain this info. This is a more powerful system, but it’s more fragile (any stylesheet can change the value to what it wants and influence other stylesheets…). IMO it’s out of the question as long as we support LESS stylesheets because they do not work well with CSS variables and there’s no easy parallel solution we could make with LESS variables.
Proposals
Set the <body> classes as API.
Add the advanced-user class to the <body>
Details
This entails updating the documentation. I’d take time to take care of it of course.
This means that any change forward on the classes available on the body object should be proposed/voted and documented.
We should eventually add automated tests to cover that those classes are available (at the very least for a standard use-case).
Explanation
This is a very stable implementation. I’d rather have custom stylesheets rely on these than anything else. Without an API, we end up having to repeat this state information in the velocity templates for every component that needs it, which can be innefficient computationally (e.g. check edit rights a few times through the templates for the page, one should be enough to provide enough info for CSS to work as expected). In addition, it clutters the code and generated HTML.
This would be the first HTML API as far as I know (it’s a bit different from indications found on front end components docs). We can use the experience on this to see what works/what are the limits of doing this.
Here’s my +1 for proposals 1. and 2. @mleduc What do you think of this (especially no.1)? How do you do those “conditional styles” in Cristal?
Thank you for your interest in the topic!
Lucas C.
Right now, the template results in two different HTML depending on the “advanced user” state. CSS is powerful enough today to factorize them into one HTML template that is partially displayed. I believe it’s a similar case for a lot of templates you can find at: Code search results · GitHub and it could be useful there.
I didn’t provide much argumentation about this second item in my initial post.
It would become useful for any complex feature that has a different UI for advanced users.
From what I know and somewhat confirmed by the query I shared above, this is something quite frequent in our codebase already, and I don’t see a reason why we’d stop.
In this example, we could improve code quality and migrate the older examples to this, but it’s not a priority at all compared to some other quality improvements .
In https://github.com/xwiki/xwiki-platform/pull/5001 it actually makes implementation of the UI for the attachment status easier/cleaner. Without it, I’d have to pretty much add the same class at the level of the attachment status list container, OR add a few paths in my template to change the resulting HTML depending on whether the user is advanced or not. IMO this use by itself is not itself to warrant adding this class at the body level, but it appears it could be even more useful so I wanted to make something reusable.
TLDR it’s used once in the PR, it can be used more given we have the time to migrate existing UIs and it could be used more in new UIs.
On the example of “advanced user”, it barely does, since we store the value in a velocity variable, we don’t have to compute it multiple times. The information for the status of a page are rarely on the page itself, for most of them we have to query admin preferences or user profiles, which is somewhat costly. E.g. Code search results · GitHub
This optimization point could be solved similarly if we enforce a “velocity global variables” API.
I agree we need to put some context information in the HTML in order for various parts of the UI to use it. And obviously, the closer we put this information to the root of the HTML the more UI elements will be able to use it. But using CSS classes is not always the best option. In fact, most of the context information is currently put on the root element using data- attributes:
I think the rule we followed so far was to: use a CSS class on the body element for purely UI context state, and a data- attribute on the root element for context information is not strictly UI related.
-0. I don’t agree that all the CSS classes (or attributes) added to the body element should be public API. We may have XWiki extensions that need to add a CSS class (or attribute) to the body element for their internal behavior, that is not meant to be used by other modules / extensions. There should be a way to say “this CSS class / attribute is internal API, even if it is set on the body element”.
-1. I don’t think this context information is strictly UI related. I would rather put this on the root element with something like data-xwiki-user-advanced="true", and of course make it available in meta.js for any JavaScript code to be able to use it.
The idea was that those in XS would be stable and should be documented. Extensions and customizations could still use it freely, but any use of it in XS would need to be documented.
So any CSS class added by a bundled XS extension is public and those added by installed (contributed) extensions are private? How does someone developing an XWiki extension know which CSS classes from the BODY tag are from XS (public) and which are from some installed extension (private)? How can an extension make their added BODY class public? XS is a collection of extensions, that evolves over time. Does it mean that when a new extension is bundled their BODY CSS classes become public automatically, and when an extension is removed their BODY CSS classes become private?
I find it cleaner to define a naming convention for public / private CSS classes.
AFAIU that makes them API already, doesn’t it? So maybe we need to review this page to ensure we’re ok with what’s documented and make it more clear that it’s an API.
.