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

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

Closed
MartinMa opened this issue May 4, 2020 · 9 comments
Closed

Comments

@MartinMa
Copy link

MartinMa commented May 4, 2020

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
Copy link

johnagan commented Jul 6, 2020

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
Copy link
Member

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
Copy link

johnagan commented Jul 6, 2020

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

@adamwathan
Copy link
Member

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
Copy link

johnagan commented Jul 11, 2020

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
Copy link

johnagan commented Jul 11, 2020

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
Copy link
Member

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
Copy link

hayyaun commented Jun 20, 2024

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
Copy link
Contributor

wongjn commented Jun 20, 2024

The documentation is about configuring screens, not container.

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

No branches or pull requests

5 participants