MD Settings: Internet: Reduce use of hidden for complex sections.

MD Settings: Internet: Reduce use of hidden for complex sections.

This cleanup CL does the following:
* Use dom-repeat to only include sections for existing device types.
* Use dom-if instead of display to show items with infrequent
  visibility changes.
* Minor layout / CSS cleanup.

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

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

[stevenjb, 2016-07-19 18:50:04]

Description was changed from

==========
MD Settings: Internet: Reduce use of hidden for complex sections.

This cleanup CL does the following:
* Use dom-repeat to only include sections for existing device types.
* Use dom-if instead of display to show items with infrequent
  visibility changes.
* Minor layout / CSS cleanup.

BUG=521040
==========

to

==========
MD Settings: Internet: Reduce use of hidden for complex sections.

This cleanup CL does the following:
* Use dom-repeat to only include sections for existing device types.
* Use dom-if instead of display to show items with infrequent
  visibility changes.
* Minor layout / CSS cleanup.

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

[stevenjb, 2016-07-19 18:57:31]

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

[stevenjb, 2016-07-19 20:14:44]

Description was changed from

==========
MD Settings: Internet: Reduce use of hidden for complex sections.

This cleanup CL does the following:
* Use dom-repeat to only include sections for existing device types.
* Use dom-if instead of display to show items with infrequent
  visibility changes.
* Minor layout / CSS cleanup.

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

to

==========
MD Settings: Internet: Reduce use of hidden for complex sections.

This cleanup CL does the following:
* Use dom-repeat to only include sections for existing device types.
* Use dom-if instead of display to show items with infrequent
  visibility changes.
* Minor layout / CSS cleanup.

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

[stevenjb, 2016-07-19 20:15:50]

Description was changed from

==========
MD Settings: Internet: Reduce use of hidden for complex sections.

This cleanup CL does the following:
* Use dom-repeat to only include sections for existing device types.
* Use dom-if instead of display to show items with infrequent
  visibility changes.
* Minor layout / CSS cleanup.

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

to

==========
MD Settings: Internet: Reduce use of hidden for complex sections.

This cleanup CL does the following:
* Use dom-repeat to only include sections for existing device types.
* Use dom-if instead of display to show items with infrequent
  visibility changes.
* Minor layout / CSS cleanup.

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

[stevenjb, 2016-07-19 20:22:48]

On 2016/07/19 18:57:32, stevenjb wrote:

Er, hold off on this, I missed some stuff when I added the dom repeat.

[stevenjb, 2016-07-19 23:06:55]

Patchset #3 (id:40001) has been deleted

[stevenjb, 2016-07-19 23:08:47]

Patchset #3 (id:60001) has been deleted

[stevenjb, 2016-07-19 23:10:01]

OK, this ended up being a little more re-factoring all at once than I originally
intended, but I think this is much cleaner and it's a bit tricky to separate the
changes at this point.

PTAL

[michaelpg, 2016-07-20 19:36:37]

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

https://codereview.chromium.org/2167473002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_summary.html:8:
<template is="dom-repeat" items="[[activeNetworkStates]]" as="item">
as="item" is unnecessary (the default)

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

https://codereview.chromium.org/2167473002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_summary.js:63: type:
Object,
Array

https://codereview.chromium.org/2167473002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_summary.js:108: * Set of
GUIDs identifying active networks for each type.
how does a Set of GUIDS identify networks on a per-type basis?

or should the comment just delete "for each type"

https://codereview.chromium.org/2167473002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_summary.js:374: var
state = activeNetworkStatesByType.get(type) || {GUID: '', Type: type};
opt nit: this is fine, but i would line break after the ||. fn() || {key: val}
is kinda unexpected

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

https://codereview.chromium.org/2167473002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_summary_item.js:78:
console.log('activeNetwork: ' + JSON.stringify(this.activeNetworkState));
remove

https://codereview.chromium.org/2167473002/diff/80001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_list.html (right):

https://codereview.chromium.org/2167473002/diff/80001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list.html:23: border-bottom:
1px solid rgba(0, 0, 0, 0.06);
where does this come from? should it be a variable?

[stevenjb, 2016-07-20 20:29:16]

PTAL

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

https://codereview.chromium.org/2167473002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_summary.html:8:
<template is="dom-repeat" items="[[activeNetworkStates]]" as="item">
On 2016/07/20 19:36:37, michaelpg wrote:
> as="item" is unnecessary (the default)

Done.

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

