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

Option/variable to use px but convert to em #93

Closed
gunnx opened this issue Jul 29, 2015 · 25 comments
Closed

Option/variable to use px but convert to em #93

gunnx opened this issue Jul 29, 2015 · 25 comments

Comments

@gunnx
Copy link

gunnx commented Jul 29, 2015

Sometimes you want to write your breakpoints in px but have them convert to em for the actual media query output

@KittyGiraudel
Copy link
Collaborator

Do you have a use case?

Le mer 29 juil. 2015 14:28, Keith Gunn notifications@github.com a écrit :

Sometimes you want to write your breakpoints in px but have them convert
to em for the actual media query output


Reply to this email directly or view it on GitHub
#93.

@gunnx
Copy link
Author

gunnx commented Jul 29, 2015

I've been using another mixin library mq-sass that does this by default, though I prefer it to be a configurable option.

@eduardoboucas
Copy link
Owner

As a side note, have you seen this?

@gunnx
Copy link
Author

gunnx commented Jul 29, 2015

I've read a few articles for the pros and cons, a lot will depend on usage. Its simple enough for me to convert to EM before setting breakpoints.

@eduardoboucas
Copy link
Owner

So if you want to output your media queries in em, why do you want to define them as px? Is it just to be cleaner on the eye, or is there any other reason?

@hinok
Copy link
Contributor

hinok commented Jul 29, 2015

@gunnx What about this approach? Use em-calc function, just define it somewhere earlier

/// Function converts / calculates px to em
/// @param {number} $size - Width in px
/// @param {number} $base - Base size in px, defaults 16px
/// @return {number} Width in em
@function em-calc($size, $base: 16px) {
  @return $size / $base * 1em;
}

$breakpoints: (
  phone: em-calc(640px),
  tablet: em-calc(768px),
  desktop: em-calc(1024px)
);

@gunnx
Copy link
Author

gunnx commented Jul 29, 2015

Yes that's basically what I was trying out 👍

@eduardoboucas
Copy link
Owner

This is a valid use case, but I think this would be a good case for an include-media plugin (like we did with this one), rather than including this feature in the core.

A very simple plugin could be something like this rough SassMeister gist.

@KittyGiraudel
Copy link
Collaborator

Keep it simple:

// Plugin starts here

@each $breakpoint, $value in $breakpoints {
  $breakpoints: map-merge($breakpoints, ($breakpoint: ($value / 16px * 1em)));
}

// Plugin ends here

You could also wrap it in an im-emify-breakpoints(..) mixin if you prefer.

By the way, I would like to see it as a plugin. Sounds good. :)

@tedw
Copy link

tedw commented Jun 6, 2016

FYI, it's best to always use em in media queries to support users who change their default font size:
http://zellwk.com/blog/media-query-units/

@mrmartineau
Copy link

@hugogiraudel @eduardoboucas any news on this? the bugs outlined in the post above seem like @hugogiraudel's fix above should make it into core or the plugin he speaks of.

@KittyGiraudel
Copy link
Collaborator

Ping @eduardoboucas.

@eduardoboucas
Copy link
Owner

I still don't see why this should go into core, as there is currently nothing stopping you from writing media queries in ems with include-media. Can you elaborate on your requirements, @mrmartineau?

@mrmartineau
Copy link

mrmartineau commented Oct 12, 2016

I don't mind if it doesn't go into core, as long as there's a plugin that can be added to get to the same outcome. If you read the article linked by @tedw you will see that both Safari and Mobile Safari have bugs when media-queries are defined in px and the user has changed their default font-size. I am happy to make the plugin but wouldn't know how to atm.

@eduardoboucas
Copy link
Owner

eduardoboucas commented Oct 12, 2016

I'm not arguing against using em instead of px in media queries. What I'm saying is, if you do this:

$breakpoints: (
  'small': 18em,
  'medium': 37em,
  'large': 75em
);

... you achieve what you're after. So if I understand correctly, what you're looking for is a convenience method to convert px to em, which I think is outside the scope of this library.

Does that make sense?

@mrmartineau
Copy link

Yes, sorry I meant to say that. It is for convenience, and lets face it, laziness, that I want to be able to define the breakpoints in px and then have it convert to em. Having said all this, in my framework we have a function to convert px to em which could be used inside the $breakpoints map, like so:

$breakpoints: (
  'small': em(500, 16),
  'medium': em(750, 16),
  'large': em(1000, 16)
);

I have kind of talked myself out of this 'fix' 😂 but it still might be useful to have the plugin..

@eduardoboucas
Copy link
Owner

eduardoboucas commented Oct 12, 2016

How about this?

@function im-to-em($breakpoints, $base-value: 16px) {
  $new-breakpoints: ();

  @each $name, $value in $breakpoints {
    $em-value: ($value / $base-value) * 1em;
    $new-breakpoints: map-merge($new-breakpoints, ($name: $em-value));
  }

  @return $new-breakpoints;
}

$breakpoints: im-to-em((
  'small': 300px,
  'medium': 600px,
  'large': 1200px
));

@KittyGiraudel
Copy link
Collaborator

Agree with @eduardoboucas. Converting pixels to em is out of the scope of this library.

@eduardoboucas
Copy link
Owner

(Thanks for the edit @hugogiraudel 😅 )

@mrmartineau
Copy link

that's fair enough, it is such a powerful lib that it is nice to be able to add things on top. Thanks @eduardoboucas for the 'plugin' - super cool of you! 👍

@eduardoboucas
Copy link
Owner

Glad to help! 🎉

Will create a Gist and add to the README, in case anyone else is looking for the same.

@mrmartineau
Copy link

thank you very much

@mrmartineau
Copy link

@eduardoboucas strangely enough, I have just tried your plugin and my suggested px-to-em function and they both generate completely wrong values. It is more than likely that the calc is screwed rather than include-media but I will try to create a test case to see what's actually going on.

@eduardoboucas
Copy link
Owner

You can use this gist on Sassmeister: http://www.sassmeister.com/gist/2236c831f75357005a4f16bd58da60c3

@mrmartineau
Copy link

thank you, I am just looking at Sassmeister now. I think something screwy must be going on in my framework that is breaking this. Thanks again

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

6 participants