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

Issue 29334038: Issue 2802/2358 - Dynamically generate tooltips in options page (Closed)

Created:
Jan. 19, 2016, 6:24 p.m. by Thomas Greiner
Modified:
June 8, 2016, 4:06 p.m.
Reviewers:
saroyanm
CC:
Felix Dahlke
Visibility:
Public.

Description

Issue 2802/2358 - Dynamically generate tooltips in options page

Patch Set 1 #

Total comments: 1

Patch Set 2 : Rebased to d12b18c2a168 #

Patch Set 3 : Rebased to 536c9b810423 #

Total comments: 15

Patch Set 4 : Rebased to 75534a4a1e0e and addressed comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+165 lines, -91 lines) Patch
M locale/en-US/new-options.json View 1 2 3 6 chunks +32 lines, -8 lines 0 comments Download
M new-options.html View 1 2 3 7 chunks +25 lines, -57 lines 0 comments Download
M new-options.js View 1 2 3 12 chunks +102 lines, -21 lines 0 comments Download
M skin/new-options.css View 1 2 3 3 chunks +6 lines, -5 lines 0 comments Download

Messages

Total messages: 7
Thomas Greiner
Jan. 19, 2016, 6:28 p.m. (2016-01-19 18:28:29 UTC) #1
Thomas Greiner
https://codereview.adblockplus.org/29334038/diff/29334039/skin/options.css File skin/options.css (left): https://codereview.adblockplus.org/29334038/diff/29334039/skin/options.css#oldcode401 skin/options.css:401: } Please ignore this change. It's already covered by ...
Jan. 19, 2016, 7:28 p.m. (2016-01-19 19:28:44 UTC) #2
saroyanm
Thomas can you please rebase this review when you will have time and it will ...
March 21, 2016, 10:04 a.m. (2016-03-21 10:04:13 UTC) #3
Thomas Greiner
On 2016/03/21 10:04:13, saroyanm wrote: > Thomas can you please rebase this review when you ...
March 21, 2016, 2:21 p.m. (2016-03-21 14:21:27 UTC) #4
saroyanm
https://codereview.adblockplus.org/29334038/diff/29338813/options.html File options.html (right): https://codereview.adblockplus.org/29334038/diff/29338813/options.html#newcode115 options.html:115: <div class="fill"> Why do we need this fill wrapper ...
March 24, 2016, 6:27 p.m. (2016-03-24 18:27:51 UTC) #5
Thomas Greiner
https://codereview.adblockplus.org/29334038/diff/29338813/options.html File options.html (right): https://codereview.adblockplus.org/29334038/diff/29338813/options.html#newcode115 options.html:115: <div class="fill"> On 2016/03/24 18:27:50, saroyanm wrote: > Why ...
May 31, 2016, 5 p.m. (2016-05-31 17:00:02 UTC) #6
saroyanm
June 8, 2016, 3:48 p.m. (2016-06-08 15:48:32 UTC) #7
LGTM

https://codereview.adblockplus.org/29334038/diff/29338813/options.html
File options.html (right):

https://codereview.adblockplus.org/29334038/diff/29338813/options.html#newcod...
options.html:115: <div class="fill">
On 2016/05/31 17:00:01, Thomas Greiner wrote:
> On 2016/03/24 18:27:50, saroyanm wrote:
> > Why do we need this fill wrapper ?
> 
> To fill the space between the checkbox and the "popular" label. Since the
latter
> will be removed in #3741, it won't be necessary anymore though.

Got it, yes make sense to take care of that during rebasing if this change will
land earlier.

Powered by Google App Engine
This is Rietveld