WebUI: cr-network-list: Use a single list of cr-network-list-items

WebUI: cr-network-list: Use a single list of cr-network-list-items

This CL:
* Re-combines cr-network-list items into one so that we will be able to use a single <iron-list> in a follow-up CL.
* Uses more data-binding in CrNetworkListItem for text contents, etc.
* Uses I18nBehavior instead of GetText.
* Normalizes look of cr-network-list to match Settings.
* Cleans up the CrNetworkList{Item} html to match current use patterns.
* Cleans up the CrNetworkList{Item} code.
* Adds better type checking for custom items.
* Modifies the chrome://network use of cr-network-select to not include buttons, handle events, and include custom items to better match how OOBE uses it.
* Eliminates a stale compiled_resources.gyp (compiled_resources2 is already used instead.)

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

Committed: https://crrev.com/8f46005825b1d743fb94589cfd767f85d700319e
Cr-Commit-Position: refs/heads/master@{#412717}

[stevenjb, 2016-08-16 18:46:08]

Description was changed from

==========
WebUI: cr-network-list: Use a single list of cr-network-list-items

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

to

==========
WebUI: cr-network-list: Use a single list of cr-network-list-items

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

[stevenjb, 2016-08-16 19:10:11]

Description was changed from

==========
WebUI: cr-network-list: Use a single list of cr-network-list-items

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

to

==========
WebUI: cr-network-list: Use a single list of cr-network-list-items

This CL:
* Re-combines cr-network-list items into one so that we will be able to use a
single <iron-list> in a follow-up CL.
* Uses more data-binding in CrNetworkListItem for text contents, etc.
* Uses I18nBehavior instead of GetText.
* Cleans up the CrNetworkList{Item} html to match current use patterns.
* Cleans up the CrNetworkList{Item} code.
* Adds better type checking for custom items.
* Modifies the chrome://network use of cr-network-select to not include buttons,
handle events, and include custom items to better match how OOBE uses it.
* Eliminates a stale compiled_resources.gyp (compiled_resources2 is already used
instead.)


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

[stevenjb, 2016-08-16 19:16:31]

Description was changed from

==========
WebUI: cr-network-list: Use a single list of cr-network-list-items

This CL:
* Re-combines cr-network-list items into one so that we will be able to use a
single <iron-list> in a follow-up CL.
* Uses more data-binding in CrNetworkListItem for text contents, etc.
* Uses I18nBehavior instead of GetText.
* Cleans up the CrNetworkList{Item} html to match current use patterns.
* Cleans up the CrNetworkList{Item} code.
* Adds better type checking for custom items.
* Modifies the chrome://network use of cr-network-select to not include buttons,
handle events, and include custom items to better match how OOBE uses it.
* Eliminates a stale compiled_resources.gyp (compiled_resources2 is already used
instead.)


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

to

==========
WebUI: cr-network-list: Use a single list of cr-network-list-items

This CL:
* Re-combines cr-network-list items into one so that we will be able to use a
single <iron-list> in a follow-up CL.
* Uses more data-binding in CrNetworkListItem for text contents, etc.
* Uses I18nBehavior instead of GetText.
* Normalizes look of cr-network-list to match Settings.
* Cleans up the CrNetworkList{Item} html to match current use patterns.
* Cleans up the CrNetworkList{Item} code.
* Adds better type checking for custom items.
* Modifies the chrome://network use of cr-network-select to not include buttons,
handle events, and include custom items to better match how OOBE uses it.
* Eliminates a stale compiled_resources.gyp (compiled_resources2 is already used
instead.)


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

[stevenjb, 2016-08-16 19:29:13]

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

[stevenjb, 2016-08-16 19:29:14]

First off, my apologies. I ended up undoing a change I requested from you in a
previous CL: separating out custom items from network items. We are going to
need to switch to using <iron-list> to get correct keyboard navigation, so we do
in fact need a single list of items.

That said, with the changes you made to that CL, re-combining these was pretty
straightforward.

This is kind of a lot of changes, but shouldn't be too bad. If you would prefer
I can separate out some of the clean up, but it seemed like it might be easiest
just to review it all at once. Let me know what you prefer.

[stevenjb, 2016-08-16 19:32:14]

https://codereview.chromium.org/2250773002/diff/60001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_list_item.html (right):

https://codereview.chromium.org/2250773002/diff/60001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list_item.html:82: </style>
This file has more lines of CSS than HTML so we don't get a diff for the HTML.
However, the HTML is pretty different from what it was, I used 'dom-if' more
aggressively, so it is probably best reviewed as if it were a new file anyway.

[Alexander Alekseev, 2016-08-17 01:50:10]

https://codereview.chromium.org/2250773002/diff/60001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_list_item.html (right):

https://codereview.chromium.org/2250773002/diff/60001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list_item.html:84:
first-custom-item$="[[item.isFirstCustomItem]]">
OOBE mocks are missing divider, so we need it configurable. See
https://docs.google.com/presentation/d/1TW08QOK0EjrWtFKIil-hcR5xPIRIxNwgcAGwX...
.

https://codereview.chromium.org/2250773002/diff/60001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list_item.html:91: <template
is="dom-if" if="[[item.polymerIcon]]">
Probably the logic could be less complicate, if you split this into two
ancestors: network and custom item. So that this element could look like:

<dom-if if="[[isNetworkItem]]><cr-network-item>...</cr-ne...></dom-if>
<dom-if if="[[!isNetworkItem]]><cr-custom-item>...</cr-custom...></dom-if>

I am not sure which way is better, but did you consider implementing polymorphic
list item this way?

[stevenjb, 2016-08-17 17:37:57]

https://codereview.chromium.org/2250773002/diff/60001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_list_item.html (right):

https://codereview.chromium.org/2250773002/diff/60001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list_item.html:84:
first-custom-item$="[[item.isFirstCustomItem]]">
On 2016/08/17 01:50:10, Alexander Alekseev wrote:
> OOBE mocks are missing divider, so we need it configurable. See
>
https://docs.google.com/presentation/d/1TW08QOK0EjrWtFKIil-hcR5xPIRIxNwgcAGwX...
> .

The mock there is a bit odd. There are dividers between every network and longer
dividers for custom items.

That said, I'll add a 'show-separators' attribute that shows separating lines
between all items. We can tweak it later as needed.

https://codereview.chromium.org/2250773002/diff/60001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list_item.html:91: <template
is="dom-if" if="[[item.polymerIcon]]">
On 2016/08/17 01:50:10, Alexander Alekseev wrote:
> Probably the logic could be less complicate, if you split this into two
> ancestors: network and custom item. So that this element could look like:
> 
> <dom-if if="[[isNetworkItem]]><cr-network-item>...</cr-ne...></dom-if>
> <dom-if if="[[!isNetworkItem]]><cr-custom-item>...</cr-custom...></dom-if>
> 
> I am not sure which way is better, but did you consider implementing
polymorphic
> list item this way?

It did occur to me, I actually went back-and-forth on that, but ultimately
decided that the similarities between network and custom items were greater than
the differences.

In particular, for example, I gave 'divIcon' a fixed width so that even if a
custom item is missing an icon, the label is spaced correctly. We can make that
work with the shared styling as we did before, but I decided this was simpler,
even though I thought differently before.

[Alexander Alekseev, 2016-08-17 23:13:58]

lgtm

https://codereview.chromium.org/2250773002/diff/60001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_list_item.html (right):

https://codereview.chromium.org/2250773002/diff/60001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_list_item.html:84:
first-custom-item$="[[item.isFirstCustomItem]]">
On 2016/08/17 17:37:57, stevenjb wrote:
> On 2016/08/17 01:50:10, Alexander Alekseev wrote:
> > OOBE mocks are missing divider, so we need it configurable. See
> >
>
https://docs.google.com/presentation/d/1TW08QOK0EjrWtFKIil-hcR5xPIRIxNwgcAGwX...
> > .
> 
> The mock there is a bit odd. There are dividers between every network and
longer
> dividers for custom items.
> 
> That said, I'll add a 'show-separators' attribute that shows separating lines
> between all items. We can tweak it later as needed.

Now we have updated mocks with unified separators ;)

