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

tailwind #1762

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

tailwind #1762

wants to merge 9 commits into from

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Oct 17, 2024

Adds a tailwind plugin to esbuild.build for css files.

I could not find how to pass a custom configuration directly, so I'm currently creating an intermediate root/.observablehq/cache/tailwind.config.js to direct tailwind to the project files, so it can analyze the "content" that might contain relevant class names.

And you can use it like so in your page.md:

<div class="tw-rounded-xl tw-p-8 tw-text-lg tw-font-mono tw-bg-black tw-text-white">
  Hello, tailwind
</div>

This intermediate file imports {src}/tailwind.config.js if it exists, allowing the user to configure tailwind, for example by setting prefix: "".

Here's another example with a custom src/tailwind.config.js file:

/** @type {import('tailwindcss').Config} */
export default {
  theme: {
    colors: {
      "almost-white": "#fefefe",
      "bubble-gum": "#ff77e9",
    }
  },
  plugins: []
};

You can then use the new colors:

<div class="tw-rounded-xl tw-p-8 tw-text-lg tw-font-mono tw-bg-bubble-gum tw-text-almost-white">
  Hello, tailwind
</div>

The way tailwind works, is by scanning all the "content" files (in our case, the markdown, js, and page loaders), for strings that match its classnames, and then builds the corresponding styles.

TODO:

  • figure out dark mode (using the "selector" strategy, I guess)
  • fix tests
  • figure out hashing
    • fix live preview
  • make the tailwind config part of the framework config
  • adopt tailwind’s grid? maybe slightly adapted?
  • customize breakpoints
    • use @container breakpoints, adapt to the true available size (depending on toc)
  • fix everything that is reset
    • h1…h6 (dirty patch for now)
    • lists
    • others?
  • add tests
  • document

closes #595
supersedes #596

@Fil Fil requested a review from mbostock October 17, 2024 10:40
@Fil Fil marked this pull request as draft October 17, 2024 10:40
@Fil
Copy link
Contributor Author

Fil commented Oct 17, 2024

Here's a solution for dark mode:

Add darkMode: ["variant", "&:where([class~=dark], [class~=dark] *)"] to the default tailwind config, then add a javascript class toggle on the root.

```js
document.querySelector("html").classList.toggle("dark", dark);
```

<div class="
  tw-bg-white dark:tw-bg-black
  tw-text-red-600 dark:tw-text-emerald-600
">
  TEST DARK MODE
</div>

it could be better (I think framework should add the darkclass on the root in parallel with the color-scheme property), but it works. The standard tailwind color classes work in dark mode out of the box, so that's good too.

We should also coordinate this with #1638

@Fil
Copy link
Contributor Author

Fil commented Oct 19, 2024

Another thing to figure out is the hash. I don't think we want to update the css each time any md or js file changes… so maybe we have to watch all these files for changes in the list of statically analyzable "potential tailwind class names" that they contain.

@mbostock
Copy link
Member

In preview we could generate a tailwind.css that is specific to just the current page (+ any imported local modules) rather than continuously trying to recompute it for the entire app.

@Fil
Copy link
Contributor Author

Fil commented Oct 25, 2024

I've updated the logic a bit, now we have continuous updating of a separate tailwind.css

(It's still not limited to the current page, because I don't know how to pass an object configuration—it seems it only wants a file-based configuration).

I've had to blocklist the grid classes, since they conflict with ours. I guess it's going to be a fork in the road: continue with our own grid, or switch to tailwind's. The difference is that in tailwind you have to be explicit about your breakpoints.

Speaking of breakpoints we should make sure we use the same. DONE

@Fil
Copy link
Contributor Author

Fil commented Oct 25, 2024

I've now tested tailwind plugins, it just works. For example,

yarn add --dev @tailwindcss/typography

then in docs/tailwind.config.js:

import typography from "@tailwindcss/typography";

export default {
  plugins: [ typography ]
};

…roken

but this resets h1,h2,h3…, which we don't want. So let's remove that particular declaration (dirty for now)

(for instance, `focus:ring-2 focus:ring-purple-600 focus:ring-offset-2`)
src/rollup.ts Outdated
Comment on lines 62 to 65
// dirty patch for tailwind: remove margin:0 and styles resets for headers
// etc. It should probably be a tailwind plugin instead.
if (path === getClientPath("tailwind.css"))
text = text.replaceAll(/}[^{]*h1,\n*h2,\n*h3,\n*h4,\n*h5,\n*h6[^}]+}\s*/g, "}");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You want the typography plugin and prose class.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, but there are other resets that wreck our page, and we might want to find a way to remove the resets independently of adding /prose/ (this works already, anyway). I was thinking maybe a custom plugin we'd inline.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may need to commit to Tailwind and rewrite any of our own styles accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that's a good possibility. For now I'm merely taking stock of the things that change.

The other big change I've noticed are the grid-cols-* classes, which I have to block for now because they are quite different. Ours is responsive to width, tailwind's responsiveness is based on a breakpoint modifier (e.g. lg:grid-cols-3).

blocklist: ["grid", "grid-cols-2", "grid-cols-3", "grid-cols-4"],

@Fil Fil mentioned this pull request Oct 25, 2024
@CobusT
Copy link
Contributor

CobusT commented Oct 25, 2024

Do we need to change the default breakpoints for tailwind? Users may find that surprising.

@Fil
Copy link
Contributor Author

Fil commented Oct 25, 2024

These are the breakpoints of framework, my assumption here is that users are working inside “main” so the reference for sizes is the width of main.

Here's a way to test:

## breakpoints

<ul>
  <li><span class="text-red-500">&lt;sm</span> (always on)</li>
  <li><span class="sm:text-red-500">sm</span></li>
  <li><span class="md:text-red-500">md</span></li>
  <li><span class="lg:text-red-500">lg</span></li>
  <li><span class="xl:text-red-500">xl</span></li>
</ul>

if we leave tailwind's breakpoints unchanged, these element light up at very "random" places.

(It's the same issue that the tailwindcss-container-queries plugin addresses.)

@CobusT
Copy link
Contributor

CobusT commented Oct 25, 2024

These are the breakpoints of framework, my assumption here is that users are working inside “main” so the reference for sizes is the width of main.

That probably makes sense, then. If they want to reset it they can do that in their own config file, but this change is probably more good than unexpected :-)

@Fil
Copy link
Contributor Author

Fil commented Oct 25, 2024

The reference is here, with an example showing the contorsions you have to do when you have a sidebar.
https://v1.tailwindcss.com/docs/breakpoints

In our case it's even more complicated, because the space available to draw stuff (which I believe is what should drive the definitions of sm, md, lg), depends on whether the TOC is shown, and on the themes used (e.g. wide). And that's not only a function of css and the page width… I have a hunch that the @container approach might not be fully correct either because the toc is an aside, so we have to substract its width from the value. But we're already using this container for our own grids, so maybe I'm missing something?

(I don't like the way the TOC works, and the fact that it is not visible at all on smaller screens is also problematic, so it might be the final draw telling that we have to rewrite it with tailwind.)

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.

Tailwind integration
3 participants