Skip to content

[ENH]: Hard-code dynamic Tailwind util classes (improve ToC styling) #4307

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

hesreallyhim
Copy link
Contributor

Description of changes

Fixes #4129

  • The doc-site codebase uses Tailwind CSS utility classes (ex. .pl-2, .pl-3 => padding-left: 0.5rem;, padding-left: 0.75rem;, etc.), which provide short-hand, computed classes from Tailwind (very handy).
  • Tailwind ships an optimized production bundle by doing static analysis and only including style declarations for classes (including utility classes) that it finds in the code.
  • So, trying to use a utility class that is dynamically constructed at runtime via props or something doesn't (usually) work, because if you write, e.g., .pl-${props.item.number}, it doesn't know which .pl-<VALUE> classes it has to include. (Caveat: Sometimes you get away with it because elsewhere in the code someone wrote .pl-4 so, by accident, if your dynamic class is .pl-4 at runtime, then it will find the style declarations for it, and all is well.)
  • Long story short, you can't rely on dynamic utility classes (see Issue for (even) more long-winded explanation).
  • This appears a few times in the codebase, and probably it's not a severe issue. The case that brought it to my attention is the Table of Contents, where I noticed that sub-sections (h2's) are indented more than top sections (h1's), but that h3's de-indent and are at the same level as h1. Example:
Screenshot 2025-04-17 at 1 01 32 PM

"Getting Started" and "Contributing Code and Ideas" have class .pl-2 (they're h2's), and "Pull Requests", "CIPs", and "Discord", have class .pl-3 (they're h3's), but they are not indented further under the h2's. So the hierarchy of header levels is not mirrored in the table of contents indenting.

The reason is that .pl-2 is hard-coded elsewhere in the codebase, so the style declaration gets bundled and shipped, but .pl-3 is not... so when the style .pl-3 gets computed, it doesn't match any style declaration, and it doesn't compute the padding-left.

Fix

Instead of refactoring out all of the dynamic utility classes, or coming up with an overly-clever solution to make it work, this PR is a patch to handle .pl-X classes to make the table of contents work as intended. You can easily figure out that .pl-x resolves to padding-left: ${0.25 * x}rem;

So to fix the ToC, assuming a generous potential heading-hierarchy of 10 levels, I just hard-coded .pl-{0..10} into globals.css.

The Sequel

When I initially did this, it didn't really look that interesting (i.e., the indenting is not that noticeable):

Screenshot 2025-04-17 at 1 02 52 PM

THIS is because the h1 items were getting .pl-1, but .pl-1 itself was not defined, so they had padding-left: 0. So the padding-left went from 0rem, to 0.5rem (pretty noticeable), but after declaring .pl-1 in the "expected" way, it went from 0.25rem to 0.5rem, then to 0.75rem (not that noticeable).

SO, in order to make this all come together and still keep the relative differences between the header levels' padding-left the same as before, while adding support for further levels of indenting, I decided to set the Table of Contents .pl-<LEVEL> values to be (level - 1) * 2, as in:

  • h1 => .pl-0
  • h2 => .pl-2
  • h3 => .pl-4
  • etc.

The result is that h1's and h2's are indented the same way as before (0rem, 0.5rem), and subsequent header levels are indented with the same delta as previously between h1's and h2's (i.e. 0.5rem more). The result is this:

Screenshot 2025-04-17 at 2 00 47 PM

Which is more noticeable and overall looks better, IMO, and doesn't involve re-defining any of the .pl- utility values themselves, so they can still be relied on to work the same as the Tailwind defaults.

Another cool example. (You're still reading this?)

The reference/js/collection page.

BEFORE:

Screenshot 2025-04-17 at 2 45 36 PM

Class is no indent, Methods is indented, but then individual methods are de-indented (e.g. add is de-intended relative to Methods.

AFTER:
Screenshot 2025-04-17 at 2 45 46 PM

Indenting of Class > indenting of Methods > individual method (e.g. "add") > "Parameters"

Test plan

How are these changes tested?

Run the app locally, look at the classes assigned to the different ToC items, compare to their corresponding header-levels in the main body, look at their indentings, see that all .pl- utility classes actually have style definitions.

Ran the build, no failures.

  • Tests pass locally with pytest for python, yarn test for js, cargo test for rust

Documentation Changes

Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the docs repository?

Copy link

Reviewer Checklist

Please leverage this checklist to ensure your code review is thorough before approving

Testing, Bugs, Errors, Logs, Documentation

  • Can you think of any use case in which the code does not behave as intended? Have they been tested?
  • Can you think of any inputs or external events that could break the code? Is user input validated and safe? Have they been tested?
  • If appropriate, are there adequate property based tests?
  • If appropriate, are there adequate unit tests?
  • Should any logging, debugging, tracing information be added or removed?
  • Are error messages user-friendly?
  • Have all documentation changes needed been made?
  • Have all non-obvious changes been commented?

System Compatibility

  • Are there any potential impacts on other parts of the system or backward compatibility?
  • Does this change intersect with any items on our roadmap, and if so, is there a plan for fitting them together?

Quality

  • Is this code of a unexpectedly high quality (Readability, Modularity, Intuitiveness)

@hesreallyhim
Copy link
Contributor Author

@jeffchuber just tagging you bcz I think you've been on some of the front-end PR's lately, hope that's all right

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

Successfully merging this pull request may close these issues.

[DOC]: Illegal use of dynamic Tailwind utility classes in docs
1 participant