MD Settings: rework how advanced UI shows

MD Settings: rework how advanced UI shows

Before, there were some events.  And booleans.  And maybe some binding.
It generally worked, but it was a little complex.  Now there's just
2-way bindings everywhere.  Changes from toggling the advanced buttons
in the main UI or in the nav flow to all other parties.  I also checked
that opening a page like /clearBrowserData works (all advanced toggles
and states work as expected).

Also, we probably had to call .open() or .close() explicitly because
<paper-submenu> has a bug when using only the opened="{{binding}}":
https://github.com/PolymerElements/paper-menu/issues/88

A fix for that bug is here:
https://github.com/PolymerElements/paper-menu/pull/107

I might also make a local change in Chrome's fork of Polymer if we must.

R=dpapad@chromium.org
BUG=650951
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/5f9bf9e2c68a25b9bd02639c70d31c9c627cb458
Cr-Commit-Position: refs/heads/master@{#425795}

[Dan Beam, 2016-10-14 02:37:42]

Description was changed from

==========
MD Settings: rework how advanced UI shows

Before, there were some events.  And booleans.  And maybe some binding.
It generally worked, but it was a little complex.  Now there's just
2-way bindings everywhere.  Changes from toggling the advanced buttons
in the main UI or in the nav flow to all other parties.  I also checked
that opening a page like /clearBrowserData works (all advanced toggles
and states work as expected).

Also, we probably had to call .open() or .close() explicitly because
<paper-submenu> has a bug when using only the opened="{{binding}}":
https://github.com/PolymerElements/paper-menu/issues/88

A fix for that bug is here:
https://github.com/PolymerElements/paper-menu/pull/107

I might also make a local change in Chrome's fork of Polymer if we must.

R=dschuyler@chromium.org
BUG=650951
==========

to

==========
MD Settings: rework how advanced UI shows

Before, there were some events.  And booleans.  And maybe some binding.
It generally worked, but it was a little complex.  Now there's just
2-way bindings everywhere.  Changes from toggling the advanced buttons
in the main UI or in the nav flow to all other parties.  I also checked
that opening a page like /clearBrowserData works (all advanced toggles
and states work as expected).

Also, we probably had to call .open() or .close() explicitly because
<paper-submenu> has a bug when using only the opened="{{binding}}":
https://github.com/PolymerElements/paper-menu/issues/88

A fix for that bug is here:
https://github.com/PolymerElements/paper-menu/pull/107

I might also make a local change in Chrome's fork of Polymer if we must.

R=dschuyler@chromium.org
BUG=650951
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2016-10-14 02:47:22]

note: this will result in "double tap" bug with these steps:

0) open chrome://md-settings
1) show advanced
2) open side-nav
3) tap advanced

without this fix https://codereview.chromium.org/2412343004 in some form,
tapping advanced the first time will do nothing.  every tap afterward will work,
afaik.

[Dan Beam, 2016-10-14 03:11:01]

The CQ bit was checked by dbeam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-14 03:11:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-14 03:51:12]

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

[commit-bot: I haz the power, 2016-10-14 03:51:12]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Dan Beam, 2016-10-14 03:54:24]

The CQ bit was checked by dbeam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-14 03:54:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-14 04:44:41]

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

[commit-bot: I haz the power, 2016-10-14 04:44:42]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2016-10-14 23:11:51]

Description was changed from

==========
MD Settings: rework how advanced UI shows

Before, there were some events.  And booleans.  And maybe some binding.
It generally worked, but it was a little complex.  Now there's just
2-way bindings everywhere.  Changes from toggling the advanced buttons
in the main UI or in the nav flow to all other parties.  I also checked
that opening a page like /clearBrowserData works (all advanced toggles
and states work as expected).

Also, we probably had to call .open() or .close() explicitly because
<paper-submenu> has a bug when using only the opened="{{binding}}":
https://github.com/PolymerElements/paper-menu/issues/88

