Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Amend best practices for CSS variables definitions in light of performance lessons learned #16660

Open
krassowski opened this issue Aug 9, 2024 · 0 comments

Comments

@krassowski
Copy link
Member

What I would ultimately like to do is to stop putting things into :root when they do not need to be there. This is because performance suffers doubly:

  • from browser having too many variables declared in a scope (this is a small cumulative effect)
  • from them being set on global scope when a local one would do (this is a problem especially in Chrome, see https://issuetracker.google.com/issues/40676722)

[...] these variables are used only once in the entire CSS codebase but have a non-obvious cost which only becomes apparent when you work with large notebooks and get into the weeds of browser performance profiling. I spent a lot of time on it with https://github.com/jupyterlab/ui-profiler and more low-level tools and saw the :root group of variables show up quite often as a potential culprit of UI freezes.

Originally posted by @krassowski in #16656 (comment)

[... ] having everything scoped out in :root is quite useful for collaborating with multiple people from different backgrounds because, from my perspective, it makes the CSS look like just a configuration file. Anyway, I agree with you.

[... ] an issue to discuss the reorganization of the variable files for the various themes. The RTC user specific colors and Vega extension styles variable sets can also be refactored, for example, in my perspective.

I would like also to suggest adding a note to the documentation about this organization (there is actually already a point about this, but I think it can be extended): critical variables must be scoped in :root, the rest in their respective parent classes (with one example).

Originally posted by @joaopalmeiro in #16656 (comment)

Yes, that makes sense. To clarify, I am not sure about moving anything out of the :root right now as that could break existing themes due to specificity, but for new additions which are tightly scoped to a specific component (like in this case) it seems that there is a risk of performance degradation if we keep adding things to :root but no historic constraints, so these are maybe better properly scoped.

Anyways, collecting some benchmarks to make sure we know at which points it becomes a problem, and updating the docs with suggestion to add things into scoped blocks as you mention for things like the completer, RTC user specific colors and Vega extension styles which you mention is a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants