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

Issue 29328733: Issue 2018 - Optimized first-run page for smaller screens (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 8 months ago by Thomas Greiner
Modified:
4 years, 7 months ago
CC:
Felix Dahlke
Visibility:
Public.

Description

This review also contains minor CSS coding style inconsistency improvements and a fix for feature text in RTL environment.

Patch Set 1 #

Total comments: 3

Patch Set 2 : #

Total comments: 11

Patch Set 3 : #

Total comments: 5

Patch Set 4 : #

Total comments: 2

Patch Set 5 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+104 lines, -61 lines) Patch
M firstRun.html View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M firstRun.js View 1 chunk +3 lines, -0 lines 0 comments Download
M skin/firstRun.css View 1 2 3 4 18 chunks +99 lines, -60 lines 0 comments Download

Messages

Total messages: 12
Thomas Greiner
4 years, 8 months ago (2015-09-30 17:25:28 UTC) #1
Sebastian Noack
https://codereview.adblockplus.org/29328733/diff/29328734/skin/firstRun.css File skin/firstRun.css (right): https://codereview.adblockplus.org/29328733/diff/29328734/skin/firstRun.css#newcode309 skin/firstRun.css:309: margin-start: 20px; It seems there isn't an unprefixed margin-start, ...
4 years, 8 months ago (2015-10-05 12:23:56 UTC) #2
saroyanm
https://codereview.adblockplus.org/29328733/diff/29328734/skin/firstRun.css File skin/firstRun.css (right): https://codereview.adblockplus.org/29328733/diff/29328734/skin/firstRun.css#newcode309 skin/firstRun.css:309: margin-start: 20px; On 2015/10/05 12:23:56, Sebastian Noack wrote: > ...
4 years, 8 months ago (2015-10-05 12:51:10 UTC) #3
Thomas Greiner
https://codereview.adblockplus.org/29328733/diff/29328734/skin/firstRun.css File skin/firstRun.css (right): https://codereview.adblockplus.org/29328733/diff/29328734/skin/firstRun.css#newcode309 skin/firstRun.css:309: margin-start: 20px; On 2015/10/05 12:51:10, saroyanm wrote: > On ...
4 years, 8 months ago (2015-10-05 13:43:17 UTC) #4
saroyanm
I'm not sure if we need to also fix the dashed border issue in share ...
4 years, 8 months ago (2015-10-05 17:18:55 UTC) #5
Thomas Greiner
https://codereview.adblockplus.org/29328733/diff/29328845/firstRun.html File firstRun.html (right): https://codereview.adblockplus.org/29328733/diff/29328845/firstRun.html#newcode23 firstRun.html:23: <meta name="viewport" content="width=device-width, initial-scale=1"> On 2015/10/05 17:18:55, saroyanm wrote: ...
4 years, 8 months ago (2015-10-06 09:54:37 UTC) #6
saroyanm
https://codereview.adblockplus.org/29328733/diff/29328857/skin/firstRun.css File skin/firstRun.css (right): https://codereview.adblockplus.org/29328733/diff/29328857/skin/firstRun.css#newcode153 skin/firstRun.css:153: padding-left: 0px; I can see that here you don't ...
4 years, 8 months ago (2015-10-07 10:15:08 UTC) #7
Thomas Greiner
https://codereview.adblockplus.org/29328733/diff/29328857/skin/firstRun.css File skin/firstRun.css (right): https://codereview.adblockplus.org/29328733/diff/29328857/skin/firstRun.css#newcode153 skin/firstRun.css:153: padding-left: 0px; On 2015/10/07 10:15:07, saroyanm wrote: > I ...
4 years, 8 months ago (2015-10-07 13:24:25 UTC) #8
saroyanm
https://codereview.adblockplus.org/29328733/diff/29328905/skin/firstRun.css File skin/firstRun.css (right): https://codereview.adblockplus.org/29328733/diff/29328905/skin/firstRun.css#newcode145 skin/firstRun.css:145: max-width: 760px; You don't need to specify different max-width ...
4 years, 8 months ago (2015-10-07 17:26:40 UTC) #9
Thomas Greiner
https://codereview.adblockplus.org/29328733/diff/29328905/skin/firstRun.css File skin/firstRun.css (right): https://codereview.adblockplus.org/29328733/diff/29328905/skin/firstRun.css#newcode145 skin/firstRun.css:145: max-width: 760px; On 2015/10/07 17:26:40, saroyanm wrote: > You ...
4 years, 8 months ago (2015-10-08 14:59:54 UTC) #10
saroyanm
On 2015/10/08 14:59:54, Thomas Greiner wrote: > https://codereview.adblockplus.org/29328733/diff/29328905/skin/firstRun.css > File skin/firstRun.css (right): > > https://codereview.adblockplus.org/29328733/diff/29328905/skin/firstRun.css#newcode145 ...
4 years, 8 months ago (2015-10-08 15:06:04 UTC) #11
Sebastian Noack
4 years, 8 months ago (2015-10-09 12:02:22 UTC) #12
LGTM
Sign in to reply to this message.

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