Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(120)

Issue 2437183002: [MD settings] content site list toggles (Closed)

Created:
4 years, 2 months ago by dschuyler
Modified:
4 years, 2 months ago
Reviewers:
dpapad
CC:
arv+watch_chromium.org, chromium-reviews, dbeam+watch-settings_chromium.org, dbeam+watch-closure_chromium.org, jlklein+watch-closure_chromium.org, michaelpg+watch-md-settings_chromium.org, michaelpg+watch-md-ui_chromium.org, stevenjb+watch-md-settings_chromium.org, vitalyp+closure_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[MD settings] content site list toggles This CL makes the boolean settings in the content settings site list subpage. Also adds the settings-boolean-control-behavior. BUG=657960 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/151575ce0bb6fc26a3381c029fccc3c0ec5bfda0 Cr-Commit-Position: refs/heads/master@{#426666}

Patch Set 1 #

Total comments: 7

Patch Set 2 : review changes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+153 lines, -152 lines) Patch
M chrome/browser/resources/settings/controls/compiled_resources2.gyp View 2 chunks +16 lines, -4 lines 0 comments Download
A chrome/browser/resources/settings/controls/settings_boolean_control_behavior.html View 1 chunk +5 lines, -0 lines 0 comments Download
A + chrome/browser/resources/settings/controls/settings_boolean_control_behavior.js View 3 chunks +18 lines, -17 lines 0 comments Download
M chrome/browser/resources/settings/controls/settings_checkbox.html View 2 chunks +2 lines, -4 lines 0 comments Download
M chrome/browser/resources/settings/controls/settings_checkbox.js View 1 chunk +1 line, -92 lines 0 comments Download
A chrome/browser/resources/settings/controls/settings_toggle_button.html View 1 chunk +42 lines, -0 lines 0 comments Download
A chrome/browser/resources/settings/controls/settings_toggle_button.js View 1 chunk +13 lines, -0 lines 0 comments Download
M chrome/browser/resources/settings/privacy_page/privacy_page.html View 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/resources/settings/settings_resources.grd View 2 chunks +12 lines, -0 lines 0 comments Download
M chrome/browser/resources/settings/site_settings/site_settings_category.html View 1 3 chunks +11 lines, -22 lines 0 comments Download
M chrome/test/data/webui/settings/cr_settings_browsertest.js View 1 chunk +19 lines, -0 lines 0 comments Download
A + chrome/test/data/webui/settings/settings_toggle_button_tests.js View 3 chunks +11 lines, -11 lines 0 comments Download

Messages

Total messages: 19 (12 generated)
dschuyler
4 years, 2 months ago (2016-10-20 20:53:20 UTC) #5
dpapad
Design question: Would it make things easier and reduce the amount of code, if Instead ...
4 years, 2 months ago (2016-10-20 21:47:12 UTC) #8
dschuyler
We chatted offline about <foo type="bar"></foo> vs. <bar></bar>. It seems to simply be a style ...
4 years, 2 months ago (2016-10-20 22:33:38 UTC) #11
dpapad
LGTM https://codereview.chromium.org/2437183002/diff/1/chrome/browser/resources/settings/controls/settings_toggle_button.html File chrome/browser/resources/settings/controls/settings_toggle_button.html (right): https://codereview.chromium.org/2437183002/diff/1/chrome/browser/resources/settings/controls/settings_toggle_button.html#newcode31 chrome/browser/resources/settings/controls/settings_toggle_button.html:31: <div class="secondary">[[subLabel]]</div> On 2016/10/20 at 22:33:38, dschuyler wrote: ...
4 years, 2 months ago (2016-10-20 22:49:26 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2437183002/20001
4 years, 2 months ago (2016-10-21 00:42:19 UTC) #16
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 2 months ago (2016-10-21 00:56:36 UTC) #17
commit-bot: I haz the power
4 years, 2 months ago (2016-10-21 13:25:19 UTC) #19
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/151575ce0bb6fc26a3381c029fccc3c0ec5bfda0
Cr-Commit-Position: refs/heads/master@{#426666}

Powered by Google App Engine
This is Rietveld 408576698