MD Settings: Network details cleanup

chrome/browser/resources/settings/internet_page/internet_detail_page.html

Index: chrome/browser/resources/settings/internet_page/internet_detail_page.html
diff --git a/chrome/browser/resources/settings/internet_page/internet_detail_page.html b/chrome/browser/resources/settings/internet_page/internet_detail_page.html
index bed4f3b3e170ff265aa5b535ce8c6d94daf01543..186e0d672e2a50276a4b6692aa9b33b17450e883 100644
--- a/chrome/browser/resources/settings/internet_page/internet_detail_page.html
+++ b/chrome/browser/resources/settings/internet_page/internet_detail_page.html
@@ -21,15 +21,12 @@
 <dom-module id="settings-internet-detail-page">
   <template>
     <style include="settings-shared">
-      .section {
-        margin-bottom: 5px;
-      }
-
       iron-collapse {
         margin: 10px;
       }
 
       cr-network-icon {
+        -webkit-margin-end: 20px;
         height: 32px;
         width: 32px;
       }
@@ -38,205 +35,180 @@
         -webkit-margin-start: 10px;
       }
 
-      #networkName,
-      #networkState {
-        -webkit-margin-start: 10px;
-        font-size: 20px;
+      div.indented {
+       -webkit-margin-start: 20px;
       }
 
-      #networkName {
-        font-weight: bold;
+      .settings-box.embedded {
+        align-self: stretch;
+        border-top: none;
+        padding: 0;
       }
 
-      #networkState[connected] {
-        color: green;
+      div.subtitle {
+        font-weight: 500;
+        padding: 20px 0;
       }
 
-      #outerDiv {
-        margin: 10px 20px;
+      #buttonDiv paper-button {
+        margin: 10px 0;
       }
 
-      #ipAddressLabel {
-        -webkit-margin-end: 10px;
-        font-weight: bold;
+      #networkName {
+        font-size: 100%;

[dschuyler, 2016/07/26 19:19:47]

nit: can this font-size: 100%; be removed? 
It looks like this is only used on line 82.
iiuc, it's effectively saying 
font-size = font-size * 1

If it is needed, it would help me learn to
hear why it's needed :)

[stevenjb, 2016/07/26 20:17:59]

It is not strictly needed. I was playing with these values, they aren't final.
I'm actually going to change this to 125% and I'll remove the networkState value
(changing it to 100%). I need to get Alan to make a call.

I also realized these can be <div>

+        font-weight: 500;
       }
 
-      #preferButton {
-        -webkit-margin-end: 8px;
-        -webkit-margin-start: -3px;
-        padding: 0
+      #networkState {
+        @apply(--settings-secondary);
+        font-size: 80%;
+     }
+
+      #networkState[connected] {
+        color: var(--google-green-500);
       }
 
       #proxyDiv {
         max-width: 500px;
       }
     </style>
-    <div class="layout vertical">
+    <div class="settings-box first single-column">
       <!-- Title section: Icon + name + connection state. -->
-      <div id="titleDiv" class="layout horizontal center">
-        <cr-network-icon network-state="[[networkProperties]]">
-        </cr-network-icon>
-        <span id="networkName">[[getStateName_(networkProperties)]]</span>
-        <span id="networkState"
-            connected$="[[isConnectedState_(networkProperties)]]"
-            >[[getStateText_(networkProperties)]]</span>
-      </div>
-
-      <div id="outerDiv" class="layout vertical">
-        <!-- For Cellular, show SIM info first. -->
-        <div class="layout vertical section">
-          <network-siminfo editable
-              hidden$="[[!showCellularSim_(networkProperties)]]"
-              network-properties="[[networkProperties]]"
-              on-siminfo-change="onNetworkPropertyChange_"
-              networking-private="[[networkingPrivate]]">
-          </network-siminfo>
-        </div>
-
-        <!-- Info and properties common to all networks. -->
-        <div class="layout vertical">
-          <div class="layout horizontal section" hidden$="[[!IPAddress]]">
-            <span id="ipAddressLabel">IP Address:</span>
-            <span>[[IPAddress]]</span>
-          </div>
-          <div class="section">
-            <span hidden$="[[!showShared_(networkProperties)]]">
-              This network is shared with other users.
+      <div class="settings-box embedded">
+        <div class="start layout horizontal center">
+          <cr-network-icon network-state="[[networkProperties]]">
+          </cr-network-icon>
+          <div class="layout vertical">
+            <span id="networkName">[[getStateName_(networkProperties)]]</span>
+            <span id="networkState" 
+                connected$="[[isConnectedState_(networkProperties)]]">
+              [[getStateText_(networkProperties)]]
             </span>
           </div>
