morajabi / styled-media-query

πŸ’…πŸ’ Better media queries for styled-component
MIT License
1.32k stars 53 forks source link

Discussion for v3 API for more flexibility and readability #11

Open oyeanuj opened 7 years ago

oyeanuj commented 7 years ago

Hi @morajabi, thank you for putting out this library. It has been super helpful so far. In my current project, I keep needing to write height based media queries, and in doing so, I have to opt traditional media query rather than use this library.

So, I was wondering if you are considering adding support for other aspects of media query in the library API?

morajabi commented 7 years ago

@oyeanuj Thanks! I had something similar in mind for a while now. There's a couple of ways to do this. One possible implementation is to have 2 top level keys on the breakpoints object:

const breakpoints = {
  width: {
    large: '1170px',
    medium: '768px',
    small: '450px',
  },
  height: {
    small: '700px',
  }
}

Then probably we can change the API like this:

const Box = styled.div`
  background: black;

  ${media.width.lessThan('medium')`
    background: red;
  `}
`

Although it's a rough idea. I'm afraid I don't have time currently (maybe after this week?) I'll probably want to talk to SC teammates to refine the API to be more idiomatic to styled components for version 3.

oyeanuj commented 7 years ago

@morajabi Thanks for the quick response. Your proposal looks good! I was thinking along the same lines for the API with additional support for:

media.orientation.is('landscape')
media.orientation.not('portrait')
media.screen

A bonus API feature could be if they could be chained like below:

media.width.lessThan('medium').orientation('landscape')

Thoughts?

morajabi commented 7 years ago

@oyeanuj That would add a lot of additional API complexity which I don't want this to be harder than the default CSS media query syntax. BTW for your problem a quick fix might be to export your breakpoint variables and just use them with interpolation in SC styles:

import styled from 'styled-components'
import { breakpoints } from '../utils/media'

const Wrapper = styled.div`
  @media landscape and (min-width: ${breakpoints.medium}) {
    /* whatever */
  }
`
tlvenn commented 6 years ago

Chainable API is pretty elegant and imho does not increase the complexity. If you dont need to specify the orientation, it changes nothing and if you do, you only have to chain another constraint, you dont have to modify your current constraint to accommodate it.

morajabi commented 6 years ago

What if we stick to the syntax that we have but let the library write the most confusing part which is min-with, max-width as well as for height?

@media landscape and ${media.width.lessThan('small')} {
}

and this one for height:

@media landscape and ${media.height.between('small', 'big')} {
}

Now we are not reinventing the wheel. Thus we now have a very flexible API and syntax highlighting which I believe we don't have now because we're not using css` `

Basically, the @media syntax is fine in most situations but specifying sizes is very confusing, I'd like to just solve the pain point and not reinvent a whole new media query syntaxβ„’ πŸ˜‰

The only problem is it's a bit lengthy, and I don't like lengthy media queries, I often wrap them in a function to use them like so:

${mobile(css`
 /* -- */
`)}

Thoughts about the API and if you have suggestions to make it less lengthy? @oyeanuj @ApacheEx @tlvenn

ApacheEx commented 6 years ago

here is all media features: https://www.w3.org/TR/css3-mediaqueries/#media1 currently, we support only min-width, max-width πŸ’ͺ

should we support all media features or (min/max) width, (min/max) height, orientation would be enough - that's also a question to discuss.

I don't like lengthy media queries

πŸ’― agree

p.s. I will try to make some suggestions a bit later πŸ€”

morajabi commented 6 years ago

should we support all media features or width, height, orientation would be enough - that's also a question to discuss.

That's exactly why I'm saying going this route might not be the best way to do it, replicating everything from the original API doesn't seem good.

@ApacheEx What do you think about that API I suggested?

ApacheEx commented 6 years ago

ok,

1) yours: (looks good but a bit lengthy)

const Button = styled.button`
  width: 120px;
  @media (orientation: landscape) and ${media.width.between('small', 'big')} {
     width: 300px;
  }
`;

2) mine: (a bit complex, but quite flexible)

