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

Add first-class support for height media/container queries #13389

Open
wants to merge 4 commits into
base: next
Choose a base branch
from

Conversation

brandonmcconnell
Copy link
Contributor

This is a follow-up to closed PRs #11217 and #11225 (third time's a charm, right?)

I accounted for the latest changes recommended by @adamwathan here, specifically regarding syntax.

Syntax: min/max-h-[…]:

  • sm: (already supported, min and width assumed)
  • min-sm: (already supported, min explicit, width assumed)
  • h-sm: (✨NEW✨, min assumed, height explicit)
  • min-h-sm: (✨NEW✨, min and height explicit)
  • w-sm: (🚫 NOT SUPPORTED, min assumed, height explicit) 1
  • min-w-sm: (🚫 NOT SUPPORTED, min and height explicit) 1

Thankfully, in the next branch, most support for dimension-specific sorting and inversion appears to already be supported out of the box, as do container queries, so we may even be able to close this related issue as well once v4 goes live, assuming the tailwindcss-container-queries plugin will also be sunset at that time.

Next step(s) — feedback / testing

I'm sure we'll still want to add a few tests here, especially for cases where height and width queries are mixed, and consider how we want those to sort (e.g. inner-most variants first vs. width-first always, etc.), but I wanted to get this PR in quickly so we can hopefully include it in the v4.0.0 launch.

If someone from the core team could poke around at my existing tests, maybe try blowing my impl up or throwing in some wonky examples to stress test it, that would help to find any glaring bugs. It would also help to take a peek at the updated snapshots to make sure all the updated bits look correct.

Footnotes

  1. Note: I considered adding min-w- explicitly as well, but thought it might be best not to have two variants that do the same thing (min-w- vs. min-). Let me know if you want to support both, with the shorthand as an alias for the explicit. Should be a quick change. 2

@adamwathan
Copy link
Member

Thanks @brandonmcconnell! What's the thinking with supporting all of the default width breakpoints as heights? Do those values match up to common problems people solve with height-based media queries?

@brandonmcconnell
Copy link
Contributor Author

brandonmcconnell commented Mar 28, 2024

@adamwathan Tbh I was thinking through that as well, and I'm of a few minds here:

Option 1: Use the same named breakpoints for widths and heights

The main benefit here would be that each name is associated with one specific CSS length. One primary use case for this would be when setting orientation-specific styles (landscape vs. portrait). Named height queries when on one orientation vs. the other would take up the same amount of inline screen real estate that width queries would on portrait, and vice versa.

Option 2: Introduce unique height breakpoints

With this option, you lose the size consistency of inline queries between orientations, but these would arguably be better suited in most cases.

I'm not sure what the best height breakpoints would be if we introduce some unique ones, which causes me to lean towards options 1 or 2. It would help to do some sort of survey of the community to see if there are specific height breakpoints people often use.

Thinking on this a bit more, especially in the case of orientation-specific styles, it would actually be most useful to be able to target those with logical media queries, so I just opened a CSSWG issue to discuss that further: csswg-drafts/10156

TL;DR: I could see some argument being made for unique named height breakpoints, but without some survey and consensus, I think it would probably be best and safest to go with option 1 or 2 (or another).

Option 3: Assume height queries are always an edge case

This is the case for me when using height queries more often. With this option, we restrict ourselves to using only arbitrary height breakpoints unless we see an emergent need for named height breakpoints (using one of the previous options) in the future.

Option 3 also grants us the greatest flexibility for the future, so we're not married to an approach we don't favor and have to introduce new syntax or breaking changes down the line to resolve it.

@RobinMalfait RobinMalfait requested a review from a team as a code owner November 21, 2024 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing support for core-supported dynamic breakpoints
3 participants