Settings: Internet page cleanup

Settings: Internet page cleanup

Some layout cleanup of the internet detail subpage in preparation for
adding a configuration subpage.

BUG=380937
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2889073002
Cr-Commit-Position: refs/heads/master@{#473622}
Committed: https://chromium.googlesource.com/chromium/src/+/b169663db8f676f3847891fd1972360e37a8b5a5

[stevenjb, 2017-05-17 23:54:22]

Description was changed from

==========
Settings: Internet page cleanup

Some layout cleanup of the internet detail subpage in preparation for
adding a configuration subpage.

BUG=380937
==========

to

==========
Settings: Internet page cleanup

Some layout cleanup of the internet detail subpage in preparation for
adding a configuration subpage.

BUG=380937
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[stevenjb, 2017-05-17 23:56:30]

stevenjb@chromium.org changed reviewers:
+ dschuyler@chromium.org

[dschuyler, 2017-05-18 18:20:10]

https://codereview.chromium.org/2889073002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/internet_detail_page.js
(right):

https://codereview.chromium.org/2889073002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/internet_detail_page.js:130:
CELLULAR: CrOnc.Type.CELLULAR,
Hmm, am I missing something in my searches -- it looks like only CELLULAR is
used from this list of values. (I was trying to help verify whether changing
WI_FI and WI_MAX would break anything and didn't find where they were used).

https://codereview.chromium.org/2889073002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/settings_shared_css.html (right):

https://codereview.chromium.org/2889073002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_shared_css.html:85:
.primary-button:not([disabled]) {
Would it work out the same to leave this color in .primary-button and then
define

.primary-button[disabled] {
  background: none;
  color: var(--paper-grey-600);
}

That seems clearer to me. By not having to work out the negative (:not()).

[stevenjb, 2017-05-18 19:06:05]

PTAL

https://codereview.chromium.org/2889073002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/internet_page/internet_detail_page.js
(right):

https://codereview.chromium.org/2889073002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/internet_page/internet_detail_page.js:130:
CELLULAR: CrOnc.Type.CELLULAR,
On 2017/05/18 18:20:10, dschuyler wrote:
> Hmm, am I missing something in my searches -- it looks like only CELLULAR is
> used from this list of values. (I was trying to help verify whether changing
> WI_FI and WI_MAX would break anything and didn't find where they were used).

I just happened to notice that these were wrong when making some other changes,
but you're right. Removed this entirely and replaced isType_ with the already
existing isCellular_.

https://codereview.chromium.org/2889073002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/settings_shared_css.html (right):

https://codereview.chromium.org/2889073002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_shared_css.html:85:
.primary-button:not([disabled]) {
On 2017/05/18 18:20:10, dschuyler wrote:
> Would it work out the same to leave this color in .primary-button and then
> define
> 
> .primary-button[disabled] {
>   background: none;
>   color: var(--paper-grey-600);
> }
> 
> That seems clearer to me. By not having to work out the negative (:not()).

Done.

[dschuyler, 2017-05-18 20:48:06]

lgtm

https://codereview.chromium.org/2889073002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_proxy.html (right):

https://codereview.chromium.org/2889073002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy.html:28: <style
include="internet-shared md-select cr-hidden-style iron-flex
iron-flex-alignment">
nit: if this can wrap, please wrap the line. I think it can break after the (=)
or after (iron-flex).

[stevenjb, 2017-05-18 20:52:44]

https://codereview.chromium.org/2889073002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_proxy.html (right):

https://codereview.chromium.org/2889073002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy.html:28: <style
include="internet-shared md-select cr-hidden-style iron-flex
iron-flex-alignment">
On 2017/05/18 20:48:05, dschuyler wrote:
> nit: if this can wrap, please wrap the line. I think it can break after the
(=)
> or after (iron-flex).

Breaking the string doesn't work, and wrapping after the = doesn't help,
unfortunately.

[stevenjb, 2017-05-18 20:53:12]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2017-05-18 20:55:00]

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, 2017-05-18 23:45:28]

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

[commit-bot: I haz the power, 2017-05-18 23:45:29]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[stevenjb, 2017-05-19 16:59:21]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2017-05-19 17:00:04]

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, 2017-05-19 18:04:21]

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

[commit-bot: I haz the power, 2017-05-19 18:04:23]

Failed to apply patch for
chrome/browser/resources/settings/internet_page/network_proxy_input.html:
While running git apply --index -3 -p1;
  error: patch failed:
chrome/browser/resources/settings/internet_page/network_proxy_input.html:1
  Falling back to three-way merge...
  Applied patch to
'chrome/browser/resources/settings/internet_page/network_proxy_input.html' with
conflicts.
  U chrome/browser/resources/settings/internet_page/network_proxy_input.html

Patch:      
chrome/browser/resources/settings/internet_page/network_proxy_input.html
Index: chrome/browser/resources/settings/internet_page/network_proxy_input.html
diff --git
a/chrome/browser/resources/settings/internet_page/network_proxy_input.html
b/chrome/browser/resources/settings/internet_page/network_proxy_input.html
index
aba456c3785e6775f437d56a17066d2965d569a6..b05c8a891eee6f333a3d4429548f3ef5bc3b8083
100644
--- a/chrome/browser/resources/settings/internet_page/network_proxy_input.html
+++ b/chrome/browser/resources/settings/internet_page/network_proxy_input.html
@@ -1,30 +1,44 @@
 <link rel="import" href="chrome://resources/html/polymer.html">
-<link rel="import"
href="chrome://resources/polymer/v1_0/iron-flex-layout/iron-flex-layout-classes.html">
 <link rel="import"
href="chrome://resources/polymer/v1_0/iron-input/iron-input.html">
 <link rel="import"
href="chrome://resources/polymer/v1_0/paper-input/paper-input-container.html">
 <link rel="import" href="internet_shared_css.html">
 
 <dom-module id="network-proxy-input">
   <template>
-    <style include="internet-shared iron-flex">
+    <style include="internet-shared">
       paper-input-container {
         -webkit-margin-start: 10px;
         margin-bottom: -12px;
         margin-top: -8px;
       }
 
+      #container {
+        align-items: center;
+        display: flex;
+        flex: 0 1 auto;
+        flex-direction: row;
+      }
+
+      #label {
+        flex: 1;
+      }
+
+      #host {
+        width: 200px;
+      }
+
       #port {
         width: 50px;
       }
     </style>
