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

Covert pixels to ems in output #14

Closed
ghost opened this issue Jan 14, 2015 · 8 comments
Closed

Covert pixels to ems in output #14

ghost opened this issue Jan 14, 2015 · 8 comments

Comments

@ghost
Copy link

ghost commented Jan 14, 2015

Add the default (or optional) functionality for converting pixel values to em values. Often overlooked benefit, but The EMs have it: Proportional Media Queries FTW! is explaining it in greater detail.

@mattboon
Copy link

+1 for the future.

Usually use em for media queries but am willing to switch to px for this project as this lib is awesome!

@eduardoboucas
Copy link
Owner

So right now I'd say that we have partial support for EMs. For example, you can do this:

$breakpoints: (phone: 20em, tablet: 48em, desktop: 64em);

@include media(">=phone") {
}

/* Generates: */

@media (min-width: 20em) {
}

The one thing that doesn't work properly right now is when you use greater-than or less-than operators. By default, it adds or subtracts 1 pixel when you use those operators.

Example:

  • px: >phone means >320px so it translates to (min-width: 321px).
  • em: >phone means >20em so it translates to (min-width: 21em) (not what we want).

So maybe we need to add a different factor for each unit? So if you're dealing with pixels it adds 1px, but if you have EMs it adds 0.01em or something?

Let me know your thoughts guys, and thanks for the feedback!

@eduardoboucas
Copy link
Owner

I think that the good thing about this mixin is its flexibility and how it allows developers to use in many different ways. For that reason, I don't think it is a good idea to convert the breakpoints from pixels to ems - if you define $breakpoints in pixels, then it's because you want to use pixels and I don't think we should force EMs.

What I do think is that we need to support it properly, and the implementation is actually quite simple. Something like this:

@function _get-interval-for-unit($value) {
  $intervals: ("px": 1, "em": 0.0625);

  @return map-get($intervals, unit($value));
}

This will establish the intervals that should be used for each unit, so:

$breakpoints: (phone: 30em, tablet: 48em, desktop: 64em);

@include media(">phone") {
}

/* Generates: */

@media (min-width: 30.0625em) {
}

How does that sound?

@KittyGiraudel
Copy link
Collaborator

Sass-MQ uses -.01em.

Or I suppose you could use calc() but I don't know if it works fine within Media Queries.

@media (min-width: calc(28em - 1px)) {
  /* ... */
}

@eduardoboucas
Copy link
Owner

Yes, I saw that Sass-MQ was doing this already and I quite liked it.

My thoughts behind creating a separate function (and a map) instead of hard-coding a value for ems was that it becomes easier and cleaner if we want to support other units, even though pixels and ems will most likely cover the majority of use cases.

calc() doesn't seem to work within a media query expression.

@KittyGiraudel
Copy link
Collaborator

My thoughts behind creating a separate function (and a map) instead of hard-coding a value for ems was that it becomes easier and cleaner if we want to support other units, even though pixels and ems will most likely cover the majority of use cases.

Makes sense. You might want to support rem as well.

@eduardoboucas
Copy link
Owner

I've created a new branch (add-intervals) with this stuff in it.

I'll keep doing some testing (feel free to pull and test on your end as well if you fancy it) and if nothing blows up I'll merge this in.

Thanks for the feedback guys!

@eduardoboucas
Copy link
Owner

Added in #19.

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

3 participants