A fix for that bug is here:
https://github.com/PolymerElements/paper-menu/pull/107

I might also make a local change in Chrome's fork of Polymer if we must.

R=dschuyler@chromium.org
BUG=650951
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: rework how advanced UI shows

Before, there were some events.  And booleans.  And maybe some binding.
It generally worked, but it was a little complex.  Now there's just
2-way bindings everywhere.  Changes from toggling the advanced buttons
in the main UI or in the nav flow to all other parties.  I also checked
that opening a page like /clearBrowserData works (all advanced toggles
and states work as expected).

Also, we probably had to call .open() or .close() explicitly because
<paper-submenu> has a bug when using only the opened="{{binding}}":
https://github.com/PolymerElements/paper-menu/issues/88

A fix for that bug is here:
https://github.com/PolymerElements/paper-menu/pull/107

I might also make a local change in Chrome's fork of Polymer if we must.

R=dpapad@chromium.org
BUG=650951
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2016-10-14 23:11:52]

dbeam@chromium.org changed reviewers:
+ dpapad@chromium.org
- dschuyler@chromium.org

[Dan Beam, 2016-10-14 23:12:24]

-dschuyler@
+dpapad@

[dpapad, 2016-10-17 19:11:43]

LGTM with optional test improvement suggestion.

https://codereview.chromium.org/2420833002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_menu/settings_menu.js (left):

