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 vars to media-queries #2314

Closed
zoepage opened this issue Mar 28, 2018 · 16 comments
Closed

Add vars to media-queries #2314

zoepage opened this issue Mar 28, 2018 · 16 comments

Comments

@zoepage
Copy link
Member

zoepage commented Mar 28, 2018

https://www.npmjs.com/package/postcss-media-variables

@zoepage
Copy link
Member Author

zoepage commented Mar 28, 2018

This is failing for me, so lets postpone that issue as its not blocking. If anyone wants to take over, feel free. :)

@magsout
Copy link
Member

magsout commented Mar 28, 2018

After updated postcss-* and added postcss-media-variables

Running "watch" task
Waiting...
>> File "webcompat/static/css/src/grid.css" changed.
Running "postcss:dist" (postcss) task
Node#before is deprecated. Use Node#raws.before
>> 1 processed stylesheet created.

Done.

So I think there is a package which is not updated for the postcss version 6..

@magsout
Copy link
Member

magsout commented Mar 28, 2018

I'm reading that postcss/postcss-custom-properties#24 and MoOx/postcss-cssnext#253

Not sure it's a good idea to add this kind of plugin.

@zoepage @miketaylr ?

@magsout
Copy link
Member

magsout commented Mar 28, 2018

Maybe we can just add custom-media-queries in variables.css and let the comment to rely on the var(--page-size-m) like this

// rely on var(--page-size-m)
@custom-media --page-min-size-m (max-width: 665px);

and use as:

@media (--page-min-size-m) {
  .grid-row {
    margin: 0 calc(-1 / 2 * var(--unit-space)) var(--grid-outermargin-m);
  }

  .grid-show-gap .grid-cell {
    margin-bottom: var(--grid-outermargin-m);
  }

  .x1-fix {
    max-width: var(--unit-modul-s);
  }
}

@miketaylr
Copy link
Member

Yeah, I dunno. If it's a current limitation of CSS, maybe it's just better to live with it.

@magsout
Copy link
Member

magsout commented Mar 30, 2018

@miketaylr the good thing, we can update postcss with latest versions without error ;)

@Regaddi
Copy link
Member

Regaddi commented Apr 18, 2018

Even though CSS custom properties are a nifty feature, maybe porting some code to Sass might be valid? I know it's another layer in the toolchain. But this would completely allow usage of functions, mixins, nesting and variables everywhere you need it.
It will also help making the code more DRY.
However, when it comes to nesting styles, I'm a big fan of BEM to avoid over-nesting. There are also ready-to-use stylelint configs available.

Porting to Sass is pretty easy, since CSS is completely valid Sass code.

@magsout
Copy link
Member

magsout commented Apr 18, 2018

@Regaddi to clarify my comments, I'm not a big fan of Sass, not anymore, I used Sass couple of years.

The question is What can we do with Sass we can not do with css?

IMO, Sass is motly another layer writing a subset of css. it helps writing css wich is repeating but that's it.

@Regaddi
Copy link
Member

Regaddi commented Apr 18, 2018

The question is What can we do with Sass we can not do with css?

We can use vars in or for media queries and solve this issue 😅

Joking aside, CSS has matured a lot in the last few years and I'm really happy about this evolution. One thing that I still dislike is that I have to repeat parts of my selectors over and over again. But this is probably just personally. If I have the opportunity to do the same thing with less code, why not? 😄

So basically yes, Sass is mostly another layer of CSS, but IMO a pretty convenient one for developers.

In the end it's an architectural decision and I just wanted to drop my ideas 😄

@magsout
Copy link
Member

magsout commented Apr 19, 2018

@Regaddi

We can use vars in or for media queries and solve this issue 😅

good point 👍

If I have the opportunity to do the same thing with less code, why not? 😄

another good point

So basically yes, Sass is mostly another layer of CSS, but IMO a pretty convenient one for developers.

yes, it is.

In the end it's an architectural decision and I just wanted to drop my ideas 😄

and you're totaly right, it's a good thing ;) thanks for that.

@zoepage
Copy link
Member Author

zoepage commented Apr 20, 2018

To give some context, we didn't see a need for SASS before. This is why we also didn't use it.
I think, the option for usage of functions, mixins, nesting and variables is a pro on using it.

My only concern is that SASS can blow up the code massively (as already mentioned), this is why I really like the OOCSS approach, which goes well with React as well as other frameworks. BEM is something we decided against as its a design pattern that has been used in various ways by developers and although there is a very clear definition of it, devs like to use it in their versions (my experience in the past few years and various projects).

As OOCSS is already the base and BEM would also include a re-factor of the CSS, I'd be contra BEM, but okay with SASS.

@Regaddi Regaddi self-assigned this Apr 20, 2018
Regaddi pushed a commit that referenced this issue Apr 20, 2018
Regaddi pushed a commit that referenced this issue Apr 20, 2018
@miketaylr
Copy link
Member

To me it feels like switching to SASS to solve this one issue feels like a bit much. I'm a respectful -1, though I appreciate the PR and contribution from @Regaddi.

@Regaddi
Copy link
Member

Regaddi commented Apr 24, 2018

@miketaylr It is not really solely solving this particular issue. With Sass we're able to establish a clean stylesheet architecture, which can't be achieved with pure CSS in a satisfying way. Sass allows to strictly divide the stylesheet code into configurational, reusable and applying parts. It allows to e.g. abstract the styles for labels without duplicating CSS code (as we currently do). So for this particular issue here the switch seems like a big cannonball, but in the long-term I can see great advantages.

@zoepage
Copy link
Member Author

zoepage commented Apr 24, 2018

The biggest advantage I see it the normalisation of margins / paddings and gaps between the grid elements through functions that can live in one place and be reused. solving the breakpoints issue is a bonus for me.

@miketaylr
Copy link
Member

With Sass we're able to establish a clean stylesheet architecture, which can't be achieved with pure CSS in a satisfying way.

This feels a bit subjective, since clean and satisfying mean different things to each of us. :)

Sass allows to strictly divide the stylesheet code into configurational, reusable and applying parts.

Yeah, this could be good. I feel like we should consider this at some point, and maybe collect concrete use cases for it now, but I don't see that we need it today.

Related, https://engineering.linkedin.com/blog/2018/04/css-at-scale--linkedins-new-open-source-projects-take-on-stylesh is an interesting read. Obviously we're not at LinkedIn scale, but they list some problems they had with bloat caused by using Sass mixins and the like.

@miketaylr
Copy link
Member

I'm going to WONTFIX this, we can revisit if we feel like we need it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants