tailwindlabs / tailwindcss

A utility-first CSS framework for rapid UI development.
https://tailwindcss.com/
MIT License
82.09k stars 4.15k forks source link

Replacing screens media queries with raw key breaks container core plugin #1701

Closed MartinMa closed 4 years ago

MartinMa commented 4 years ago

Tailwind CSS offers preconfigured breakpoints for your project. Namely sm, md, lg and xl. By default it takes into account the width property of the viewport, checking for min-width.

For example:

@screen xl {
  .foo {
    @apply text-6xl;
  }
}

becomes

@media (min-width: 1280px) {
  .foo {
    font-size: 4rem;
}

It is possible to overwrite the standard breakpoints in tailwind.config.js by providing own values for the min-width media query condition like this:

// tailwind.config.js
module.exports = {
  theme: {
    screens: {
      'sm': {'min': '576px'},
      'md': {'min': '768px'},
      'lg': {'min': '992px'},
      'xl': {'min': '1200px'}
    }
  }
}

You can even overwrite the rules completely with the raw key to provide your own customized condition as mentioned here.

For example:

// tailwind.config.js
module.exports = {
  theme: {
    screens: {
      lg: {
        raw: '(min-width: 1280px) and (min-height: 768px)'
      }
    }
  }
}

However, this has a side effect on the container core plugin (maybe even other plugins, too). The container core plugin intends to create style definitions for every screens breakpoint to limit the width of a container.

The .container class sets the max-width of an element to match the min-width of the current breakpoint.

By default it creates the following style definition for xl:

@media (min-width: 1280px) {
  .container {
    max-width: 1280px;
}

However, this does not work anymore (rule is not created), when overwriting the breakpoint condition using the raw key as mentioned above. No rule for the lg breakpoint is created for the container class.

This is clear if you know how things interoperate with other parts of Tailwind CSS. For me it was unexpected and surprising.

I think to documentation should be adjusted accordingly. Explain that the container core plugin depends on the screens min key value and should always be provided alongside custom raw key values. What do you think?

johnagan commented 4 years ago

I'm not sure if I'm having a similar issue or if what I'm experiencing is expected and I just don't understand why, but when I enter in custom screens with min values it gets generated as a max-width.

Here's the code that generates it: https://github.com/tailwindcss/tailwindcss/blob/358479185258490a372e4d3b2273a4dc11a822fd/src/plugins/container.js#L91

I'm pretty sure this should be:

'min-width': minWidth;
adamwathan commented 4 years ago

Can you provide an example of a complete custom raw config and what you would expect the container output to be given that config? That will help us understand what we should do, whether that be ignore the screen size or respect it using some more sophisticated intelligence than we use currently.

johnagan commented 4 years ago

@adamwathan thoughts on the line of code I shared above. Is that a typo?

adamwathan commented 4 years ago

Not a typo, the container max-widths are derived from min-width breakpoints. If you can provide an example of your input and desired output I can give you a better response.

johnagan commented 4 years ago

It seems to me that if I passed in a max value, it should use that for the container width, rather than disregarding it.

an example config of:

{
  "screens": {
    "minMax": { "min": "640px", "max": "767px" },
    "maxOnly": { "max": "767px" }
  }
}

The output it produces skips the max end of the range on minMax and completely skips maxOnly

/* minMax */
@media (min-width: 640px) {
  .container {
    max-width: 640px;
  }
}
/* maxOnly was not generated */

but I would expect that since I passed in a max, it would be:

/* minMax */
@media (min-width: 640px) and (max-width: 767px) {
  .container {
    min-width: 640px;
    max-width: 767px;
  }
}

/* maxOnly */
@media (max-width: 767px) {
  .container {
    max-width: 767px;
  }
}

I apologize if I'm way off here - it's possible I'm too close to it and I've ended up in a rabbit hole. I've been including a workaround in my css that looks like this (values and variable names are just for this example):

.container {
  @screen minMax {
    min-width: 640px;
    max-width: 767px;
  }
  @screen maxOnly {
    max-width: 767px;
  }
}
johnagan commented 4 years ago

Since I hijacked this issue, I investigated @MartinMa's question and I think I see the issue:

screens: {
  lg: {
    raw: '(min-width: 1280px) and (min-height: 768px)'
  }
}

I think it should be:

screens: {
  lg: '(min-width: 1280px) and (min-height: 768px)'
}

and it will produce:

@media (min-width: (min-width: 1280px) and (min-height: 768px)) {
  .container {
    max-width: (min-width: 1280px) and (min-height: 768px);
  }
}

Plus all the lg utility classes such as:

@media (min-width: (min-width: 1280px) and (min-height: 768px)) {
  .lg\:space-y-0 > :not(template) ~ :not(template) {
    --space-y-reverse: 0;
    margin-top: calc(0px * calc(1 - var(--space-y-reverse)));
    margin-bottom: calc(0px * var(--space-y-reverse));
  }
...
adamwathan commented 4 years ago

No plans to change any behavior here, the container won't be modified to parse and understand raw screens. Best to stick to the higher level API if you want the container classes to reflect your custom screens 👍

hayyaun commented 3 months ago

No plans to change any behavior here, the container won't be modified to parse and understand raw screens. Best to stick to the higher level API if you want the container classes to reflect your custom screens 👍

There's a doc about this. Is this older than the comment or newer? It's not working for me

wongjn commented 3 months ago

The documentation is about configuring screens, not container.