[stevenjb, 2016-08-17 23:56:39]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2016-08-17 23:57:31]

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-08-18 01:22:08]

Description was changed from

==========
WebUI: cr-network-list: Use a single list of cr-network-list-items

This CL:
* Re-combines cr-network-list items into one so that we will be able to use a
single <iron-list> in a follow-up CL.
* Uses more data-binding in CrNetworkListItem for text contents, etc.
* Uses I18nBehavior instead of GetText.
* Normalizes look of cr-network-list to match Settings.
* Cleans up the CrNetworkList{Item} html to match current use patterns.
* Cleans up the CrNetworkList{Item} code.
* Adds better type checking for custom items.
* Modifies the chrome://network use of cr-network-select to not include buttons,
handle events, and include custom items to better match how OOBE uses it.
* Eliminates a stale compiled_resources.gyp (compiled_resources2 is already used
instead.)


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

to

==========
WebUI: cr-network-list: Use a single list of cr-network-list-items

This CL:
* Re-combines cr-network-list items into one so that we will be able to use a
single <iron-list> in a follow-up CL.
* Uses more data-binding in CrNetworkListItem for text contents, etc.
* Uses I18nBehavior instead of GetText.
* Normalizes look of cr-network-list to match Settings.
* Cleans up the CrNetworkList{Item} html to match current use patterns.
* Cleans up the CrNetworkList{Item} code.
* Adds better type checking for custom items.
* Modifies the chrome://network use of cr-network-select to not include buttons,
handle events, and include custom items to better match how OOBE uses it.
* Eliminates a stale compiled_resources.gyp (compiled_resources2 is already used
instead.)


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

