Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(804)

Issue 29442559: [Demo] Issue 4961 - Fix card group field alignment on acceptableads.com (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months ago by ire
Modified:
2 months, 3 weeks ago
Reviewers:
juliandoucette
CC:
saroyanm
Visibility:
Public.

Description

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

Patch Set 1 #

Total comments: 8

Patch Set 2 : Change fallback method to floats #

Unified diffs Side-by-side diffs Delta from patch set Stats (+69 lines, -32 lines) Patch
M css/main.css View 1 1 chunk +19 lines, -11 lines 0 comments Download
M index.html View 1 1 chunk +19 lines, -3 lines 0 comments Download
M scss/main.scss View 1 2 chunks +30 lines, -11 lines 0 comments Download
M yarn.lock View 1 1 chunk +1 line, -7 lines 0 comments Download

Messages

Total messages: 7
ire
3 months ago (2017-05-19 05:30:45 UTC) #1
ire
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#newcode53 scss/main.scss:53: $card__footer--height: 30px; @julian Here is the main difference between ...
3 months ago (2017-05-19 05:37:57 UTC) #2
juliandoucette
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#newcode31 scss/main.scss:31: $card__header--height: 100px; This is ...
3 months ago (2017-05-23 11:52:00 UTC) #3
ire
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#newcode31 scss/main.scss:31: $card__header--height: 100px; On 2017/05/23 11:52:00, juliandoucette wrote: > This ...
3 months ago (2017-05-23 17:21:44 UTC) #4
ire
On 2017/05/23 17:21:44, ire wrote: > On 2017/05/23 11:52:00, juliandoucette wrote: > > This doesn't ...
2 months, 4 weeks ago (2017-05-25 21:30:57 UTC) #5
juliandoucette
> After a bunch of research, I realised the reason it wasn't working was because ...
2 months, 3 weeks ago (2017-05-30 10:30:07 UTC) #6
ire
2 months, 3 weeks ago (2017-05-31 08:26:30 UTC) #7
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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5