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

Issue 29695555: Fixes #6 - Add media mention banner (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 months, 1 week ago by ire
Modified:
7 months, 3 weeks ago
Base URL:
https://hg.adblockplus.org/web.adblockplus.org
Visibility:
Public.

Description

Fixes #6 - Add media mention banner Issue - https://gitlab.com/eyeo/web.adblockplus.org/issues/6 Branch: index_page

Patch Set 1 #

Total comments: 36

Patch Set 2 : Addressed comments #2 #

Patch Set 3 : Optimised images #

Patch Set 4 : Updated max-width #

Total comments: 4

Patch Set 5 : Undo change to full-width layout #

Unified diffs Side-by-side diffs Delta from patch set Stats (+299 lines, -114 lines) Patch
M includes/index.tmpl View 1 2 3 4 3 chunks +58 lines, -1 line 0 comments Download
M static/css/index.css View 1 2 3 4 8 chunks +223 lines, -100 lines 0 comments Download
M static/css/index-desktop.css View 1 chunk +0 lines, -13 lines 0 comments Download
M static/css/index-mobile.css View 1 1 chunk +5 lines, -0 lines 0 comments Download
A static/img/external-icon.png View Binary file 0 comments Download
A static/img/external-icon.svg View 1 chunk +13 lines, -0 lines 0 comments Download
A static/img/media/business-insider.png View 1 2 Binary file 0 comments Download
A static/img/media/business-insider-2x.png View 1 2 Binary file 0 comments Download
A static/img/media/mediapost.png View 1 2 Binary file 0 comments Download
A static/img/media/mediapost-2x.png View 1 2 Binary file 0 comments Download
A static/img/media/nyt.png View 1 2 Binary file 0 comments Download
A static/img/media/nyt-2x.png View 1 2 Binary file 0 comments Download
A static/img/media/techcrunch.png View 1 2 Binary file 0 comments Download
A static/img/media/techcrunch-2x.png View 1 2 Binary file 0 comments Download
A static/img/media/wsj.png View 1 2 Binary file 0 comments Download
A static/img/media/wsj-2x.png View 1 2 Binary file 0 comments Download

Messages

Total messages: 10
ire
8 months, 1 week ago (2018-02-12 11:26:38 UTC) #1
juliandoucette
Thanks Ire! https://codereview.adblockplus.org/29695555/diff/29695556/includes/index.tmpl File includes/index.tmpl (right): https://codereview.adblockplus.org/29695555/diff/29695556/includes/index.tmpl#newcode73 includes/index.tmpl:73: <div id="content"> Did you intend to include ...
7 months, 4 weeks ago (2018-02-19 14:15:57 UTC) #2
ire
Thanks Julian! New patch uploaded https://codereview.adblockplus.org/29695555/diff/29695556/includes/index.tmpl File includes/index.tmpl (right): https://codereview.adblockplus.org/29695555/diff/29695556/includes/index.tmpl#newcode73 includes/index.tmpl:73: <div id="content"> On 2018/02/19 ...
7 months, 4 weeks ago (2018-02-20 10:49:05 UTC) #3
juliandoucette
LGTM + NITs (Nothing blocking AFAICT - provided we investigate NITs and address the #container ...
7 months, 4 weeks ago (2018-02-21 11:22:54 UTC) #4
ire
Thanks Julian, will commit & push now https://codereview.adblockplus.org/29695555/diff/29695556/includes/index.tmpl File includes/index.tmpl (right): https://codereview.adblockplus.org/29695555/diff/29695556/includes/index.tmpl#newcode73 includes/index.tmpl:73: <div id="content"> ...
7 months, 3 weeks ago (2018-02-22 08:43:53 UTC) #5
ire
On 2018/02/22 08:43:53, ire wrote: > Thanks Julian, will commit & push now I haven't ...
7 months, 3 weeks ago (2018-02-22 08:55:13 UTC) #6
juliandoucette
Thanks Ire! Still LGTM + NITs. https://codereview.adblockplus.org/29695555/diff/29695556/includes/index.tmpl File includes/index.tmpl (right): https://codereview.adblockplus.org/29695555/diff/29695556/includes/index.tmpl#newcode193 includes/index.tmpl:193: <small>(Links open in ...
7 months, 3 weeks ago (2018-02-23 13:46:19 UTC) #7
ire
Thanks https://codereview.adblockplus.org/29695555/diff/29695556/includes/index.tmpl File includes/index.tmpl (right): https://codereview.adblockplus.org/29695555/diff/29695556/includes/index.tmpl#newcode193 includes/index.tmpl:193: <small>(Links open in a new window)</small> On 2018/02/23 ...
7 months, 3 weeks ago (2018-02-26 08:32:12 UTC) #8
juliandoucette
One more thing: It's hard to see the external window icon over top of the ...
7 months, 3 weeks ago (2018-02-26 08:39:11 UTC) #9
ire
7 months, 3 weeks ago (2018-02-26 08:51:11 UTC) #10
On 2018/02/26 08:39:11, juliandoucette wrote:
> One more thing:
> 
> It's hard to see the external window icon over top of the faded logos in this
> section. I think we should adjust the opacity and/or set a solid black
> background behind the icon itself.
> 
> This may be done separately.

Already pushed before I saw this :/

Will handle it separately
Sign in to reply to this message.

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