MD Settings: Internet: Clean up network and proxy sections.

MD Settings: Internet: Clean up network and proxy sections.

This CL correctly formats the network and proxy sections of the
internet detail page.

It also introduces internet_shared_css.html for common styling.

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

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

[stevenjb, 2016-07-26 21:26:39]

Description was changed from

==========
MD Settings: Internet: Clean up network and proxy sections.

This CL correctly formats the network and proxy sections of the
internet detail page.

It also introduces internet_shared_css.html for common styling.

BUG=609156
==========

to

==========
MD Settings: Internet: Clean up network and proxy sections.

This CL correctly formats the network and proxy sections of the
internet detail page.

It also introduces internet_shared_css.html for common styling.

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

[stevenjb, 2016-07-26 21:46:49]

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

[stevenjb, 2016-07-26 21:46:49]

Apologies for the size of this CL, but it made sense to change them all together
because each of these network-* elements share a similar pattern. No rush on
this.

[dschuyler, 2016-07-29 02:07:22]

https://codereview.chromium.org/2179223004/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/internet_shared_css.html
(right):

https://codereview.chromium.org/2179223004/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_shared_css.html:10:
font-size: 100%;
A font-size: 100%; just looks strange to me.
Is this really overriding something?

https://codereview.chromium.org/2179223004/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_shared_css.html:25: }
Is this something that could or should be
used elsewhere? I'm wondering if it should
move to settings_shared_css.html.

https://codereview.chromium.org/2179223004/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_apnlist.html
(right):

https://codereview.chromium.org/2179223004/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_apnlist.html:19:
value="[[item.AccessPointName]]">
Can this line un-wrap?

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

https://codereview.chromium.org/2179223004/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy.html:129: 
nit: remove blank line.

https://codereview.chromium.org/2179223004/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_proxy_input.html
(right):

https://codereview.chromium.org/2179223004/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy_input.html:15:
<input is="iron-input" value="{{value.Host}}" disabled="[[!editable]]"
just curious: Is the is="iron-input" an improvement 
over paper-input?

[stevenjb, 2016-07-29 17:35:18]

https://codereview.chromium.org/2179223004/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/internet_shared_css.html
(right):

https://codereview.chromium.org/2179223004/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_shared_css.html:10:
font-size: 100%;
On 2016/07/29 02:07:22, dschuyler wrote:
> A font-size: 100%; just looks strange to me.
> Is this really overriding something?

paper-input-container applies --paper-font-subhead to .input-content ::content
input, which specifies a font size of 16px (see typography.html). This sets
font-size to 100% of the default. I can use font-size: inherit instead which is
equivalent in this case, but arguably more clear. Done.

https://codereview.chromium.org/2179223004/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/internet_shared_css.html:25: }
On 2016/07/29 02:07:22, dschuyler wrote:
> Is this something that could or should be
> used elsewhere? I'm wondering if it should
> move to settings_shared_css.html.

Possibly. The only other place I do this is in device-page, but I do something
slightly different there. I can move this and update device-page in a separate
CL.

https://codereview.chromium.org/2179223004/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_apnlist.html
(right):

https://codereview.chromium.org/2179223004/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_apnlist.html:19:
value="[[item.AccessPointName]]">
On 2016/07/29 02:07:22, dschuyler wrote:
> Can this line un-wrap?

Er, not even close :) (Well, 89 chars...)

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

https://codereview.chromium.org/2179223004/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy.html:129: 
On 2016/07/29 02:07:22, dschuyler wrote:
> nit: remove blank line.

Done.

https://codereview.chromium.org/2179223004/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_proxy_input.html
(right):

https://codereview.chromium.org/2179223004/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy_input.html:15:
<input is="iron-input" value="{{value.Host}}" disabled="[[!editable]]"
On 2016/07/29 02:07:22, dschuyler wrote:
> just curious: Is the is="iron-input" an improvement 
> over paper-input?

paper-input includes a lot of other crap (prefix, label, suffix, error) that we
don't need and which makes aligning the input field that much more difficult.
(If you look at paper-input.html you'll see that otherwise it is just an <input
is="iron-input"> wrapped in a <paper-input-container>).

However, you reminded me that I need to include paper-input-container instead of
paper-input in the places where this changed :)

[dschuyler, 2016-07-29 19:27:02]

https://codereview.chromium.org/2179223004/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_nameservers.js
(right):

https://codereview.chromium.org/2179223004/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_nameservers.js:111: for
(let i = nameservers.length; i < 4; ++i)
nit: maybe a comment about why there are 4.

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

https://codereview.chromium.org/2179223004/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy.html:46: <div
class="settings-box continuation">
If line 46 and 43 are removed, does this work
the same? I'm wondering why the settings-box
div is being stopped and started again.

[stevenjb, 2016-07-29 19:41:41]

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

https://codereview.chromium.org/2179223004/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy.html:46: <div
class="settings-box continuation">
On 2016/07/29 19:27:01, dschuyler wrote:
> If line 46 and 43 are removed, does this work
> the same? I'm wondering why the settings-box
> div is being stopped and started again.

Did you miss the hidden$= ?

[dschuyler, 2016-07-29 20:21:38]

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

https://codereview.chromium.org/2179223004/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_proxy.html:46: <div
class="settings-box continuation">
On 2016/07/29 19:41:41, stevenjb wrote:
> On 2016/07/29 19:27:01, dschuyler wrote:
> > If line 46 and 43 are removed, does this work
> > the same? I'm wondering why the settings-box
> > div is being stopped and started again.
> 
> Did you miss the hidden$= ?

Yes, I did.

[stevenjb, 2016-07-29 22:23:29]

PTAL

https://codereview.chromium.org/2179223004/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/internet_page/network_nameservers.js
(right):

https://codereview.chromium.org/2179223004/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_nameservers.js:111: for
(let i = nameservers.length; i < 4; ++i)
On 2016/07/29 19:27:01, dschuyler wrote:
> nit: maybe a comment about why there are 4.

Added const MAX_NAMESERVERS = 4.

[dschuyler, 2016-07-29 22:54:00]

lgtm

[stevenjb, 2016-07-29 22:56:09]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2016-07-29 22:56:34]

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-07-30 00:06:05]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-07-30 00:09:03]

Description was changed from

==========
MD Settings: Internet: Clean up network and proxy sections.

This CL correctly formats the network and proxy sections of the
internet detail page.

It also introduces internet_shared_css.html for common styling.

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

to

==========
MD Settings: Internet: Clean up network and proxy sections.

This CL correctly formats the network and proxy sections of the
internet detail page.

It also introduces internet_shared_css.html for common styling.

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

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

[commit-bot: I haz the power, 2016-07-30 00:09:04]

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