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

Issue 2788853003: [MD settings] clickable toggle labels on category default toggles (Closed)

Created:
3 years, 8 months ago by dschuyler
Modified:
3 years, 8 months ago
Reviewers:
stevenjb
CC:
chromium-reviews, michaelpg+watch-md-settings_chromium.org, michaelpg+watch-md-ui_chromium.org, dbeam+watch-settings_chromium.org, stevenjb+watch-md-settings_chromium.org, arv+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[MD settings] clickable toggle labels on category default toggles This CL makes top level category default rows react to mouse clicks. These are the one or two rows at the top of a content settings category (such as "cookies"). The rows are toggles. Before this CL the labels on the toggles wouldn't act on mouse clicks when they should. Also, there are several name changes and tiddying up that could be in a separate CL if that is preferred. BUG=704003 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2788853003 Cr-Commit-Position: refs/heads/master@{#461533} Committed: https://chromium.googlesource.com/chromium/src/+/ae7fc3094d50f04c513f4b62c5f45475cbba17f5

Patch Set 1 #

Total comments: 8

Patch Set 2 : improved comments #

Total comments: 2

Patch Set 3 : restore separator line #

Unified diffs Side-by-side diffs Delta from patch set Stats (+34 lines, -45 lines) Patch
M chrome/browser/resources/settings/site_settings/category_default_setting.html View 1 2 1 chunk +11 lines, -17 lines 0 comments Download
M chrome/browser/resources/settings/site_settings/category_default_setting.js View 1 7 chunks +23 lines, -28 lines 0 comments Download

Messages

Total messages: 26 (16 generated)
dschuyler
3 years, 8 months ago (2017-03-31 23:15:45 UTC) #5
dschuyler
On 2017/03/31 23:15:45, dschuyler wrote: It's hard to describe where in the UI this change ...
3 years, 8 months ago (2017-03-31 23:16:54 UTC) #6
stevenjb
https://codereview.chromium.org/2788853003/diff/1/chrome/browser/resources/settings/site_settings/category_default_setting.html File chrome/browser/resources/settings/site_settings/category_default_setting.html (right): https://codereview.chromium.org/2788853003/diff/1/chrome/browser/resources/settings/site_settings/category_default_setting.html#newcode13 chrome/browser/resources/settings/site_settings/category_default_setting.html:13: class="settings-box first" Interesting. I guess since settings-toggle-button is kind ...
3 years, 8 months ago (2017-03-31 23:28:15 UTC) #7
dschuyler
https://codereview.chromium.org/2788853003/diff/1/chrome/browser/resources/settings/site_settings/category_default_setting.html File chrome/browser/resources/settings/site_settings/category_default_setting.html (right): https://codereview.chromium.org/2788853003/diff/1/chrome/browser/resources/settings/site_settings/category_default_setting.html#newcode13 chrome/browser/resources/settings/site_settings/category_default_setting.html:13: class="settings-box first" On 2017/03/31 23:28:14, stevenjb wrote: > Interesting. ...
3 years, 8 months ago (2017-04-01 00:09:39 UTC) #13
stevenjb
lgtm https://codereview.chromium.org/2788853003/diff/40001/chrome/browser/resources/settings/site_settings/category_default_setting.js File chrome/browser/resources/settings/site_settings/category_default_setting.js (right): https://codereview.chromium.org/2788853003/diff/40001/chrome/browser/resources/settings/site_settings/category_default_setting.js#newcode28 chrome/browser/resources/settings/site_settings/category_default_setting.js:28: toggleOnLabel: String, This is fine, but I still ...
3 years, 8 months ago (2017-04-01 00:15:38 UTC) #14
dschuyler
https://codereview.chromium.org/2788853003/diff/40001/chrome/browser/resources/settings/site_settings/category_default_setting.js File chrome/browser/resources/settings/site_settings/category_default_setting.js (right): https://codereview.chromium.org/2788853003/diff/40001/chrome/browser/resources/settings/site_settings/category_default_setting.js#newcode28 chrome/browser/resources/settings/site_settings/category_default_setting.js:28: toggleOnLabel: String, On 2017/04/01 00:15:37, stevenjb wrote: > This ...
3 years, 8 months ago (2017-04-01 00:21:00 UTC) #15
dschuyler
I realized that I should not have put the .continuation class on the second toggle. ...
3 years, 8 months ago (2017-04-01 01:40:49 UTC) #18
stevenjb
still lgtm
3 years, 8 months ago (2017-04-03 17:54:53 UTC) #21
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/2788853003/60001
3 years, 8 months ago (2017-04-03 18:17:06 UTC) #23
commit-bot: I haz the power
3 years, 8 months ago (2017-04-03 21:00:03 UTC) #26
Message was sent while issue was closed.
Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/ae7fc3094d50f04c513f4b62c5f4...

Powered by Google App Engine
This is Rietveld 408576698