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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 1 month ago by Thomas Greiner
Modified:
3 years, 8 months ago
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
4 years, 1 month ago (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 ...
4 years, 1 month ago (2016-01-19 19:28:44 UTC) #2
saroyanm
Thomas can you please rebase this review when you will have time and it will ...
3 years, 11 months ago (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 ...
3 years, 11 months ago (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 ...
3 years, 11 months ago (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 ...
3 years, 8 months ago (2016-05-31 17:00:02 UTC) #6
saroyanm
3 years, 8 months ago (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.
Sign in to reply to this message.

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