[ENH]: Hard-code dynamic Tailwind util classes (improve ToC styling)#4307
[ENH]: Hard-code dynamic Tailwind util classes (improve ToC styling)#4307hesreallyhim wants to merge 4 commits intochroma-core:mainfrom
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
|
@jeffchuber just tagging you bcz I think you've been on some of the front-end PR's lately, hope that's all right |
|
@jeffchuber bump |
|
Hey @hesreallyhim, we don't want to override the Tailwind styles. I prepared an example with a simple workaround here if you want to take a look: #4493. You'll see that another bug exists when sections have the same name (and thus ID), we will not scroll the page correctly. Let me know if you're up for the challenge! |
|
@itaismith yeah you're correct that's non-optimal to override them. i'll have to spin up your branch and see what you're trying to achieve, but i do think my solution is roughly correct if the scope is just the ToC (and if you want it to look the way it does in the screenshot of my branch). Apparently Tailwind reads source files as plain text and doesn't even parse them, so even a line like: |
Description of changes
Fixes #4129
.pl-2,.pl-3=>padding-left: 0.5rem;,padding-left: 0.75rem;, etc.), which provide short-hand, computed classes from Tailwind (very handy)..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-4so, by accident, if your dynamic class is.pl-4at runtime, then it will find the style declarations for it, and all is well.)"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-2is hard-coded elsewhere in the codebase, so the style declaration gets bundled and shipped, but.pl-3is not... so when the style.pl-3gets 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-Xclasses to make the table of contents work as intended. You can easily figure out that.pl-xresolves topadding-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}intoglobals.css.The Sequel
When I initially did this, it didn't really look that interesting (i.e., the indenting is not that noticeable):
THIS is because the h1 items were getting
.pl-1, but.pl-1itself was not defined, so they hadpadding-left: 0. So thepadding-leftwent from0rem, to0.5rem(pretty noticeable), but after declaring.pl-1in the "expected" way, it went from0.25remto0.5rem, then to0.75rem(not that noticeable).SO, in order to make this all come together and still keep the relative differences between the header levels'
padding-leftthe 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:.pl-0.pl-2.pl-4The 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.5remmore). The result is this: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/collectionpage.BEFORE:
Class is no indent, Methods is indented, but then individual methods are de-indented (e.g.
addis de-intended relative toMethods.AFTER:

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.
pytestfor python,yarn testfor js,cargo testfor rustDocumentation Changes
Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the docs repository?