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

Issue 29709593: Noissue - Lower opacity of media images on hover on abp.org homepage (Closed)

Created:
Feb. 26, 2018, 8:55 a.m. by ire
Modified:
Feb. 28, 2018, 8:46 a.m.
Reviewers:
juliandoucette
Base URL:
https://hg.adblockplus.org/web.adblockplus.org
Visibility:
Public.

Description

Noissue - Lower opacity of media images on hover on abp.org homepage

Patch Set 1 #

Patch Set 2 : HighPDI icon and solid black background color #

Total comments: 4

Patch Set 3 : Addressed NITs #

Unified diffs Side-by-side diffs Delta from patch set Stats (+305 lines, -114 lines) Patch
M includes/index.tmpl View 1 2 3 chunks +58 lines, -1 line 0 comments Download
M static/css/index.css View 1 2 8 chunks +229 lines, -100 lines 0 comments Download
M static/css/index-desktop.css View 1 2 1 chunk +0 lines, -13 lines 0 comments Download
M static/css/index-mobile.css View 1 2 1 chunk +5 lines, -0 lines 0 comments Download
A static/img/external-icon.png View 1 2 Binary file 0 comments Download
A static/img/external-icon.svg View 1 2 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: 5
ire
Feb. 26, 2018, 8:55 a.m. (2018-02-26 08:55:07 UTC) #1
juliandoucette
Still looks blurry to me. This is probably not helped by the fact that the ...
Feb. 26, 2018, 1:10 p.m. (2018-02-26 13:10:22 UTC) #2
ire
On 2018/02/26 13:10:22, juliandoucette wrote: > Still looks blurry to me. This is probably not ...
Feb. 26, 2018, 6:46 p.m. (2018-02-26 18:46:08 UTC) #3
juliandoucette
LGTM + NITs > The opacity is already at 20% I know. I meant that ...
Feb. 27, 2018, 12:57 p.m. (2018-02-27 12:57:05 UTC) #4
ire
Feb. 28, 2018, 8:45 a.m. (2018-02-28 08:45:18 UTC) #5
Thanks Julian!

On 2018/02/27 12:57:05, juliandoucette wrote:
> I know. I meant that the opacity can be higher (the logo can be more visible)
if
> the external link icon has a solid black background behind it. Please use your
> best judgement.

Done

https://codereview.adblockplus.org/29709593/diff/29709623/static/css/index.css
File static/css/index.css (right):

https://codereview.adblockplus.org/29709593/diff/29709623/static/css/index.cs...
static/css/index.css:749: padding: 1em;
On 2018/02/27 12:57:04, juliandoucette wrote:
> NIT/Suggest: I think round edges would be nicer than sharp edges. This could
be
> accomplished via border-radius.

Done.

Powered by Google App Engine
This is Rietveld