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

Issue 4528719876063232: Issue 2213 - landing page for mobile beta launch (CMS) (Closed)

Created:
May 18, 2015, 3:54 p.m. by saroyanm
Modified:
May 29, 2015, 9:06 a.m.
CC:
Felix Dahlke, rossg
Visibility:
Public.

Description

Anwiki review: http://codereview.adblockplus.org/4814987935612928/ Related ticket: https://issues.adblockplus.org/ticket/2213

Patch Set 1 #

Total comments: 2

Patch Set 2 : required fix and pngout convertion of image #

Patch Set 3 : name strings #

Unified diffs Side-by-side diffs Delta from patch set Stats (+20 lines, -18 lines) Patch
M pages/adblock-browser.html View 1 2 2 chunks +20 lines, -18 lines 0 comments Download
A static/img/adblockbrowser_beta_promo.png View 1 Binary file 0 comments Download

Messages

Total messages: 10
saroyanm
Sebastian can you please have a look, actually initially I was thinking we are going ...
May 18, 2015, 3:58 p.m. (2015-05-18 15:58:39 UTC) #1
Sebastian Noack
Please process the image with pngout. http://codereview.adblockplus.org/4528719876063232/diff/5629499534213120/pages/adblock-browser.html File pages/adblock-browser.html (right): http://codereview.adblockplus.org/4528719876063232/diff/5629499534213120/pages/adblock-browser.html#newcode270 pages/adblock-browser.html:270: <input id="subscribe-textbox" type="email" ...
May 18, 2015, 4:08 p.m. (2015-05-18 16:08:15 UTC) #2
saroyanm
http://codereview.adblockplus.org/4528719876063232/diff/5629499534213120/pages/adblock-browser.html File pages/adblock-browser.html (right): http://codereview.adblockplus.org/4528719876063232/diff/5629499534213120/pages/adblock-browser.html#newcode270 pages/adblock-browser.html:270: <input id="subscribe-textbox" type="email" name="email" placeholder="{{s10 Your email address}}" /> ...
May 18, 2015, 4:51 p.m. (2015-05-18 16:51:10 UTC) #3
saroyanm
On 2015/05/18 16:08:15, Sebastian Noack wrote: > Please process the image with pngout. Done.
May 18, 2015, 4:51 p.m. (2015-05-18 16:51:24 UTC) #4
Sebastian Noack
LGTM
May 18, 2015, 4:53 p.m. (2015-05-18 16:53:58 UTC) #5
Wladimir Palant
Looks ok but we should really use more meaningful string identifiers than s8.
May 18, 2015, 6:01 p.m. (2015-05-18 18:01:52 UTC) #6
saroyanm
On 2015/05/18 18:01:52, Wladimir Palant wrote: > Looks ok but we should really use more ...
May 18, 2015, 6:22 p.m. (2015-05-18 18:22:39 UTC) #7
Wladimir Palant
LGTM
May 18, 2015, 6:30 p.m. (2015-05-18 18:30:13 UTC) #8
Sebastian Noack
LGTM
May 18, 2015, 6:35 p.m. (2015-05-18 18:35:23 UTC) #9
saroyanm
May 18, 2015, 7:35 p.m. (2015-05-18 19:35:16 UTC) #10
@Ross actually we had a review for the Adblock  Browser Landing Page, can you
please test the changes. 
Applying the delta diff with the patchset of number 1 should be enough.
The changes as you can see are small so nothing big is expected I have a quick
test with Chrome and IE 8 and it works okey.

Powered by Google App Engine
This is Rietveld