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

Issue 29338208: Issue 3796 - Added preference to remove developer tools panel (Closed)

Created:
March 14, 2016, 1:07 p.m. by Sebastian Noack
Modified:
March 15, 2016, 2:04 p.m.
Reviewers:
Thomas Greiner, kzar
Visibility:
Public.

Description

Issue 3796 - Added preference to remove developer tools panel

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+41 lines, -21 lines) Patch
M _locales/en_US/messages.json View 1 chunk +3 lines, -0 lines 0 comments Download
M background.js View 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/devtools.js View 1 chunk +7 lines, -5 lines 0 comments Download
M lib/prefs.js View 1 chunk +7 lines, -0 lines 3 comments Download
M options.html View 1 chunk +3 lines, -0 lines 0 comments Download
M options.js View 3 chunks +18 lines, -16 lines 0 comments Download

Messages

Total messages: 8
Sebastian Noack
March 14, 2016, 1:07 p.m. (2016-03-14 13:07:40 UTC) #1
kzar
LGTM (Thomas I guess you'll need to add this to the new options page too.)
March 14, 2016, 1:15 p.m. (2016-03-14 13:15:26 UTC) #2
Thomas Greiner
https://codereview.adblockplus.org/29338208/diff/29338209/lib/prefs.js File lib/prefs.js (right): https://codereview.adblockplus.org/29338208/diff/29338209/lib/prefs.js#newcode163 lib/prefs.js:163: defaults.show_devtools_panel = true; Since there's no mention in the ...
March 14, 2016, 4:26 p.m. (2016-03-14 16:26:06 UTC) #3
Sebastian Noack
https://codereview.adblockplus.org/29338208/diff/29338209/lib/prefs.js File lib/prefs.js (right): https://codereview.adblockplus.org/29338208/diff/29338209/lib/prefs.js#newcode163 lib/prefs.js:163: defaults.show_devtools_panel = true; On 2016/03/14 16:26:05, Thomas Greiner wrote: ...
March 14, 2016, 4:39 p.m. (2016-03-14 16:39:36 UTC) #4
kzar
https://codereview.adblockplus.org/29338208/diff/29338209/lib/prefs.js File lib/prefs.js (right): https://codereview.adblockplus.org/29338208/diff/29338209/lib/prefs.js#newcode163 lib/prefs.js:163: defaults.show_devtools_panel = true; On 2016/03/14 16:39:35, Sebastian Noack wrote: ...
March 14, 2016, 5:52 p.m. (2016-03-14 17:52:20 UTC) #5
Thomas Greiner
Unfortunately, this preference is not purely cosmetic since the panel is active even when if ...
March 14, 2016, 7:24 p.m. (2016-03-14 19:24:44 UTC) #6
Sebastian Noack
On 2016/03/14 19:24:44, Thomas Greiner wrote: > e.g. Reloading the page on the Networks tab ...
March 14, 2016, 8:02 p.m. (2016-03-14 20:02:35 UTC) #7
Thomas Greiner
March 15, 2016, 2:01 p.m. (2016-03-15 14:01:45 UTC) #8
On 2016/03/14 20:02:35, Sebastian Noack wrote:
> On 2016/03/14 19:24:44, Thomas Greiner wrote:
> > e.g. Reloading the page on the Networks tab will populate the table on our
> > panel.
> 
> It does not. You have to explicitly switch to our devtools panel in order to
> activate it. Though, once it got activated it stays active for that tab until
> the devtools (or the tab) is closed again.

Indeed, I wasn't aware of that.

> > The best solution I could imagine is showing the panel tab by default and
> asking
> > whether people want to enable it, the first time they visit that panel.
> 
> I agree that this would be a nice user experience (except for the glitch that
> the panel would still keep showing until you reopen the devtools panel).
> However, we can still implement that later in addition.

Since we're on the same page in that regard I'm fine with only implementing the
opt-out for now.

LGTM

Powered by Google App Engine
This is Rietveld