-          <div class="layout horizontal center section"
-              hidden$="[[!showPreferNetwork_(networkProperties)]]">
-            <paper-icon-button id="preferButton" toggles
-                active="{{preferNetwork}}"
-                disabled=
-                    "[[isNetworkPolicyEnforced(networkProperties.Priority)]]"
-                icon="[[getPreferredIcon_(preferNetwork)]]">
-            </paper-icon-button>
-            <span>Prefer this network</span>
-            <cr-policy-network-indicator
-                property="[[networkProperties.Priority]]">
-            </cr-policy-network-indicator>
-          </div>
-          <div class="layout horizontal center section"
-              hidden$="[[!showAutoConnect_(networkProperties)]]">
-            <paper-checkbox checked="{{autoConnect}}">
-              Automatically connect to this network
-            </paper-checkbox>
-            <cr-policy-network-indicator
-                property="[[getManagedAutoConnect_(networkProperties)]]">
-            </cr-policy-network-indicator>
-          </div>
-          <!-- Properties to always show if present. -->
-          <div class="section">
-            <network-property-list
-                fields="[[getInfoFields_(networkProperties)]]"
-                property-dict="[[networkProperties]]">
-            </network-property-list>
-          </div>
         </div>
-
-        <!-- Button row: Advanced + Disconnect | Connect. -->
         <div id="buttonDiv" class="layout horizontal center">
-          <paper-button toggles noink active="{{advancedExpanded}}"
-              hidden$="[[!hasAdvancedOrDeviceFields_(networkProperties)]]">
-            Advanced
-          </paper-button>
-          <span class="flex"></span>
-          <paper-button hidden$="[[!showViewAccount_(networkProperties)]]"
-              on-tap="onViewAccountTap_">
+          <paper-button class="secondary-button" on-tap="onViewAccountTap_"
+              hidden$="[[!showViewAccount_(networkProperties)]]">
             View Account
           </paper-button>
-          <paper-button hidden$="[[!showActivate_(networkProperties)]]"
-              on-tap="onActivateTap_">
+          <paper-button class="secondary-button" on-tap="onActivateTap_"
+              hidden$="[[!showActivate_(networkProperties)]]">
             Activate
           </paper-button>
-          <paper-button hidden$="[[!showConnect_(networkProperties)]]"
-              disabled="[[!enableConnect_(networkProperties, defaultNetwork)]]"
-              on-tap="onConnectTap_">
+          <paper-button class="primary-button" on-tap="onConnectTap_"
+              hidden$="[[!showConnect_(networkProperties)]]"
+              disabled="[[!enableConnect_(networkProperties, defaultNetwork)]]">
             Connect
           </paper-button>
-          <paper-button hidden$="[[!showDisconnect_(networkProperties)]]"
-              on-tap="onDisconnectTap_">
+          <paper-button class="primary-button" on-tap="onDisconnectTap_"
+              hidden$="[[!showDisconnect_(networkProperties)]]">
             Disconnect
           </paper-button>
         </div>
+      </div>
+      <!-- Shared. -->
+      <div class="settings-box embedded"
+          hidden$="[[!showShared_(networkProperties)]]">
+        This network is shared with other users.
+      </div>
+      <!-- Prefer this network. -->
+      <div class="settings-box embedded"
+          hidden$="[[!showPreferNetwork_(networkProperties)]]">
+        <paper-checkbox checked="{{preferNetwork}}"
+            disabled="[[isNetworkPolicyEnforced(networkProperties.Priority)]]">
+          Prefer this network
+        </paper-checkbox>
+        <cr-policy-network-indicator property="[[networkProperties.Priority]]">
+        </cr-policy-network-indicator>
+      </div>
+      <!-- Autoconnect. -->
+      <div class="settings-box embedded"
+          hidden$="[[!showAutoConnect_(networkProperties)]]">
+        <paper-checkbox checked="{{autoConnect}}">
+          Automatically connect to this network
+        </paper-checkbox>
+        <cr-policy-network-indicator
+            property="[[getManagedAutoConnect_(networkProperties)]]">
+        </cr-policy-network-indicator>
+      </div>
+    </div>
 
