Site Settings Desktop: Change how adding site exceptions work.

Site Settings Desktop: Change how adding site exceptions work.

Instead of the whitelist model, move to one that allows adding either
Block/Allow entries. This requires the category heading to be always
shown, because otherwise there's no context. This CL also adds support
for Enter (to submit) in the dialog.

BUG=614277, 543635
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/e77d721e353b457b741a3a2781b7181e32fbcb63
Cr-Commit-Position: refs/heads/master@{#399145}

[Finnur, 2016-06-08 15:15:01]

Description was changed from

==========
Site Settings Desktop: Change how adding site exceptions work.

Instead of the whitelist model, move to one that allows adding either
Block/Allow entries. This requires the category heading to be always
shown, because otherwise there's no context. This CL also adds support
for Enter (to submit) in the dialog.

BUG=614277, 543635
==========

to

==========
Site Settings Desktop: Change how adding site exceptions work.

Instead of the whitelist model, move to one that allows adding either
Block/Allow entries. This requires the category heading to be always
shown, because otherwise there's no context. This CL also adds support
for Enter (to submit) in the dialog.

BUG=614277, 543635
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[Finnur, 2016-06-08 15:25:27]

finnur@chromium.org changed reviewers:
+ michaelpg@chromium.org

[michaelpg, 2016-06-09 17:38:00]

lgtm

https://codereview.chromium.org/2044963003/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/site_settings/add_site_dialog.js (right):

https://codereview.chromium.org/2044963003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/add_site_dialog.js:51: */
@private

https://codereview.chromium.org/2044963003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/add_site_dialog.js:61:
onAddTap_: function() {
maybe rename "onSubmit_" or something, and make both HTML events point here?

https://codereview.chromium.org/2044963003/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/site_settings/site_list.html (right):

https://codereview.chromium.org/2044963003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/site_list.html:22: left: 8px;
TIL you have to over-specify position to be RTL friendly:
https://developer.mozilla.org/en-US/docs/Web/CSS/right#wikiArticle

ie: "left: 8px; right: 8px;"

https://codereview.chromium.org/2044963003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/site_list.html:32:
text-transform: inherit;
nit: none

[Finnur, 2016-06-09 21:53:08]

Good points. Thanks!

https://codereview.chromium.org/2044963003/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/site_settings/add_site_dialog.js (right):

https://codereview.chromium.org/2044963003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/add_site_dialog.js:51: */
On 2016/06/09 17:38:00, michaelpg wrote:
> @private

Done.

https://codereview.chromium.org/2044963003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/add_site_dialog.js:61:
onAddTap_: function() {
Renamed. But... both HTML events? I presume you mean 'both calls', as in from JS
and HTML. If not, happy to do follow-up CL.

https://codereview.chromium.org/2044963003/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/site_settings/site_list.html (right):

https://codereview.chromium.org/2044963003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/site_list.html:22: left: 8px;
On 2016/06/09 17:38:00, michaelpg wrote:
> TIL you have to over-specify position to be RTL friendly:
> https://developer.mozilla.org/en-US/docs/Web/CSS/right#wikiArticle
> 
> ie: "left: 8px; right: 8px;"

Done.

https://codereview.chromium.org/2044963003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/site_list.html:32:
text-transform: inherit;
On 2016/06/09 17:38:00, michaelpg wrote:
> nit: none

Done.

[Finnur, 2016-06-09 21:53:14]

The CQ bit was checked by finnur@chromium.org

[Finnur, 2016-06-09 21:53:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/2044963003/#ps20001
(title: "Address feedback")

[commit-bot: I haz the power, 2016-06-09 21:54:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2044963003/20001

[michaelpg, 2016-06-09 22:31:05]

https://codereview.chromium.org/2044963003/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/site_settings/add_site_dialog.js (right):

https://codereview.chromium.org/2044963003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/site_settings/add_site_dialog.js:61:
onAddTap_: function() {
On 2016/06/09 21:53:08, Finnur wrote:
> Renamed. But... both HTML events? I presume you mean 'both calls', as in from
JS
> and HTML. If not, happy to do follow-up CL.

I meant to additionally change 
  on-keys-pressed="onEnterPressed_"
to
  on-keys-pressed="onSubmit_"
so we don't have the extra function. It's a minor thing, fine as is.

[Finnur, 2016-06-09 22:57:35]

The CQ bit was unchecked by finnur@chromium.org

[Finnur, 2016-06-09 23:01:11]

Oh, hey -- yeah, I like that improvement. Done.

[Finnur, 2016-06-09 23:01:16]

The CQ bit was checked by finnur@chromium.org

[Finnur, 2016-06-09 23:01:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/2044963003/#ps40001
(title: "Address feedback")

[commit-bot: I haz the power, 2016-06-09 23:02:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2044963003/40001

[commit-bot: I haz the power, 2016-06-10 01:03:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-10 01:03:51]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_TIMED_OUT, no build
URL)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL)

[Finnur, 2016-06-10 10:09:04]

The CQ bit was checked by finnur@chromium.org

[commit-bot: I haz the power, 2016-06-10 10:09:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2044963003/40001

[commit-bot: I haz the power, 2016-06-10 10:12:54]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-10 10:13:00]

CQ bit was unchecked

[commit-bot: I haz the power, 2016-06-10 10:15:17]

Description was changed from

==========
Site Settings Desktop: Change how adding site exceptions work.

Instead of the whitelist model, move to one that allows adding either
Block/Allow entries. This requires the category heading to be always
shown, because otherwise there's no context. This CL also adds support
for Enter (to submit) in the dialog.

BUG=614277, 543635
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Site Settings Desktop: Change how adding site exceptions work.

Instead of the whitelist model, move to one that allows adding either
Block/Allow entries. This requires the category heading to be always
shown, because otherwise there's no context. This CL also adds support
for Enter (to submit) in the dialog.

BUG=614277, 543635
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/e77d721e353b457b741a3a2781b7181e32fbcb63
Cr-Commit-Position: refs/heads/master@{#399145}
==========

[commit-bot: I haz the power, 2016-06-10 10:15:19]

Patchset 3 (id:??) landed as
https://crrev.com/e77d721e353b457b741a3a2781b7181e32fbcb63
Cr-Commit-Position: refs/heads/master@{#399145}