Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29438582: Issue 5135 - Reduce font size and padding on smaller screens on acceptableads.com (Closed)

Created:
May 16, 2017, 4:26 a.m. by ire
Modified:
July 7, 2017, 8:29 a.m.
Reviewers:
juliandoucette
CC:
saroyanm, Thomas Greiner
Base URL:
https://hg.adblockplus.org/web.acceptableads.com
Visibility:
Public.

Description

Issue 5135 - Reduce font size and padding on smaller screens on acceptableads.com

Patch Set 1 #

Total comments: 22

Patch Set 2 : Remove @media screen, Resize h2, Keep icon size #

Total comments: 2

Patch Set 3 : More thorough resizing #

Total comments: 13

Patch Set 4 : Handle responsive styling #

Total comments: 24

Patch Set 5 : Implement container classes in scss #

Total comments: 7

Patch Set 6 : Refactor and use .section #

Total comments: 4

Patch Set 7 : Fix alignment of hr under headings #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+374 lines, -34 lines) Patch
M includes/about/acceptable-ads-are.md View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M includes/index/style.html View 1 2 1 chunk +1 line, -1 line 0 comments Download
M includes/solutions/style.html View 1 2 4 1 chunk +1 line, -1 line 0 comments Download
M includes/tool-certification/style.html View 1 2 3 4 1 chunk +18 lines, -0 lines 0 comments Download
M includes/tool-certification/summary.md View 1 2 3 4 1 chunk +2 lines, -1 line 0 comments Download
M pages/about/best-practices.md View 1 2 3 4 5 1 chunk +3 lines, -1 line 0 comments Download
M pages/about/index.html View 1 2 3 4 5 1 chunk +3 lines, -3 lines 0 comments Download
M pages/blog/index.md View 1 2 3 4 5 1 chunk +4 lines, -1 line 0 comments Download
M pages/committee/apply.md View 1 2 3 4 5 2 chunks +3 lines, -2 lines 0 comments Download
M pages/committee/index.md View 1 2 3 4 5 1 chunk +6 lines, -1 line 0 comments Download
M pages/committee/members.tmpl View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M pages/solutions/ad-networks.md View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M pages/solutions/ad-tech-suppliers.md View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M pages/solutions/advertisers.md View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M pages/solutions/get-whitelisted/index.md View 1 2 3 4 5 1 chunk +3 lines, -1 line 0 comments Download
M pages/solutions/publishers.md View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M pages/tool-certification/index.md View 1 2 3 4 1 chunk +2 lines, -2 lines 0 comments Download
M static/css/main.css View 1 2 3 4 5 6 11 chunks +151 lines, -5 lines 0 comments Download
M static/scss/components/_cards.scss View 1 2 1 chunk +20 lines, -0 lines 0 comments Download
M static/scss/components/_footnotes.scss View 1 2 1 chunk +5 lines, -0 lines 0 comments Download
M static/scss/components/_groups.scss View 1 2 3 4 1 chunk +23 lines, -2 lines 0 comments Download
M static/scss/content/_buttons.scss View 1 2 3 4 1 chunk +20 lines, -0 lines 0 comments Download
M static/scss/content/_reset.scss View 1 2 3 4 5 6 1 chunk +13 lines, -2 lines 2 comments Download
M static/scss/content/_typography.scss View 1 2 3 4 5 6 4 chunks +69 lines, -1 line 1 comment Download
M static/scss/layout/_footer.scss View 1 2 1 chunk +17 lines, -0 lines 0 comments Download

Messages

