[Demo] Issue 4961 - Fix card group field alignment on acceptableads.com

[Demo] Issue 4961 - Fix card group field alignment on acceptableads.com

[ire, 2017-05-19 05:37:57]

https://codereview.adblockplus.org/29442559/diff/29442560/scss/main.scss
File scss/main.scss (right):

https://codereview.adblockplus.org/29442559/diff/29442560/scss/main.scss#newc...
scss/main.scss:53: $card__footer--height: 30px;
@julian Here is the main difference between the two methods. This one requires a
set height for the card footer

https://codereview.adblockplus.org/29442559/diff/29442560/scss/main.scss#newc...
scss/main.scss:65: @media (min-width: 800px) { width: 33%; }
@julian Another difference is that the breakpoints have to be set manually. So
they may not break at the exact optimal points based on a max/min width we want
each card to be.

[juliandoucette, 2017-05-23 11:52:00]

Thanks Ire! :)

Feedback below.

https://codereview.adblockplus.org/29442559/diff/29442560/scss/main.scss
File scss/main.scss (right):

https://codereview.adblockplus.org/29442559/diff/29442560/scss/main.scss#newc...
scss/main.scss:31: $card__header--height: 100px;
This is not ideal. We have a problem with long headers on aa.com.

https://codereview.adblockplus.org/29442559/diff/29442560/scss/main.scss#newc...
scss/main.scss:53: $card__footer--height: 30px;
On 2017/05/19 05:37:57, ire wrote:
> @julian Here is the main difference between the two methods. This one requires
a
> set height for the card footer

Acknowledged.

I think this is a reasonable limitation because we can control the content in a
button easier than the content in a title or description.

https://codereview.adblockplus.org/29442559/diff/29442560/scss/main.scss#newc...
scss/main.scss:65: @media (min-width: 800px) { width: 33%; }
On 2017/05/19 05:37:57, ire wrote:
> @julian Another difference is that the breakpoints have to be set manually. So
> they may not break at the exact optimal points based on a max/min width we
want
> each card to be.

Acknowledged.

This doesn't resize well :(

Ideally, the fallback solution should stack. If not, I think it should scroll.
Squished content is worse than not aligned content.

[ire, 2017-05-23 17:21:44]

https://codereview.adblockplus.org/29442559/diff/29442560/scss/main.scss
File scss/main.scss (right):

https://codereview.adblockplus.org/29442559/diff/29442560/scss/main.scss#newc...
scss/main.scss:31: $card__header--height: 100px;
On 2017/05/23 11:52:00, juliandoucette wrote:
> This is not ideal. We have a problem with long headers on http://aa.com.

Noted. I'm not sure, but there may be a way with Grid to have a fluid height.
I'll do a bit more research on that.

https://codereview.adblockplus.org/29442559/diff/29442560/scss/main.scss#newc...
scss/main.scss:53: $card__footer--height: 30px;
On 2017/05/23 11:52:00, juliandoucette wrote:
> On 2017/05/19 05:37:57, ire wrote:
> > @julian Here is the main difference between the two methods. This one
requires
> a
> > set height for the card footer
> 
> Acknowledged.
> 
> I think this is a reasonable limitation because we can control the content in
a
> button easier than the content in a title or description.

Acknowledged.

https://codereview.adblockplus.org/29442559/diff/29442560/scss/main.scss#newc...
scss/main.scss:65: @media (min-width: 800px) { width: 33%; }
On 2017/05/23 11:52:00, juliandoucette wrote: 
> This doesn't resize well :(
> 
> Ideally, the fallback solution should stack. If not, I think it should scroll.
> Squished content is worse than not aligned content.

It actually should be stacked. Maybe there's an issue, I will check it out to
make sure.

[ire, 2017-05-25 21:30:57]

On 2017/05/23 17:21:44, ire wrote:
> On 2017/05/23 11:52:00, juliandoucette wrote: 
> > This doesn't resize well :(
> > 
> > Ideally, the fallback solution should stack. If not, I think it should
scroll.
> > Squished content is worse than not aligned content.
> 
> It actually should be stacked. Maybe there's an issue, I will check it out to
> make sure.

After a bunch of research, I realised the reason it wasn't working was because
`display: table-cell` only allows one row. This meant that we couldn't have each
card stacked on each other on smaller screen sizes. So, I had to rework the
fallback method to use floats. Unfortunately, this meant that all three sections
of the card (header, content, footer), need a fixed height. I believe this is
pretty much what we already have implemented on the site right now. As far as I
can tell, this might be the only solution (besides the Grid Layout enhancement).

[juliandoucette, 2017-05-30 10:30:07]

> After a bunch of research, I realised the reason it wasn't working was because
> `display: table-cell` only allows one row. This meant that we couldn't have
each
> card stacked on each other on smaller screen sizes. So, I had to rework the
> fallback method to use floats. Unfortunately, this meant that all three
sections
> of the card (header, content, footer), need a fixed height. I believe this is
> pretty much what we already have implemented on the site right now. As far as
I
> can tell, this might be the only solution (besides the Grid Layout
enhancement).

LGTM

This pure CSS fallback and enhancement seems to be the best we can do without
JavaScript. And I think we can get away with a fixed height title on cards that
have title and description on aa.com.

[ire, 2017-05-31 08:26:30]

On 2017/05/30 10:30:07, juliandoucette wrote: 
> LGTM
> 
> This pure CSS fallback and enhancement seems to be the best we can do without
> JavaScript. And I think we can get away with a fixed height title on cards
that
> have title and description on http://aa.com.

Great. So I will implement this for the actual issue and upload a separate code
review.