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

Issue 29598558: Issue 5980 - Use correct browser logos for browser select on help.eyeo.com (Closed)

Created:
Nov. 5, 2017, 6:21 p.m. by ire
Modified:
Nov. 7, 2017, 7:25 a.m.
Reviewers:
juliandoucette
Base URL:
https://hg.adblockplus.org/help.eyeo.com
Visibility:
Public.

Description

Issue 5980 - Use correct browser logos for browser select on help.eyeo.com

Patch Set 1 #

Total comments: 1

Patch Set 2 : Odd opacity styles #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+591 lines, -4 lines) Patch
M globals/browsers.py View 1 chunk +7 lines, -3 lines 0 comments Download
M includes/browser-select.tmpl View 1 chunk +1 line, -1 line 0 comments Download
A static/img/png/chrome.png View Binary file 0 comments Download
A static/img/png/firefox.png View Binary file 0 comments Download
A static/img/png/ios.png View Binary file 0 comments Download
A static/img/png/maxthon.png View Binary file 0 comments Download
A static/img/png/msedge.png View Binary file 0 comments Download
A static/img/png/msie.png View Binary file 0 comments Download
A static/img/png/opera.png View Binary file 0 comments Download
A static/img/png/safari.png View Binary file 0 comments Download
A static/img/png/samsungBrowser.png View Binary file 0 comments Download
A static/img/png/yandexbrowser.png View Binary file 0 comments Download
A static/img/svg/chrome.svg View 1 chunk +227 lines, -0 lines 0 comments Download
A static/img/svg/firefox.svg View 1 chunk +135 lines, -0 lines 0 comments Download
A static/img/svg/ios.svg View 1 chunk +15 lines, -0 lines 2 comments Download
A static/img/svg/maxthon.svg View 1 chunk +16 lines, -0 lines 0 comments Download
A static/img/svg/msedge.svg View 1 chunk +14 lines, -0 lines 0 comments Download
A static/img/svg/msie.svg View 1 chunk +68 lines, -0 lines 0 comments Download
A static/img/svg/opera.svg View 1 chunk +27 lines, -0 lines 0 comments Download
A static/img/svg/safari.svg View 1 chunk +23 lines, -0 lines 0 comments Download
A static/img/svg/samsungBrowser.svg View 1 chunk +15 lines, -0 lines 0 comments Download
A static/img/svg/yandexbrowser.svg View 1 chunk +21 lines, -0 lines 0 comments Download
M static/scss/components/_browser-select.scss View 1 1 chunk +22 lines, -0 lines 0 comments Download

Messages

Total messages: 4
ire
Nov. 5, 2017, 6:22 p.m. (2017-11-05 18:22:18 UTC) #1
ire
The way Martin wants this to work is like this: All of the icons will ...
Nov. 5, 2017, 6:26 p.m. (2017-11-05 18:26:59 UTC) #2
juliandoucette
LGTM > I don't know if this is worth it, as IMO having the icons ...
Nov. 6, 2017, 10:39 a.m. (2017-11-06 10:39:35 UTC) #3
ire
Nov. 7, 2017, 7:23 a.m. (2017-11-07 07:23:59 UTC) #4
On 2017/11/06 10:39:35, juliandoucette wrote:
> LGTM
> 
> > I don't know if this is worth it, as IMO having the icons less opaque still
> > works and graying them out is an enhancement, not really a requirement. What
> do
> > you think?
> 
> I don't think that the old IE and FF fixes are necessary.

Alright then. I'll leave the fallback for IE6-9 since IE9 is supported, but I'll
remove the other for old Firefox versions since it seems more "hacky" to me.

https://codereview.adblockplus.org/29598558/diff/29598583/static/img/svg/ios.svg
File static/img/svg/ios.svg (right):

https://codereview.adblockplus.org/29598558/diff/29598583/static/img/svg/ios....
static/img/svg/ios.svg:1: <?xml version="1.0" encoding="UTF-8"?>
On 2017/11/06 10:39:35, juliandoucette wrote:
> NIT: This icon looks larger than the rest?

I think it's just the shape of it that makes it look slightly bigger. I don't
think the difference is really noticeable though (to me anyway, I don't really
see it)

Powered by Google App Engine
This is Rietveld