https://codereview.chromium.org/2420833002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/settings_menu/settings_menu.js:27: attached:
function() {
Nice!!

https://codereview.chromium.org/2420833002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_ui/settings_ui.js (right):

https://codereview.chromium.org/2420833002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/settings_ui/settings_ui.js:35:
advancedOpened_: {
@private

https://codereview.chromium.org/2420833002/diff/20001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/settings_menu_test.js (right):

https://codereview.chromium.org/2420833002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/settings/settings_menu_test.js:21: test('openAdvanced',
function() {
This tests that the binding causes the menu to update correctly, but can we also
add a test where a 'tap' is simulated on the "Advanced" entry in the menu?

https://codereview.chromium.org/2420833002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/settings/settings_menu_test.js:25:
assertTrue(settingsMenu.$.advancedPage.opened);
Also, can we test the reverse too (closing after being open)?

settingsMenu.advancedOpened = false;
Polymer.dom.flush();
assertFalse(settingsMenu.$.advancedPage.opened);

[Dan Beam, 2016-10-17 20:52:17]

https://codereview.chromium.org/2420833002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_menu/settings_menu.js (left):

https://codereview.chromium.org/2420833002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/settings_menu/settings_menu.js:27: attached:
function() {
On 2016/10/17 19:11:43, dpapad wrote:
> Nice!!

Acknowledged.

https://codereview.chromium.org/2420833002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_ui/settings_ui.js (right):

https://codereview.chromium.org/2420833002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/settings_ui/settings_ui.js:35:
advancedOpened_: {
On 2016/10/17 19:11:43, dpapad wrote:
> @private

Done.

https://codereview.chromium.org/2420833002/diff/20001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/settings_menu_test.js (right):

https://codereview.chromium.org/2420833002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/settings/settings_menu_test.js:21: test('openAdvanced',
function() {
On 2016/10/17 19:11:43, dpapad wrote:
> This tests that the binding causes the menu to update correctly, but can we
also
> add a test where a 'tap' is simulated on the "Advanced" entry in the menu?

Done.

https://codereview.chromium.org/2420833002/diff/20001/chrome/test/data/webui/...
chrome/test/data/webui/settings/settings_menu_test.js:25:
assertTrue(settingsMenu.$.advancedPage.opened);
On 2016/10/17 19:11:43, dpapad wrote:
> Also, can we test the reverse too (closing after being open)?
> 
> settingsMenu.advancedOpened = false;
> Polymer.dom.flush();
> assertFalse(settingsMenu.$.advancedPage.opened);

Done.

[Dan Beam, 2016-10-17 20:52:23]

The CQ bit was checked by dbeam@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-17 20:52:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Dan Beam, 2016-10-17 21:37:59]

The CQ bit was unchecked by dbeam@chromium.org

[Dan Beam, 2016-10-17 21:38:05]

The CQ bit was checked by dbeam@chromium.org

[Dan Beam, 2016-10-17 21:38:05]

The patchset sent to the CQ was uploaded after l-g-t-m from dpapad@chromium.org
Link to the patchset: https://codereview.chromium.org/2420833002/#ps40001
(title: "more tests")

[commit-bot: I haz the power, 2016-10-17 21:39:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-17 22:35:25]

Description was changed from

==========
MD Settings: rework how advanced UI shows

Before, there were some events.  And booleans.  And maybe some binding.
It generally worked, but it was a little complex.  Now there's just
2-way bindings everywhere.  Changes from toggling the advanced buttons
in the main UI or in the nav flow to all other parties.  I also checked
that opening a page like /clearBrowserData works (all advanced toggles
and states work as expected).

Also, we probably had to call .open() or .close() explicitly because
<paper-submenu> has a bug when using only the opened="{{binding}}":
https://github.com/PolymerElements/paper-menu/issues/88

A fix for that bug is here:
https://github.com/PolymerElements/paper-menu/pull/107

I might also make a local change in Chrome's fork of Polymer if we must.

R=dpapad@chromium.org
BUG=650951
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: rework how advanced UI shows

Before, there were some events.  And booleans.  And maybe some binding.
It generally worked, but it was a little complex.  Now there's just
2-way bindings everywhere.  Changes from toggling the advanced buttons
in the main UI or in the nav flow to all other parties.  I also checked
that opening a page like /clearBrowserData works (all advanced toggles
and states work as expected).

Also, we probably had to call .open() or .close() explicitly because
<paper-submenu> has a bug when using only the opened="{{binding}}":
https://github.com/PolymerElements/paper-menu/issues/88

A fix for that bug is here:
https://github.com/PolymerElements/paper-menu/pull/107

I might also make a local change in Chrome's fork of Polymer if we must.

R=dpapad@chromium.org
BUG=650951
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-10-17 22:35:26]

Committed patchset #3 (id:40001)

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

Description was changed from

==========
MD Settings: rework how advanced UI shows

Before, there were some events.  And booleans.  And maybe some binding.
It generally worked, but it was a little complex.  Now there's just
2-way bindings everywhere.  Changes from toggling the advanced buttons
in the main UI or in the nav flow to all other parties.  I also checked
that opening a page like /clearBrowserData works (all advanced toggles
and states work as expected).

Also, we probably had to call .open() or .close() explicitly because
<paper-submenu> has a bug when using only the opened="{{binding}}":
https://github.com/PolymerElements/paper-menu/issues/88

A fix for that bug is here:
https://github.com/PolymerElements/paper-menu/pull/107

I might also make a local change in Chrome's fork of Polymer if we must.

R=dpapad@chromium.org
BUG=650951
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: rework how advanced UI shows

Before, there were some events.  And booleans.  And maybe some binding.
It generally worked, but it was a little complex.  Now there's just
2-way bindings everywhere.  Changes from toggling the advanced buttons
in the main UI or in the nav flow to all other parties.  I also checked
that opening a page like /clearBrowserData works (all advanced toggles
and states work as expected).

Also, we probably had to call .open() or .close() explicitly because
<paper-submenu> has a bug when using only the opened="{{binding}}":
https://github.com/PolymerElements/paper-menu/issues/88

A fix for that bug is here:
https://github.com/PolymerElements/paper-menu/pull/107

I might also make a local change in Chrome's fork of Polymer if we must.

R=dpapad@chromium.org
BUG=650951
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/5f9bf9e2c68a25b9bd02639c70d31c9c627cb458
Cr-Commit-Position: refs/heads/master@{#425795}
==========

[commit-bot: I haz the power, 2016-10-17 22:37:07]

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