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.77k stars 1.09k forks source link

Border Radius with Color on Container #427

Open iamsankalp opened 8 years ago

iamsankalp commented 8 years ago

Seems like using border radius and a border with some color on 'container' given via a class is not working as expected. The edges are rounded but the colored border is not.

image

How can we reproduce this bug? 1) Give a class to container- let's say: <container class="content"> 2) .content{ border-radius: 10px; border: 1px solid red; } What did you expect to happen? Border color along with radius. Like this image: image

What happened instead? Like the image attached in the very beginning. What email clients does this happen in? Tested in my browser only so far.

PS: I believe border radius is not supported well across all clients, just that it's presence doesn't break anything so it's fine - to whatever clients it can show up. :)

rafibomb commented 8 years ago

This seems like a real challenge with tables. Same happens for buttons. To add a border radius the border needs to be removed. Are you aware of a workaround on this?

iamsankalp commented 8 years ago

@rafibomb Yeah, I did find a workaround. Give border-collapse: separate; to the same class. Just not sure if it's technically a good way since there are many border-collapse defined. What do you think?

rafibomb commented 8 years ago

Good find! Not sure what the implications are for changing that property, but if it passes a Litmus test on Outlook 2000-16 and Gmail it's a good solution.

@tdhartwick Any thoughts on the border-collapse solution here?

rafibomb commented 8 years ago

@iamsankalp Is this something you can add as a PR to the 2.2 branch for this? We can flush it out in testing but I think it's a good solution!

rafibomb commented 8 years ago

Hi @iamsankalp Would love to get this into 2.2 coming soon - are you able to submit a PR so we'll test it?

iamsankalp commented 8 years ago

@rafibomb - sorry, i missed the previous message completely. I'll try doing a PR.

iamsankalp commented 8 years ago

Hey @rafibomb I believe the solution doesn't solve it for Outlook 2010, 2013 as i could check via Litmus.

Holding off the PR for now.

image

rafibomb commented 8 years ago

Hi @iamsankalp That's ok! We know it wont work for Outlook, maybe just 2016. I have a litmus test here that shows it works for other clients https://litmus.com/checklist/emails/public/3d043d3

Would still love to see your PR and we can collaborate to get it across the line!

rafibomb commented 8 years ago

Saw the PR - I was thinking that someone will add a class of .radius to the container to add the $container-radius variable to it. Maybe default it to 0.

// adds radius to container
table.container.radius {
  border-radius: $container-radius;
  border-collapse: separate;
}

Thoughts?

iamsankalp commented 8 years ago

Nice solution, @rafibomb!

So, we'll create a variable $container-radius. But the class name stays the same - i.e. radius. Um... should the class name also be different so that relevance is clear? Plus, the fact that button and container-radius values may not necessarily be same. I'm overthinking maybe.

Sankalp

rafibomb commented 8 years ago

In Foundation for Sites, .radius is a modifier class that can be added to certain components like callouts and buttons. We can keep it the same and document it

rafibomb commented 8 years ago

Ok added it here - https://github.com/zurb/foundation-emails/commit/17b8905bb4e018a4149983f7bd0050a40fe4afb7 Are you able to update the docs?

rafibomb commented 7 years ago

I think this is just open to update the docs! @iamsankalp Are you able to add it for the release next Thursday?

DanielRuf commented 4 years ago

Any updates on this?