foundation / foundation-emails

Quickly create responsive HTML emails that work on any device and client. Even Outlook.
https://get.foundation/emails/docs/
MIT License
7.76k stars 1.09k forks source link

Opt-in responsive images #363

Open ClementParis016 opened 8 years ago

ClementParis016 commented 8 years ago

Right now, if you insert an <img> in your template, it gets the following CSS (Scss) on small screens:

@media only screen and (max-width: #{$global-breakpoint}) {
  table.body img {
    width: auto !important;
    height: auto !important;
  }
}

File: scss/components/_media-query.scss:9

Wouldn't it be more convenient that the above styles are applied only if you add a .responsive CSS class to your <img>? Because those damn !important make everything more difficult when you want a fixed size for an image, even on small screens.

I would kindly make a PR for this if you want :wink:

larsbo commented 8 years ago

@ClementParis016 I use max-width for fixed size images on small screens. But I didn't test it in many email clients so far.

ClementParis016 commented 8 years ago

@larsbo indeed, we could work with min-width/max-width and min-height/max-height, but apparently these are not well supported in Outlook (see https://www.campaignmonitor.com/css/).

rafibomb commented 8 years ago

Just to understand the use case, you want to have an option to make images stretch to 100% on the small breakpoint?

If so, then that makes sense - we'll need a naming pattern that is more contextual. How about .small-expanded?

rafibomb commented 8 years ago

Also a PR would be appreciated and accepted for this!

ClementParis016 commented 8 years ago

@rafibomb not exactly, the use case is not to have an image to be 100% width of its parent on small screen (even if this could be needed), but rather to allow images to have fixed dimensions.

For example, if you have an icon that need to be 14x14px whatever the screen dimensions are, even if you set these dimensions to your image as follows:

img.icon {
  width: 14px;
  height: 14px;
}

then it gets overrided by the framework's CSS (in scss/components/_media-query.scss:9):

@media only screen and (max-width: #{$global-breakpoint}) {
  table.body img {
    width: auto !important;
    height: auto !important;
  }
}

I'm not even sure the !important is needed here because img default styles come from scss\components\_normalize.scss:49:

img {
  outline: none;
  text-decoration: none;
  -ms-interpolation-mode: bicubic;
  width: auto;
  max-width: 100%;
  clear: both;
  display: block;
}

In the end, I'm not sure how to manage this, so what do you think about it?

ghost commented 8 years ago

I've found the only way to handle your test case when using retina images is to create a class for the image and create a media query for small explicitly overriding the table.body img class for small and adding the !important values after it. This is frustrating as it means you have to create a class for every image you need to do this to.

ClementParis016 commented 8 years ago

@coreyschaaf Retina images is exactly my use case. My images are twice their displayed dimensions, so I set the target dimensions as withand height attributes on the <img> tag, but the media query overrides these dimensions on small screens.

So, I think we should make this optional by adding a .responsive class to images that should be auto-sized on small screens because as you've said, it would be painful to have to override the media query for each image. But in the other hand, that could break existing layouts...

@rafibomb what do you think?

ClementParis016 commented 8 years ago

So, apparently this commit on the develop branch addresses the issue: a76d810f342df207ffa836d92f91e0deb1ed4273

With the removal of !important we can now define fixed dimensions using inline styles on the images:

<img src="/some/path/to/image.png" alt="" width="49" height="59" style="width: 49px; height: 59px;">
digitalmaster commented 7 years ago

Is it just me or should all responsive behavior be op-in by default? I too had this issue with fixed retina image being automatically affected by styles in _media-query.scss but that's just one unintended responsive behavior.

gojefferson commented 6 years ago

I really like the idea of opting into responsive images instead of having them all be responsive by default. +1 on that. 👍