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

[#2] Tailwind and PostCSS #8

Merged
merged 4 commits into from
May 16, 2024
Merged

[#2] Tailwind and PostCSS #8

merged 4 commits into from
May 16, 2024

Conversation

joshuapease
Copy link
Contributor

@joshuapease joshuapease commented May 15, 2024

Overview

Adds an empty Tailwind config and a standard PostCSS config.

Also including postcss-pxtorem per @maxfenton's recommendation.

It seems especially useful for arbitrary TW values (such as m-[20px])

How to test

Create a directory for your new Craft project

mkdir my-craft-project

# Then move into your directory
cd my-craft-project

Run the following composer create-project command.

It uses this branch as the starter project.

docker run --rm -it -v "$PWD":/app -v ${COMPOSER_HOME:-$HOME/.composer}:/tmp composer create-project viget/craft-site-starter=dev-jp/vite-setup ./ --ignore-platform-reqs

After completing the setup steps (it prompts for your project's name), run the following commands.

👇 These will gradually become more automated as this project progresses.

# Start DDEV
ddev start

# Install Craft
ddev craft install

# Install Vite plugin
ddev craft plugin/install vite

# Launch the site!
ddev launch

Here's what you'll see!

❓ Also... somehow Vite is reloading when I edit .twig files. Even though I haven't added vite-plugin-restart

Anyone else want to confirm if I'm crazy or not?

CleanShot 2024-05-14 at 17 21 15@2x

@joshuapease joshuapease marked this pull request as ready for review May 15, 2024 00:23
@joshuapease
Copy link
Contributor Author

@ekfuhrmann Wanted to get your thoughts on what a good starting point is for Tailwind.

An empty config? Or are there defaults that the UI team always reaches for?

src/css/app.css Outdated Show resolved Hide resolved
@joshuapease joshuapease merged commit 9355c9a into 5.x May 16, 2024
@joshuapease joshuapease deleted the jp/2-tailwind-and-postcss branch May 16, 2024 15:17

Choose a reason for hiding this comment

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

We've discussed this a bit in respects to Blueprint but the consensus was that we should include our own custom spacing system in the base tailwind. Not saying we should necessarily do it here, but having all of these values ready to go out of the box could be useful, and then it's easy enough to modify from there.

spacing: {
  0: '0px',
  1: '1px',
  2: '2px',
  4: '4px',
  6: '6px',
  8: '8px',
  10: '10px',
  12: '12px',
  16: '16px',
  20: '20px',
  24: '24px',
  28: '28px',
  32: '32px',
  36: '36px',
  40: '40px',
  44: '44px',
  48: '48px',
  52: '52px',
  56: '56px',
  60: '60px',
  64: '64px',
  80: '80px',
  96: '96px',
  112: '112px',
  128: '128px',
},

@ekfuhrmann
Copy link

The only suggestion is to include our spacing system. Since we're using postcss-pxtorem (which I love), keying it up as just px values is fine. I normally using a helper function like pxPair() to simplify it a bit, but in the grand scheme of things is pretty unnecessary.

Comment on lines +8 to +28
propList: [
'font',
'font-size',
'line-height',
'letter-spacing',
'border*',
'background*',
'grid*',
'top',
'left',
'bottom',
'right',
'inset',
'width',
'height',
'margin*',
'padding*',
'max-*',
'min-*',
'gap*',
],

Choose a reason for hiding this comment

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

Is this entirely necessary? We'll need to keep it upgraded based on props. We'll want to include size-* for instance.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as off-topic.

Copy link
Contributor

@maxfenton maxfenton May 16, 2024

Choose a reason for hiding this comment

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

@ekfuhrmann
Ok, so we could use propList: [*] and not need to update this; as I was adding postcss-pxtorem to Viget projects, I wanted to be cautious and know what was being changed.

I think we have two options:

  • an explicit list of properties (and add size* or size-*)
  • a wildcard propList [*]

Choose a reason for hiding this comment

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

Generally I use the following settings which seems to work best for me:

{
  rootValue: 16,
  propList: ['*'],
  mediaQuery: true,
  minPixelValue: 0,
},

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is propList referring to CSS properties?

size- is just a Tailwind class right?

I'm cool with using a * and letting projects opt out of specific properties.

From the docs

Use ! to not match a property. Example: ['*', '!letter-spacing']

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.

4 participants