Skip to content
This repository has been archived by the owner on Mar 18, 2022. It is now read-only.

Add Tailwind CSS preset #22

Merged
merged 18 commits into from
Aug 13, 2018
Merged

Add Tailwind CSS preset #22

merged 18 commits into from
Aug 13, 2018

Conversation

knowler
Copy link
Member

@knowler knowler commented Jul 17, 2018

This is a work in progress. So far I’ve just added the stubs for a Tailwind CSS preset as I figured it would be good to know what we are trying to generate. A lot of this is based on my guide for adding Tailwind to Sage, however, there are some differences:

  1. tailwind.js will be called tailwind.config.js to follow Sage’s config file naming scheme. It still does live in styles though, which could be discussed (read more: 589b623).
  2. I’ve added a few changes to Tailwind’s config: (see 31c2503)
    1. Add Sage primary colour.
    2. Add Sage primary colour (30%) to .shadow-outline.
    3. Centre + add padding to the container.
  3. I’ve added some styles via Tailwind’s @apply to replace the Bootstrap ones. This is a work in progress as there are a few partials left to do. I would like feedback about whether or not we should be adding these. I had noticed the todo comments in the Tachyons stubs so I figured I would give it a shot for Tailwind.

The installer will need to do the following:

  • Add Tailwind to the devDependencies.
  • Add Tailwind at-rules to package.json.
  • Add the modified PostCSS config.
  • Add the Tailwind config.
  • Add updated main.scss and partials.
  • On subsequent preset change, remove Tailwind-specific files
  • On subsequent preset change, remove Tailwind-specific at-rules

Left to do for the stubs:

  • Add search form styles.
  • Add styles for classes generated for WordPress.

Resolves #12.

Typically, this file is called `tailwind.js`, however, following Sage's
naming convention for config files, `tailwind.config.js` is more
suitable. Tailwind also recommends creating this file in the project's
root, others add it to `scripts/`, but I prefer to place it with the
`styles/` since — though it is JS — it is the source for the majority of
the styles. A strong case could be made for `build/` to place it
alongside `postcss.config.js`, since Tailwind is a PostCSS plugin.
Instead of a setting a variable we will do things the Tailwind way by
setting a new colour called `primary` which will be the Sage colour. As
well, the `.shadow-outline`'s colour is set to the Sage's colour (30%).
Finally, centring + horizontal padding on `.container` have been
enabled.
`main.scss` will most likely stay the same. I added the comments from
the their recommended stylesheet structure example since they are
informative: https://tailwindcss.com/docs/installation#3-use-tailwind-in-your-css

As well this adds a Tailwind interpretation of Sage's base styles. Of
course, since the markup is remaining in tact, I am using Tailwind's
`@apply` to take advantage of the existing selectors in the markup. A
stylistic choice I made here is to omit the period at the beginning of
the class names being applied (i.e. `@apply bg-primary text-white`
instead of `@apply .bg-primary .text-white` similar to `@extend` in
Sass). This is because when extracting components in Tailwind, it is
more ergonomic to grab all the selectors in a class attribute and chuck
them behind an `@apply` vice versa then to prepend a period everytime.
@mmirus
Copy link
Contributor

mmirus commented Jul 18, 2018

Nice work, @knowler. Can take a look at your stubs if you'd like, but I trust your judgment on that front. If I get a little extra time I'll at least skim through to see how you've set it up.

On my end, I pushed changes that add the Tailwind preset option and set the installer up to copy the stubs to the theme and add Tailwing to devDependencies. This PR is now functional.

As we discussed, I used a PATH repository to test locally, and updated the sage-installer version to point to the branch. Sage composer.json:

  "repositories": [
    {
      "type": "path",
      "url": "../sage-installer",
      "options": {
        "symlink": false
      }
    }
  ],
  ..
  "require-dev": {
    "squizlabs/php_codesniffer": "^2.8.0",
    "roots/sage-installer": "dev-tailwind-preset"
  },

In my case, I have the installer in a folder on the same level as the folder for the test copy of Sage I'm using--modify the repo path as needed.

