MD Settings: refactor some main page properties, fix Advanced toggle

chrome/browser/resources/settings/settings_main/settings_main.html

Index: chrome/browser/resources/settings/settings_main/settings_main.html
diff --git a/chrome/browser/resources/settings/settings_main/settings_main.html b/chrome/browser/resources/settings/settings_main/settings_main.html
index 990c85c6c2e61ce39e1eca9a1468443339875ca0..a84a59a9ec0e9816930dad36d6f423c3e8b11330 100644
--- a/chrome/browser/resources/settings/settings_main/settings_main.html
+++ b/chrome/browser/resources/settings/settings_main/settings_main.html
@@ -38,25 +38,27 @@
       }
     </style>
     <content select="paper-icon-button"></content>
-    <template is="dom-if" if="[[showBasicPage_]]">
-      <settings-basic-page prefs="{{prefs}}" current-route="{{currentRoute}}">
-      </settings-basic-page>
-    </template>
-    <template is="dom-if" if="[[showAdvancedToggle_]]">
-      <div id="toggleSpacer"></div>
-      <div id="toggleContainer">
-        <div id="advancedToggle" on-tap="toggleAdvancedPage_">
-          <span>$i18n{advancedPageTitle}</span>
-          <iron-icon icon="[[arrowState_(showAdvancedPage_)]]"></iron-icon>
+    <template is="dom-if" if="[[!showPages_.about]]">

[dschuyler, 2016/07/21 19:02:05]

Consider adding a comment about why there is
an extra level of dom-if is here. Since it's not
strictly necessary, is it an optimization or 
just preference - if it's an optimization, is
it optimizing for the 'basic' settings page
(the expected common case)?

[michaelpg, 2016/07/21 19:55:28]

it's just for simplicity. These three elements (basic page, advanced toggle,
advanced page) are logically grouped: they can all be shown at the same time,
and they can't be mixed with the About page. The old code has them all at the
top level alongside About; the new code is a tree with About and non-About in
separate subtrees.

Without the dom-if we need to add back "showAdvancedToggle_" and keep track of
that separately, or add a function showAdvancedToggle_(inSubpage_, showPages_).
That seems redundant to me, but maybe it comes down to preference.

[michaelpg, 2016/07/22 15:12:17]

In the interest of making rebasing easier and this CL smaller I've reverted
these changes. Using a computed binding instead (showAdvancedToggle_).

+      <template is="dom-if" if="[[showPages_.basic]]">
+        <settings-basic-page prefs="{{prefs}}" current-route="{{currentRoute}}">
+        </settings-basic-page>
+      </template>
+      <div hidden$="[[inSubpage_]]">
+        <div id="toggleSpacer"></div>
+        <div id="toggleContainer">
+          <div id="advancedToggle" on-tap="toggleAdvancedPage_">
+            <span>$i18n{advancedPageTitle}</span>
+            <iron-icon icon="[[arrowState_(showPages_.advanced)]]"></iron-icon>
+          </div>
         </div>
       </div>
+      <template is="dom-if" if="[[showPages_.advanced]]">
+        <settings-advanced-page prefs="{{prefs}}"
+            current-route="{{currentRoute}}">
+        </settings-advanced-page>
+      </template>
     </template>
-    <template is="dom-if" if="[[showAdvancedPage_]]">
-      <settings-advanced-page prefs="{{prefs}}"
-          current-route="{{currentRoute}}">
-      </settings-advanced-page>
-    </template>
-    <template is="dom-if" if="[[showAboutPage_]]">
+    <template is="dom-if" if="[[showPages_.about]]">
       <settings-about-page current-route="{{currentRoute}}">
       </settings-about-page>
     </template>