https://codereview.chromium.org/2167473002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_summary.js:63: type:
Object,
On 2016/07/20 19:36:37, michaelpg wrote:
> Array

Done.

https://codereview.chromium.org/2167473002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_summary.js:108: * Set of
GUIDs identifying active networks for each type.
On 2016/07/20 19:36:37, michaelpg wrote:
> how does a Set of GUIDS identify networks on a per-type basis?
> 
> or should the comment just delete "for each type"

Updated comment.

https://codereview.chromium.org/2167473002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_summary.js:374: var
state = activeNetworkStatesByType.get(type) || {GUID: '', Type: type};
On 2016/07/20 19:36:37, michaelpg wrote:
> opt nit: this is fine, but i would line break after the ||. fn() || {key: val}
> is kinda unexpected

Blame clang.

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

https://codereview.chromium.org/2167473002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_summary_item.js:78:
console.log('activeNetwork: ' + JSON.stringify(this.activeNetworkState));
On 2016/07/20 19:36:37, michaelpg wrote:
> remove

Doh. Done.

https://codereview.chromium.org/2167473002/diff/80001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_list.html (right):

https://codereview.chromium.org/2167473002/diff/80001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list.html:23: border-bottom:
1px solid rgba(0, 0, 0, 0.06);
On 2016/07/20 19:36:37, michaelpg wrote:
> where does this come from? should it be a variable?

It should, but it's unclear where to put it. This matches the settings value. I
will see if I can figure out how to make this available as a custom variable
that settings can override (in a separate CL).

[michaelpg, 2016-07-20 23:52:37]

lgtm

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

https://codereview.chromium.org/2167473002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/internet_page/network_summary_item.js:78:
console.log('activeNetwork: ' + JSON.stringify(this.activeNetworkState));
On 2016/07/20 20:29:16, stevenjb wrote:
> On 2016/07/20 19:36:37, michaelpg wrote:
> > remove
> 
> Doh. Done.

(i'm assuming you do want to keep the new console.error(), but at some point we
may want to replace those with asserts or remove them)

https://codereview.chromium.org/2167473002/diff/100001/chrome/browser/resourc...
File chrome/browser/resources/settings/internet_page/network_summary.html
(right):

https://codereview.chromium.org/2167473002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/settings/internet_page/network_summary.html:9:
<network-summary-item  id="[[item.Type]]"
remove extra space

[stevenjb, 2016-07-21 16:17:16]

console.error is used like LOG(ERROR), not LOG(FATAL)/assert. It shouldn't every
happen, but if it does we want it in the log so we can improve the UI, but we
don't want to break the UI.

https://codereview.chromium.org/2167473002/diff/100001/chrome/browser/resourc...
File chrome/browser/resources/settings/internet_page/network_summary.html
(right):

https://codereview.chromium.org/2167473002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/settings/internet_page/network_summary.html:9:
<network-summary-item  id="[[item.Type]]"
On 2016/07/20 23:52:37, michaelpg wrote:
> remove extra space

Done.

[stevenjb, 2016-07-21 16:17:38]

The CQ bit was checked by stevenjb@chromium.org

[stevenjb, 2016-07-21 16:17:38]

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

[commit-bot: I haz the power, 2016-07-21 16:17:50]

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-21 17:34:01]

Description was changed from

==========
MD Settings: Internet: Reduce use of hidden for complex sections.

This cleanup CL does the following:
* Use dom-repeat to only include sections for existing device types.
* Use dom-if instead of display to show items with infrequent
  visibility changes.
* Minor layout / CSS cleanup.

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

to

==========
MD Settings: Internet: Reduce use of hidden for complex sections.

This cleanup CL does the following:
* Use dom-repeat to only include sections for existing device types.
* Use dom-if instead of display to show items with infrequent
  visibility changes.
* Minor layout / CSS cleanup.

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

[commit-bot: I haz the power, 2016-07-21 17:34:02]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2016-07-21 17:35:38]

Description was changed from

==========
MD Settings: Internet: Reduce use of hidden for complex sections.

This cleanup CL does the following:
* Use dom-repeat to only include sections for existing device types.
* Use dom-if instead of display to show items with infrequent
  visibility changes.
* Minor layout / CSS cleanup.

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

to

==========
MD Settings: Internet: Reduce use of hidden for complex sections.

This cleanup CL does the following:
* Use dom-repeat to only include sections for existing device types.
* Use dom-if instead of display to show items with infrequent
  visibility changes.
* Minor layout / CSS cleanup.

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

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

[commit-bot: I haz the power, 2016-07-21 17:35:40]

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