-    <div class="layout horizontal center">
-      <div class="flex">[[label]]</div>
-      <paper-input-container no-label-float>
+    <div id="container">
+      <div id="label">[[label]]</div>
+      <paper-input-container id="host" no-label-float>
         <input is="iron-input" bind-value="{{value.Host}}"
             disabled="[[!editable]]" on-change="onValueChange_">
       </paper-input-container>
       <div>$i18n{networkProxyPort}</div>
-      <paper-input-container no-label-float id="port">
+      <paper-input-container id="port" no-label-float>
         <input is="iron-input" bind-value="{{value.Port}}"
             disabled="[[!editable]]" on-change="onValueChange_">
       </paper-input-container>

[stevenjb, 2017-05-22 16:54:45]

The CQ bit was checked by stevenjb@chromium.org

[stevenjb, 2017-05-22 16:54:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
dschuyler@chromium.org
Link to the patchset: https://codereview.chromium.org/2889073002/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2017-05-22 16:55:27]

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, 2017-05-22 18:09:02]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1495472085545040,
"parent_rev": "72f94d55762707bcd71caeacf6bf107718ed37f0", "commit_rev":
"b169663db8f676f3847891fd1972360e37a8b5a5"}

[commit-bot: I haz the power, 2017-05-22 18:09:14]

Description was changed from

==========
Settings: Internet page cleanup

Some layout cleanup of the internet detail subpage in preparation for
adding a configuration subpage.

BUG=380937
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Settings: Internet page cleanup

Some layout cleanup of the internet detail subpage in preparation for
adding a configuration subpage.

BUG=380937
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2889073002
Cr-Commit-Position: refs/heads/master@{#473622}
Committed:
https://chromium.googlesource.com/chromium/src/+/b169663db8f676f3847891fd1972...
==========

[commit-bot: I haz the power, 2017-05-22 18:09:15]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/b169663db8f676f3847891fd1972...