Total messages: 32
ire
May 16, 2017, 4:26 a.m. (2017-05-16 04:26:37 UTC) #1
juliandoucette
(This is going to take me a little while.)
May 16, 2017, 3:51 p.m. (2017-05-16 15:51:33 UTC) #2
juliandoucette
https://codereview.adblockplus.org/29438582/diff/29438583/pages/about/best-practices.md File pages/about/best-practices.md (right): https://codereview.adblockplus.org/29438582/diff/29438583/pages/about/best-practices.md#newcode18 pages/about/best-practices.md:18: <div class="bg-accent section" markdown="1"> Is there a reason why ...
May 16, 2017, 7:07 p.m. (2017-05-16 19:07:35 UTC) #3
ire
https://codereview.adblockplus.org/29438582/diff/29438583/pages/about/best-practices.md File pages/about/best-practices.md (right): https://codereview.adblockplus.org/29438582/diff/29438583/pages/about/best-practices.md#newcode18 pages/about/best-practices.md:18: <div class="bg-accent section" markdown="1"> On 2017/05/16 19:07:34, juliandoucette wrote: ...
May 16, 2017, 9:32 p.m. (2017-05-16 21:32:28 UTC) #4
juliandoucette
Thanks Ire! Detail: I see you have been replying "ACK". This is not a problem. ...
May 17, 2017, 1:09 p.m. (2017-05-17 13:09:11 UTC) #5
ire
On 2017/05/17 13:09:11, juliandoucette wrote: > Detail: I see you have been replying "ACK". This ...
May 18, 2017, 12:25 a.m. (2017-05-18 00:25:28 UTC) #6
ire
https://codereview.adblockplus.org/29438582/diff/29438583/static/scss/components/_cards.scss File static/scss/components/_cards.scss (right): https://codereview.adblockplus.org/29438582/diff/29438583/static/scss/components/_cards.scss#newcode50 static/scss/components/_cards.scss:50: @media screen and (max-width: $tablet-breakpoint) On 2017/05/17 13:09:11, juliandoucette ...
May 18, 2017, 12:27 a.m. (2017-05-18 00:27:14 UTC) #7
juliandoucette
> Sure. Do you mean as a separate codereview (i.e. a different repo with the ...
May 18, 2017, 8:45 a.m. (2017-05-18 08:45:12 UTC) #8
juliandoucette
Thanks Ire! :) A few high level points: - Don't forget about removing @media screen ...
May 30, 2017, 2:22 p.m. (2017-05-30 14:22:57 UTC) #9
ire
On 2017/05/30 14:22:57, juliandoucette wrote: > - Don't forget about removing @media screen Noted. > ...
May 30, 2017, 5:44 p.m. (2017-05-30 17:44:48 UTC) #10
ire
https://codereview.adblockplus.org/29438582/diff/29438583/static/scss/components/_cards.scss File static/scss/components/_cards.scss (right): https://codereview.adblockplus.org/29438582/diff/29438583/static/scss/components/_cards.scss#newcode45 static/scss/components/_cards.scss:45: img.card-icon, On 2017/05/30 14:22:56, juliandoucette wrote: > I don't ...
May 30, 2017, 5:44 p.m. (2017-05-30 17:44:59 UTC) #11
juliandoucette
I answered your question below :) https://codereview.adblockplus.org/29438582/diff/29438583/static/scss/components/_groups.scss File static/scss/components/_groups.scss (right): https://codereview.adblockplus.org/29438582/diff/29438583/static/scss/components/_groups.scss#newcode26 static/scss/components/_groups.scss:26: @media screen and ...
June 7, 2017, 6:05 p.m. (2017-06-07 18:05:29 UTC) #12
ire
On 2017/06/07 18:05:29, juliandoucette wrote: > You can just pull and rebase if there is ...
June 8, 2017, 12:53 p.m. (2017-06-08 12:53:25 UTC) #13
juliandoucette
I still don't think we are going far enough... - Look at the homepage on ...
June 9, 2017, 2:08 p.m. (2017-06-09 14:08:27 UTC) #14
ire
> I still don't think we are going far enough... Noted. For some reason I ...
June 14, 2017, 3:41 p.m. (2017-06-14 15:41:13 UTC) #15
ire
I've gone through the site more thoroughly and have resized the font and spacing in ...
June 15, 2017, 11:16 a.m. (2017-06-15 11:16:23 UTC) #16
ire
On 2017/06/15 11:16:23, ire wrote: > https://codereview.adblockplus.org/29438582/diff/29466580/pages/committee/index.md#newcode29 > pages/committee/index.md:29: --- > This page was missing ...
June 15, 2017, 5:26 p.m. (2017-06-15 17:26:55 UTC) #17
juliandoucette
Thanks Ire! This one is really close. https://codereview.adblockplus.org/29438582/diff/29466580/pages/committee/index.md File pages/committee/index.md (right): https://codereview.adblockplus.org/29438582/diff/29466580/pages/committee/index.md#newcode29 pages/committee/index.md:29: --- On ...
June 16, 2017, 6:47 p.m. (2017-06-16 18:47:53 UTC) #18
ire
Thanks! I think you missed this comment/question I made, so copying it here again: > ...
June 19, 2017, 11:51 a.m. (2017-06-19 11:51:04 UTC) #19
juliandoucette
> Thanks! I think you missed this comment/question I made, so copying it here again: ...
June 20, 2017, 4:38 p.m. (2017-06-20 16:38:36 UTC) #20
ire
> Sorry about that - this is a tough question. No worries. Yes it is! ...
June 22, 2017, 3:41 p.m. (2017-06-22 15:41:44 UTC) #21
juliandoucette
The latest patch does not apply. Please update/rebase.
June 26, 2017, 10:19 a.m. (2017-06-26 10:19:08 UTC) #22
juliandoucette
I have just skimmed the latest patchset. Here are my first impressions. https://codereview.adblockplus.org/29438582/diff/29471582/includes/about/style.html File includes/about/style.html ...
June 26, 2017, 10:31 a.m. (2017-06-26 10:31:22 UTC) #23
ire
Done :) https://codereview.adblockplus.org/29438582/diff/29471582/includes/about/style.html File includes/about/style.html (right): https://codereview.adblockplus.org/29438582/diff/29471582/includes/about/style.html#newcode1 includes/about/style.html:1: <style> On 2017/06/26 10:31:20, juliandoucette wrote: > ...
June 27, 2017, 8:36 a.m. (2017-06-27 08:36:33 UTC) #24
juliandoucette
Thanks Ire :) I'm a little tired. Please forgive me if I missed something obvious ...
July 3, 2017, 11:12 p.m. (2017-07-03 23:12:41 UTC) #25
ire
https://codereview.adblockplus.org/29438582/diff/29475555/static/scss/content/_typography.scss File static/scss/content/_typography.scss (right): https://codereview.adblockplus.org/29438582/diff/29475555/static/scss/content/_typography.scss#newcode160 static/scss/content/_typography.scss:160: .p-container On 2017/07/03 23:12:38, juliandoucette wrote: > I don't ...
July 4, 2017, 11:03 a.m. (2017-07-04 11:03:13 UTC) #26
juliandoucette
(Unspoken: This looks good to me; otherwise.) https://codereview.adblockplus.org/29438582/diff/29475555/static/scss/content/_typography.scss File static/scss/content/_typography.scss (right): https://codereview.adblockplus.org/29438582/diff/29475555/static/scss/content/_typography.scss#newcode160 static/scss/content/_typography.scss:160: .p-container On ...
July 4, 2017, 3:33 p.m. (2017-07-04 15:33:08 UTC) #27
ire
https://codereview.adblockplus.org/29438582/diff/29475555/static/scss/content/_typography.scss File static/scss/content/_typography.scss (right): https://codereview.adblockplus.org/29438582/diff/29475555/static/scss/content/_typography.scss#newcode160 static/scss/content/_typography.scss:160: .p-container On 2017/07/04 15:33:08, juliandoucette wrote: > On 2017/07/04 ...
July 5, 2017, 10:33 a.m. (2017-07-05 10:33:10 UTC) #28
ire
https://codereview.adblockplus.org/29438582/diff/29475555/static/scss/content/_typography.scss File static/scss/content/_typography.scss (right): https://codereview.adblockplus.org/29438582/diff/29475555/static/scss/content/_typography.scss#newcode160 static/scss/content/_typography.scss:160: .p-container On 2017/07/05 10:33:09, ire wrote: > On 2017/07/04 ...
July 5, 2017, 10:53 a.m. (2017-07-05 10:53:50 UTC) #29
juliandoucette
https://codereview.adblockplus.org/29438582/diff/29480584/static/scss/content/_reset.scss File static/scss/content/_reset.scss (left): https://codereview.adblockplus.org/29438582/diff/29480584/static/scss/content/_reset.scss#oldcode89 static/scss/content/_reset.scss:89: h2 + .row, I was wondering why there was ...
July 5, 2017, 1:28 p.m. (2017-07-05 13:28:34 UTC) #30
ire
https://codereview.adblockplus.org/29438582/diff/29480584/static/scss/content/_reset.scss File static/scss/content/_reset.scss (left): https://codereview.adblockplus.org/29438582/diff/29480584/static/scss/content/_reset.scss#oldcode89 static/scss/content/_reset.scss:89: h2 + .row, On 2017/07/05 13:28:33, juliandoucette wrote: > ...
July 6, 2017, 10:31 a.m. (2017-07-06 10:31:39 UTC) #31
juliandoucette
July 6, 2017, 1:15 p.m. (2017-07-06 13:15:49 UTC) #32
LGTM

https://codereview.adblockplus.org/29438582/diff/29481594/static/scss/content...
File static/scss/content/_reset.scss (right):

https://codereview.adblockplus.org/29438582/diff/29481594/static/scss/content...
static/scss/content/_reset.scss:89: // Simulate collapsable top-margin on first
paragraph after page heading
On 2017/07/06 10:31:38, ire wrote:
> I added this comment here until we decide to revisit this issue so it's clear
> why the style is here. 

Acknowledged.

Powered by Google App Engine
This is Rietveld