Proposal: Make the body classes API

Development
1

Hello!

Context

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 .

image
image1847×989 169 KB

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

  1. Set the <body> classes as API.
  2. 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.

2

Thanks for the proposal @CharpentierLucas!

Yes, existing classes and attributes of the body element are already APIs, even if probably implicit. And should be documented if not already.

I tend to be -0 based on the yagni (you aren’t gonna need it) principle.
Your main argument is that it could simplify a trivial task.

I’d be very surprised if computing this value has a measurable performance impact.

3

Note that if that’s implemented, it could simplify the existing implementation of the edit button template:

image
image2116×1299 366 KB

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 :slight_smile: .

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.