-        <!-- Advanced section -->
-        <div id="advancedDiv" class="layout vertical"
-            hidden$="[[!hasAdvancedOrDeviceFields_(networkProperties)]]">
-          <iron-collapse opened="[[advancedExpanded]]">
-            <!-- Advanced properties -->
-            <div id="advancedInfoDiv" class="layout vertical section">
-              <network-property-list
-                  fields="[[getAdvancedFields_(networkProperties)]]"
-                  property-dict="[[networkProperties]]">
-              </network-property-list>
-            </div>
-
-            <!-- Network (APN, address, nameservers) -->
-            <div id="addressDiv" class="layout vertical section"
-                hidden$="[[!hasNetworkSection_(networkProperties)]]">
-              <div>
-                <paper-button toggles noink active="{{addressExpanded}}">
-                  Network
-                </paper-button>
-              </div>
-              <iron-collapse opened="[[addressExpanded]]">
-                <div class="layout vertical">
-                  <network-apnlist editable
-                      hidden$="[[!isType_(networkProperties, NetworkType.CELLULAR)]]"
-                      network-properties="[[networkProperties]]"
-                      on-apn-change="onNetworkPropertyChange_">
-                  </network-apnlist>
-                  <network-ip-config editable
-                      network-properties="[[networkProperties]]"
-                      on-ip-change="onIPConfigChange_">
-                  </network-ip-config>
-                  <network-nameservers editable
-                      network-properties="[[networkProperties]]"
-                      on-nameservers-change="onIPConfigChange_">
-                  </network-nameservers>
-                </div>
-              </iron-collapse>
-            </div>
-
-            <!-- Proxy -->
-            <div class="layout vertical section"
-                hidden$="[[!hasNetworkSection_(networkProperties)]]">
-              <div>
-                <paper-button toggles noink active="{{proxyExpanded}}">
-                  Proxy
-                </paper-button>
-              </div>
-              <iron-collapse opened="[[proxyExpanded]]">
-                <div id="proxyDiv" class="layout vertical">
-                  <network-proxy editable
-                      network-properties="[[networkProperties]]"
-                      on-proxy-change="onProxyChange_">
-                  </network-proxy>
-                </div>
-              </iron-collapse>
-            </div>
-
-            <!-- Device properties -->
-            <div class="layout vertical section"
-                hidden$="[[!hasDeviceFields_(networkProperties)]]">
-              <div>
-                <paper-button toggles noink active="{{deviceExpanded}}">
-                  Device
-                </paper-button>
-              </div>
-              <iron-collapse opened="[[deviceExpanded]]">
-                <network-property-list
-                    fields="[[getDeviceFields_(networkProperties)]]"
-                    property-dict="[[networkProperties]]">
-                </network-property-list>
-              </iron-collapse>
-            </div>
-          </iron-collapse>
+    <!-- SIM Info (Cellular only). -->
+    <div class="settings-box self-stretch"
+        hidden$="[[!showCellularSim_(networkProperties)]]">
+      <network-siminfo editable network-properties="[[networkProperties]]"
+          on-siminfo-change="onNetworkPropertyChange_"
+          networking-private="[[networkingPrivate]]">
+      </network-siminfo>
+    </div>
+
+    <!-- IP Address. -->
+    <div class="settings-box two-line single-column" hidden$="[[!IPAddress]]">
+      <div>IP Address</div>
+      <div class="secondary">[[IPAddress]]</div>
+    </div>
+
+    <!-- Properties to always show if present. -->
+    <network-property-list hidden$="[[!hasInfoFields_(networkProperties)]]"
+        fields="[[getInfoFields_(networkProperties)]]"
+        property-dict="[[networkProperties]]">
+    </network-property-list>
+
+    <!-- Advanced toggle. -->
+    <div class="settings-box" actionable on-tap="toggleAdvancedExpanded_"
+        hidden$="[[!hasAdvancedOrDeviceFields_(networkProperties)]]">
+      <div class="start">Advanced</div>
+      <cr-expand-button id="expandButton" expanded="{{advancedExpanded}}">
+      </cr-expand-button>
+    </div>
+
+    <!-- Advanced section -->
+    <iron-collapse opened="[[advancedExpanded]]">
+      <div class="layout vertical indented">
+        <!-- Advanced properties -->
+        <network-property-list
+            hidden$="[[!hasAdvancedFields_(networkProperties)]]"
+            fields="[[getAdvancedFields_(networkProperties)]]"
+            property-dict="[[networkProperties]]">
+        </network-property-list>
+
+        <!-- Device properties -->
+        <network-property-list
+            hidden$="[[!hasDeviceFields_(networkProperties)]]"
+            fields="[[getDeviceFields_(networkProperties)]]"
+            property-dict="[[networkProperties]]">
+        </network-property-list>
+
+        <!-- network -->
+        <div class="layout vertical"
+            hidden$="[[!hasNetworkSection_(networkProperties)]]">
+          <div class="subtitle indented">Network</div>
+
+          <!-- APN -->
+          <network-apnlist editable on-apn-change="onNetworkPropertyChange_"
+              hidden$="[[!isType_(networkProperties, NetworkType.CELLULAR)]]"
+              network-properties="[[networkProperties]]">
+          </network-apnlist>
+          <!-- IP Config -->

[dschuyler, 2016/07/26 19:19:47]

nit: I'm of a mixed opinion on the comments
in this file. Can we remove this last nesting
of comments? (Where the comments are 1:1 with
a thing with almost the same name as the comment).

(Removing more is good too).

[stevenjb, 2016/07/26 20:17:59]

I hear what you are saying, but the comments are *much* easier for me to read in
my editor, and this layout is a bit complicated. 

+          <network-ip-config editable on-ip-change="onIPConfigChange_"
+              network-properties="[[networkProperties]]">
+          </network-ip-config>
+          <!-- Nameservers -->
+          <network-nameservers editable
+              on-nameservers-change="onIPConfigChange_"
+              network-properties="[[networkProperties]]">
+          </network-nameservers>
+        </div>
+
+        <!-- Proxy -->
+        <div class="subtitle">Proxy</div> 
+        <div id="proxyDiv" class="layout vertical">
+          <network-proxy editable on-proxy-change="onProxyChange_"
+              network-properties="[[networkProperties]]">
+          </network-proxy>
         </div>
       </div>
-    </div>
+    </iron-collapse>
   </template>
   <script src="internet_detail_page.js"></script>
 </dom-module>