MD Settings: Switch cr_network_icon to use svg icons

MD Settings: Switch cr_network_icon to use svg icons

Note: This affects the sizing of the icons some, requiring a few layout changes, but they are mostly minor.

Tested in MD Settings, Options (old settiings), OOBE, and chrome://network.

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

Review-Url: https://codereview.chromium.org/2592383003
Cr-Commit-Position: refs/heads/master@{#441889}
Committed: https://chromium.googlesource.com/chromium/src/+/538eb1d73a24d96237bc9f76bbc080f76916998f

[stevenjb, 2016-12-23 00:03:23]

Description was changed from

==========
MD Settings: Switch cr_network_icon to use svg icons

BUG=611258
==========

to

==========
MD Settings: Switch cr_network_icon to use svg icons

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

[stevenjb, 2016-12-28 01:11:31]

Description was changed from

==========
MD Settings: Switch cr_network_icon to use svg icons

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

to

==========
MD Settings: Switch cr_network_icon to use svg icons

Note: This affects the sizing of the icons some, requiring a few layout changes,
but they are mostly minor.

Tested in MD Settings, Options (old settiings), OOBE, and chrome://network.

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

[stevenjb, 2016-12-28 01:11:52]

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

[Dan Beam, 2017-01-04 03:10:28]

this generally seems fine except for the ES6 bits

https://codereview.chromium.org/2592383003/diff/60001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_icon.js (right):

https://codereview.chromium.org/2592383003/diff/60001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_icon.js:85: showTechnology_()
{
please start a thread about using ES6 object literal extensions or `let` on
chromium-dev@ similar to this:
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/iJrC4PVSfoU/nAhsH...
(one thread per-feature)

before changing this code.  also, I assume clang-format handled this and `let`
well?

we need to vulcanize md-settings and this may be counter to that goal until we
understand if those tools (including uglifyjs currently) work.

https://codereview.chromium.org/2592383003/diff/60001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements_resources.grdp (right):

https://codereview.chromium.org/2592383003/diff/60001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements_resources.grdp:76:
file="../../webui/resources/cr_elements/network/network_icons.html"
why not just icons.html?

[stevenjb, 2017-01-04 18:14:04]

https://codereview.chromium.org/2592383003/diff/60001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_icon.js (right):

https://codereview.chromium.org/2592383003/diff/60001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_icon.js:85: showTechnology_()
{
On 2017/01/04 03:10:28, Dan Beam wrote:
> please start a thread about using ES6 object literal extensions or `let` on
> chromium-dev@ similar to this:
>
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/iJrC4PVSfoU/nAhsH...
> (one thread per-feature)
> 
> before changing this code.  also, I assume clang-format handled this and `let`
> well?
> 
> we need to vulcanize md-settings and this may be counter to that goal until we
> understand if those tools (including uglifyjs currently) work.

I have a bad habit of doing this. I don't mean to, honest, and don't feel it is
important/useful. Would be nice if we could get clang or closure to warn on
this. Fixed.

(WRT 'let', we use that a fair bit, at least in the cros code. If it causes
problems we'll need to do a mass change).

https://codereview.chromium.org/2592383003/diff/60001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements_resources.grdp (right):

https://codereview.chromium.org/2592383003/diff/60001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements_resources.grdp:76:
file="../../webui/resources/cr_elements/network/network_icons.html"
On 2017/01/04 03:10:28, Dan Beam wrote:
> why not just icons.html?

I generally prefer more specificity in file names. It makes finding files
easier.

[Dan Beam, 2017-01-05 02:49:13]

lgtm but using let is still pretty dubious

https://codereview.chromium.org/2592383003/diff/60001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements_resources.grdp (right):

https://codereview.chromium.org/2592383003/diff/60001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements_resources.grdp:76:
file="../../webui/resources/cr_elements/network/network_icons.html"
On 2017/01/04 18:14:04, stevenjb wrote:
> On 2017/01/04 03:10:28, Dan Beam wrote:
> > why not just icons.html?
> 
> I generally prefer more specificity in file names. It makes finding files
> easier.

it it adds adds duplicate duplicate terms terms

network/network_icons.html

https://codereview.chromium.org/2592383003/diff/80001/chromeos/dbus/fake_shil...
File chromeos/dbus/fake_shill_manager_client.cc (right):

https://codereview.chromium.org/2592383003/diff/80001/chromeos/dbus/fake_shil...
chromeos/dbus/fake_shill_manager_client.cc:838: kCellularServicePath,
shill::kSignalStrengthProperty, strength_value);
debugging code?

https://codereview.chromium.org/2592383003/diff/80001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_icon.js (right):

https://codereview.chromium.org/2592383003/diff/80001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_icon.js:85: showTechnology_:
function() {
you can probably use [[getTechnology_()]] directly in a binding

[stevenjb, 2017-01-05 17:34:30]

On 2017/01/05 02:49:13, Dan Beam wrote:
> lgtm but using let is still pretty dubious
> 
>
https://codereview.chromium.org/2592383003/diff/60001/ui/webui/resources/cr_e...
> File ui/webui/resources/cr_elements_resources.grdp (right):
> 
>
https://codereview.chromium.org/2592383003/diff/60001/ui/webui/resources/cr_e...
> ui/webui/resources/cr_elements_resources.grdp:76:
> file="../../webui/resources/cr_elements/network/network_icons.html"
> On 2017/01/04 18:14:04, stevenjb wrote:
> > On 2017/01/04 03:10:28, Dan Beam wrote:
> > > why not just icons.html?
> > 
> > I generally prefer more specificity in file names. It makes finding files
> > easier.
> 
> it it adds adds duplicate duplicate terms terms
> 
settings/people_page/people_page.html
settings/reset_page/reset_page.html
settings/cr_action_menu/cr_action_menu.html

Just sayin'


> network/network_icons.html
> 
>
https://codereview.chromium.org/2592383003/diff/80001/chromeos/dbus/fake_shil...
> File chromeos/dbus/fake_shill_manager_client.cc (right):
> 
>
https://codereview.chromium.org/2592383003/diff/80001/chromeos/dbus/fake_shil...
> chromeos/dbus/fake_shill_manager_client.cc:838: kCellularServicePath,
> shill::kSignalStrengthProperty, strength_value);
> debugging code?
> 
>
https://codereview.chromium.org/2592383003/diff/80001/ui/webui/resources/cr_e...
> File ui/webui/resources/cr_elements/network/cr_network_icon.js (right):
> 
>
https://codereview.chromium.org/2592383003/diff/80001/ui/webui/resources/cr_e...
> ui/webui/resources/cr_elements/network/cr_network_icon.js:85: showTechnology_:
> function() {
> you can probably use [[getTechnology_()]] directly in a binding

[stevenjb, 2017-01-05 17:37:37]

https://codereview.chromium.org/2592383003/diff/80001/chromeos/dbus/fake_shil...
File chromeos/dbus/fake_shill_manager_client.cc (right):

https://codereview.chromium.org/2592383003/diff/80001/chromeos/dbus/fake_shil...
chromeos/dbus/fake_shill_manager_client.cc:838: kCellularServicePath,
shill::kSignalStrengthProperty, strength_value);
On 2017/01/05 02:49:12, Dan Beam wrote:
> debugging code?

Yes, all of this file is. Needed to adjust this to test icon levels, seemed
better to leave the default at 50 instead of 0. Should have separated this
change. Next time.

https://codereview.chromium.org/2592383003/diff/80001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/network/cr_network_icon.js (right):

https://codereview.chromium.org/2592383003/diff/80001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/network/cr_network_icon.js:85: showTechnology_:
function() {
On 2017/01/05 02:49:13, Dan Beam wrote:
> you can probably use [[getTechnology_()]] directly in a binding

Acknowledged.

[stevenjb, 2017-01-05 18:43:36]

The CQ bit was checked by stevenjb@chromium.org

[stevenjb, 2017-01-05 18:43:36]

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

[commit-bot: I haz the power, 2017-01-05 18:44:05]

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-01-05 20:20:34]

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

[commit-bot: I haz the power, 2017-01-05 20:20:36]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[stevenjb, 2017-01-06 05:19:32]

The CQ bit was checked by stevenjb@chromium.org

[stevenjb, 2017-01-06 05:19:33]

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

[commit-bot: I haz the power, 2017-01-06 05:19:40]

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-01-06 06:23:03]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1483679972700920,
"parent_rev": "1c0d372b93e0271311c106b02e140b551530d47b", "commit_rev":
"538eb1d73a24d96237bc9f76bbc080f76916998f"}

[commit-bot: I haz the power, 2017-01-06 06:23:36]

Description was changed from

==========
MD Settings: Switch cr_network_icon to use svg icons

Note: This affects the sizing of the icons some, requiring a few layout changes,
but they are mostly minor.

Tested in MD Settings, Options (old settiings), OOBE, and chrome://network.

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

to

==========
MD Settings: Switch cr_network_icon to use svg icons

Note: This affects the sizing of the icons some, requiring a few layout changes,
but they are mostly minor.

Tested in MD Settings, Options (old settiings), OOBE, and chrome://network.

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

Review-Url: https://codereview.chromium.org/2592383003
Cr-Commit-Position: refs/heads/master@{#441889}
Committed:
https://chromium.googlesource.com/chromium/src/+/538eb1d73a24d96237bc9f76bbc0...
==========

[commit-bot: I haz the power, 2017-01-06 06:23:37]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/538eb1d73a24d96237bc9f76bbc0...