roots / sage-installer

Sage 9 installer
https://github.com/roots/sage
MIT License
30 stars 85 forks source link

Add Tailwind CSS preset #22

Closed knowler closed 5 years ago

knowler commented 5 years ago

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:

Left to do for the stubs:

Resolves #12.

mmirus commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

Once https://github.com/roots/sage/pull/2084 is merged, this will work correctly. Otherwise, the merge method in https://github.com/roots/sage/pull/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 https://github.com/roots/sage-installer/pull/22/commits/615dc71ef219e13bd769dcd0e795fb863fb830b0.

Thanks again for all your work on this @knowler!

knowler commented 5 years ago

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: https://github.com/tailwindcss/tailwindcss/issues/48#issuecomment-341423237