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

Enable Tailwind utility and component classes #675

Merged
merged 19 commits into from
Oct 22, 2021
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/components/.storybook/preview.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import "@chanzuckerberg/eds-tokens/lib/css/variables.css";
import "../src/styles/fonts.css";
import "../src/styles/global.css";
import "../src/styles/tailwindUtilities.css";
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it'd be nice to mirror the naming convention in traject?

  • global.css -> tailwind-base.css (I think we should rename this in traject, will leave a comment in the other PR)
  • tailwindUtilities.css -> tailwind-utilities.css

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 This makes sense to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed as suggested


import React from "react";

Expand Down
41 changes: 41 additions & 0 deletions packages/components/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,3 +99,44 @@ declare var x: AbstractComponent<Props, HTMLElement>;
```

- Preserve exact-typed objects when possible. If you must make them inexact, add an explicit `...` at the end of the object.

## Tailwind Utility Classes
Tailwind utility classes can be used with either `@apply` in CSS files such as
```css
div {
@apply rounded border-2;
/* border-radius: 0.25 rem;
border-width: 2px; */
}
```
or used directly as class names such as
```jsx
<div className="rounded border-2">
```
Use the [docs](https://tailwindcss.com/docs) to search for appropriate classes.

### Potential conflicts with other styling libraries (e.g. Bootstrap)

If you're also using styles from a separate library, conflicts may arise rarely between utility class names. Here are some conflicts we're aware of for **Bootstrap** specifically:
- Most annoying, `.hidden`
- Both Tailwind and Bootstrap have the same styling for `.hidden` but Bootstrap applies the `!important ` property, which can be annoying when trying to utilize Tailwind breakpoints, e.g.:
```html
<div class="hidden md:block lg:inline">
```
- Where the expectation is `display: none ` for breakpoints smaller than 768px, `display: block `for 768px - 1023px, and `display: inline ` for >= 1024px. However since Bootstrap applies the `!important` property, all screen sizes show `display: none`.
- This can be circumvented with the `@apply` directive and/or using custom CSS with regular media queries to scope styling into a different class name, e.g.:.
```jsx
<div className="responsive-display">
```
```css
.responsive-display {
display: none;
@apply md:block lg:inline;
/* or use traditional @media queries */
}
```
- `.invisible` and `.text-` alignment
- These classes have the same styling in both Tailwind and Bootstrap, and therefore can be used without issues. Tailwind responsive states (such as breakpoints, hover, etc.) will have higher specificity so no issues will be caused there.
- Other unfound conflicts
- If the conflict is due to Bootstrap using `!important`, follow similar strategy above as `.hidden`.
- Bootstrap [styling](https://github.com/twbs/bootstrap-sass/tree/master/assets/stylesheets/bootstrap) and mentioned conflicts in [_utilities.scss](https://github.com/twbs/bootstrap-sass/blob/master/assets/stylesheets/bootstrap/_utilities.scss#L46) and [_type.scss](https://github.com/twbs/bootstrap-sass/blob/master/assets/stylesheets/bootstrap/_type.scss#L90) for reference
4 changes: 4 additions & 0 deletions packages/components/src/styles/global.css
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,8 @@
--tw-ring-shadow: 0 0 #0000;
}

/**
* This injects Tailwind's base styles and any base styles registered by
* plugins.
*/
@tailwind base;
11 changes: 11 additions & 0 deletions packages/components/src/styles/tailwindUtilities.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* This injects Tailwind's component classes and any component classes
* registered by plugins.
*/
@tailwind components;

/**
* This injects Tailwind's utility classes and any utility classes registered
* by plugins.
*/
@tailwind utilities;
2 changes: 2 additions & 0 deletions packages/components/tailwind.config.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
const variableTokens = require("@chanzuckerberg/eds-tokens/lib/json/css-variables-nested.json");
const staticTokens = require("@chanzuckerberg/eds-tokens/lib/json/variables-nested.json");
module.exports = {
mode: "jit",
purge: ["./src/**/*.{js,jsx,ts,tsx}"],
Copy link
Contributor

Choose a reason for hiding this comment

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

If we only want to use the classes in stories for this repo, and we expect other apps to configure their own purge templates (which is reasonable), we could make this

Suggested change
purge: ["./src/**/*.{js,jsx,ts,tsx}"],
purge: ["./src/**/*.stories.{js,jsx,ts,tsx}"],

to enforce that more

and we could add a comment here about the rationale, e.g. "The main value in utility classes for EDS is in storybook stories. We avoid using them in component styles so there's no chance for conflicts with other libraries."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed and added comment above line and in readme

theme: {
colors: {
transparent: "transparent",
Expand Down