Turning off symlinks was required to work around this error:

$ ./vendor/bin/sage preset
PHP Fatal error:  Uncaught Error: Class 'Roots\Sage\Installer\Installer' not found in C:\git\sage-installer\bin\sage:8
Stack trace:
#0 {main}
  thrown in C:\git\sage-installer\bin\sage on line 8

Fatal error: Uncaught Error: Class 'Roots\Sage\Installer\Installer' not found in C:\git\sage-installer\bin\sage:8
Stack trace:
#0 {main}
  thrown in C:\git\sage-installer\bin\sage on line 8

knowler and others added 5 commits July 18, 2018 01:08
This only adds the rules if they aren't already there. A new method
specific to modifying Stylelint config could be useful for this, but
since this one was already modifying `package.json`, I'll leave this
here. Also, this may not be the best way of doing things. Someone
smarter please review to let me know. :)

Body (72 chars)
Co-authored-by: Matthew Mirus <mmirus@afphq.org>
So testing the build in Sage doesn't break. These can be left TODO for
now, as they are not super essential.
This "fixes" a known issue when using Tailwind with Sass
(bholloway/resolve-url-loader#28). It removes the `resolve-url-loader`
from the loaders for SCSS files and disables source maps. Without this
change there would be a persistent warning when running `yarn start`.

This change requires a change to the webpack.config.js in roots/sage.
The final export needs to be:

```
module.exports = merge.smartStrategy({
  'module.loaders': 'replace'
})(webpackConfig, desire(`${__dirname}/webpack.config.preset`));
```
@knowler
Copy link
Member Author

knowler commented Jul 18, 2018

This is fully functional with roots/sage#2084. There are still missing stubs that I’ve left blank with a note TODO for now. I really don’t imagine a Tailwind user would rely on these styles by default — it just makes the default built styles not so plain (@tailwind preflight is an aggressive reset, i.e. removes margin from p and h1-6).

@knowler knowler changed the title [WIP] Add Tailwind CSS preset Add Tailwind CSS preset Jul 18, 2018
Before when changing preset, the PostCSS config would still have
Tailwind as a plugin. This leaves the Webpack preset config as the only
build file in need of clean up when changing preset.
@knowler
Copy link
Member Author

knowler commented Jul 19, 2018

If everything looks good, we are ready to rock. 🤘

An warning will still appear when running yarn start until we figure something out for roots/sage#2084.

@mmirus
Copy link
Contributor

mmirus commented Jul 20, 2018

Once roots/sage#2084 is merged, this will work correctly. Otherwise, the merge method in roots/sage#2083 breaks the Webpack config when this preset is used.

If the user changes from the Tailwind preset to another preset that doesn't overwrite all of the SCSS files that the Tailwind preset modifies, Tailwind @apply directives are left behind. This isn't pretty, but it's consistent with the installer's general behavior when switching presets.

Otherwise, I reviewed everything and didn't spot any problems besides the path fixed in 615dc71.

Thanks again for all your work on this @knowler!

Due to the limitations of webpack-merge we can't use it to remove
resolve-url-loader, so instead we are silencing the warning it would
throw and explicitly disabling source maps so there is no error in the
browser console during development.
@knowler
Copy link
Member Author

knowler commented Jul 20, 2018

So using webpack.config.preset.js to remove a loader doesn't isn’t possible due to limitations with webpack-merge, so instead, since it's just a warning that's being thrown, we enable the silent option for resolve-url-loader and explicitly disable all the source maps since they throw errors in the browser console since, when using Tailwind, you don't really use them anyway.

This is ready for merge if it looks good. roots/sage#2084 still needs to be merged first though.

Read more:
tailwindlabs/tailwindcss#48 (comment)

Copy link
Contributor

@mmirus mmirus left a comment

Choose a reason for hiding this comment

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

I think everything is good to go and both this and roots/sage#2084 can be merged. Would love @QWp6t or @retlehs to double check our work.

@mmirus mmirus merged commit 0c0e962 into master Aug 13, 2018
@mmirus mmirus deleted the tailwind-preset branch August 13, 2018 18:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants