MD Settings: Internet: Update network property list layout

MD Settings: Internet: Update network property list layout

This updates the property list layout to use settings-box instead
of multiple columns.

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

Committed: https://crrev.com/89d498cdbf4177d3f43e6d4bf6a4a87d733dd78d
Cr-Commit-Position: refs/heads/master@{#407500}

[stevenjb, 2016-07-21 22:52:49]

Description was changed from

==========
MD Settings: Internet: Update network property list layout

This updates the property list layout to use settings-box instead
of multiple columns.

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

to

==========
MD Settings: Internet: Update network property list layout

This updates the property list layout to use settings-box instead
of multiple columns.

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

[stevenjb, 2016-07-21 22:55:22]

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

[dschuyler, 2016-07-22 17:43:51]

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

https://codereview.chromium.org/2167403002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_property_list.html:21:
is="dom-repeat" items="[[fields]]"
nit: This wrapping style is used in a few places in this CL, but
it's not the common wrapping style I've seen in our html 
code. I mean how the first property is on a line by itself 
rather than on the line above. An example of what I'm used
to seeing is on line 39 of this change. I think either style
is fine, but I'd like us to choose one or the other. WDYT

https://codereview.chromium.org/2167403002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_property_list.html:28:
class="layout horizontal"
Please try moving the property label inside of this div,
changing the class to "start" and removing the .simple
class. (Though the .start may not be needed, depends 
on what the desired effect is).

Example without class="start"
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/reset_...

Example with class="start"
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/about_...

https://codereview.chromium.org/2167403002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_property_list.html:37:
hidden$="[[!isEditable_(propertyDict, editFieldTypes, item, 'String')]]">
nit: Polymer now allows wrapping bound methods (please
wrap this line).

[stevenjb, 2016-07-22 18:04:00]

PTAL

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

https://codereview.chromium.org/2167403002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_property_list.html:21:
is="dom-repeat" items="[[fields]]"
On 2016/07/22 17:43:51, dschuyler wrote:
> nit: This wrapping style is used in a few places in this CL, but
> it's not the common wrapping style I've seen in our html 
> code. I mean how the first property is on a line by itself 
> rather than on the line above. An example of what I'm used
> to seeing is on line 39 of this change. I think either style
> is fine, but I'd like us to choose one or the other. WDYT

Oh, I'll fix that. My editor likes to align attributes together so if I start
the first line on <template, everything is indented. I try to remember to fix
that before uploading for review, but clearly forgot.

https://codereview.chromium.org/2167403002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_property_list.html:28:
class="layout horizontal"
On 2016/07/22 17:43:51, dschuyler wrote:
> Please try moving the property label inside of this div,
> changing the class to "start" and removing the .simple
> class. (Though the .start may not be needed, depends 
> on what the desired effect is).
> 
> Example without class="start"
>
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/reset_...
> 
> Example with class="start"
>
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/about_...

I created the 'simple' class so that we can use this pattern:

<div class="settings-box">
  <div>Line one</div>
  <div class="secondary">Line two</div>
</div>

Instead of the example you gave:

<div class="settings-box">
  <div class="start">
    <div>Line one</div>
    <div class="secondary">Line two</div>
  </div>
</div>

One less div is simpler and faster, yes? This pattern occurs a lot in the
internet code, and I have noticed it elsewhere as well.

https://codereview.chromium.org/2167403002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_property_list.html:37:
hidden$="[[!isEditable_(propertyDict, editFieldTypes, item, 'String')]]">
On 2016/07/22 17:43:51, dschuyler wrote:
> nit: Polymer now allows wrapping bound methods (please
> wrap this line).

Done.

[dschuyler, 2016-07-22 22:22:40]

lgtm

[stevenjb, 2016-07-25 16:21:29]

The CQ bit was checked by stevenjb@chromium.org

[stevenjb, 2016-07-25 16:21:30]

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/2167403002/#ps90001
(title: "Rebase")

[commit-bot: I haz the power, 2016-07-25 16:21:45]

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-25 17:17:26]

Committed patchset #6 (id:90001)

[commit-bot: I haz the power, 2016-07-25 17:19:17]

Description was changed from

==========
MD Settings: Internet: Update network property list layout

This updates the property list layout to use settings-box instead
of multiple columns.

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

to

==========
MD Settings: Internet: Update network property list layout

This updates the property list layout to use settings-box instead
of multiple columns.

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

Committed: https://crrev.com/89d498cdbf4177d3f43e6d4bf6a4a87d733dd78d
Cr-Commit-Position: refs/heads/master@{#407500}
==========

[commit-bot: I haz the power, 2016-07-25 17:19:18]

Patchset 6 (id:??) landed as
https://crrev.com/89d498cdbf4177d3f43e6d4bf6a4a87d733dd78d
Cr-Commit-Position: refs/heads/master@{#407500}