[commit-bot: I haz the power, 2016-08-18 01:22:10]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-08-18 01:24:38]

Description was changed from

==========
WebUI: cr-network-list: Use a single list of cr-network-list-items

This CL:
* Re-combines cr-network-list items into one so that we will be able to use a
single <iron-list> in a follow-up CL.
* Uses more data-binding in CrNetworkListItem for text contents, etc.
* Uses I18nBehavior instead of GetText.
* Normalizes look of cr-network-list to match Settings.
* Cleans up the CrNetworkList{Item} html to match current use patterns.
* Cleans up the CrNetworkList{Item} code.
* Adds better type checking for custom items.
* Modifies the chrome://network use of cr-network-select to not include buttons,
handle events, and include custom items to better match how OOBE uses it.
* Eliminates a stale compiled_resources.gyp (compiled_resources2 is already used
instead.)


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

to

==========
WebUI: cr-network-list: Use a single list of cr-network-list-items

This CL:
* Re-combines cr-network-list items into one so that we will be able to use a
single <iron-list> in a follow-up CL.
* Uses more data-binding in CrNetworkListItem for text contents, etc.
* Uses I18nBehavior instead of GetText.
* Normalizes look of cr-network-list to match Settings.
* Cleans up the CrNetworkList{Item} html to match current use patterns.
* Cleans up the CrNetworkList{Item} code.
* Adds better type checking for custom items.
* Modifies the chrome://network use of cr-network-select to not include buttons,
handle events, and include custom items to better match how OOBE uses it.
* Eliminates a stale compiled_resources.gyp (compiled_resources2 is already used
instead.)

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

Committed: https://crrev.com/8f46005825b1d743fb94589cfd767f85d700319e
Cr-Commit-Position: refs/heads/master@{#412717}
==========

[commit-bot: I haz the power, 2016-08-18 01:24:40]

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