Idea

Add features possibility which supports any features from https://www.w3.org/TR/css3-mediaqueries/#media1. What you put there is what you get in media (no extra logic).

I love current API, let's make something similar ❀️

const defaultFeatures = {
  landscape: {
    orientation: 'landscape',
  },
  portrait: {
    orientation: 'portrait',
  },
}

If I want to add my own features:

// ./src/styles/style-utils.js

// My own breakpoints.
const breakpoints = {
  xs: '250px',
  sm: '450px',
};

// My own features.
const features = {
  iphoneX: {
    orientation: 'landscape',
    'device-aspect-ratio': '21/9',
    ...etc 
  },
  ..etc
};

export const media = generateMedia(breakpoints, features);

Finally:


// ./src/components/Button.js

import { media } from 'styles/style-utils';

const Button = styled.button`
  width: 120px;
  ${media.width.between('xs', 'sm')('iphoneX')`
     width: 300px;
  `};
  ${media.width.greaterThan('sm')('landscape')`
     width: 400px;
  `};
  ${media.greaterThan('sm')`
     width: 400px;
  `};
`;

will generate

@media all and (min-width: 250px) and (max-width: 450px) and (orientation: landscape) and (device-aspect-ratio: 21/9) { … }

@media all and (min-width: 450px) and (orientation: landscape) { … }

@media all and (min-width: 450px) { … }

Thoughts?

morajabi commented 6 years ago

I really love the API design of features. πŸ’― @ApacheEx

I'd like to add one more thing named aliases:

const aliases = media => {
  mobile: media.width.lessThan('sm'),
  tablet: media.width.between('sm', 'lg'),
}

export const media(breakpoints, features, aliases)

Then you can use the old API as well as the new features API AND the shorthands/aliases you declared:

const Wrapper = styled.div`
   ${media.mobile``}
   ${media.for('retina')``}
   ${media.width.lessThan('small').for('retina')``}
`

Sometimes I only want a really custom media query and I only use it once, it doesn't make sense to make a feature for everything, to solve this, we can get the media string by calling it without argument:

const Wrapper = styled.div`
  @media landscape and ${media.mobile} {
  }
`

All of these are quite optional for user to provide.

I'm was a fan of V1 API because it was so minimal like so: media.mobile, I'm trying to achieve that simplicity again.

ApacheEx commented 6 years ago

wow, looks awesome πŸ’ͺ πŸ”₯

so, as conclusion:

1) add features which can be applied via for:

    ${media.for('retina')`
       display: flex;
    `};

    ${media.width.between('sm', 'lg').for('retina')`
       display: block;
    `};

2) add aliases which simplify life with lengthy/complex queries:

    ${media.mobile`
       display: flex;
    `};

    ${media.tablet.for('retina')`
       display: block;
    `};

3) add possibility to use inline media:

    @media print and ${media.mobile} {
      display: flex;
    }

    @media all and ${media.width.lessThan('small')} {
      display: block;
    }

agree? am I miss something?

oyeanuj commented 6 years ago

@ApacheEx Looks good, and the readability is helped with the aliases and chaining. Looking forward to this!

morajabi commented 6 years ago

Looks awesome @ApacheEx!

What do you think about having syntax highlighting? Currently, anything tagged with css`` gets CSS syntax highlighting, but as we don't use that currently we are missing the beautiful syntax highlighting in editors and IDEs.

/cc @julienp wdyt?

ghost commented 5 years ago

Is version 3 under development? I hope so, because the API for version 3 looks excellent and it would be a shame if it doesn't get implemented.

timhagn commented 5 years ago

Hi @all,

as this issue is open long enough and, as seen with #17, some things gotta change, I just proposed a backwards compatible way of going at the really excellent looking API for V3: Chainable Callable Class Functions : ).

Have a look at #20 and tell me what you think, should you be so kind.

Best,

Tim.

oyeanuj commented 4 years ago

@morajabi Is v3 still on the cards? If so, any preferences between @ApacheEx and @timhagn's proposals?