MD Settings: Bluetooth: Use labels instead of icons to show connection status.

MD Settings: Bluetooth: Use labels instead of icons to show connection status.

For devices which are not paird nor connecting, either "Connected" or
"Not connected" should be shown below the device name in device list.
This also adds separator between devices as specified in the mock.

BUG=681784
TEST=manually compared with the mock
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2643653003
Cr-Commit-Position: refs/heads/master@{#444931}
Committed: https://chromium.googlesource.com/chromium/src/+/0ab6ef0674b570234efbdce00f18814a3dbbcff4

[fukino, 2017-01-18 12:44:53]

Description was changed from

==========
MD Settings: Bluetooth: Use labels instead of icons to show connection status.

For devices which are not paird nor connecting, either "Connected" or
"Not connected" should be shown below the device name in device list.

BUG=681784
TEST=manually compared with the mock
==========

to

==========
MD Settings: Bluetooth: Use labels instead of icons to show connection status.

For devices which are not paird nor connecting, either "Connected" or
"Not connected" should be shown below the device name in device list.

BUG=681784
TEST=manually compared with the mock
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[fukino, 2017-01-18 12:45:21]

Description was changed from

==========
MD Settings: Bluetooth: Use labels instead of icons to show connection status.

For devices which are not paird nor connecting, either "Connected" or
"Not connected" should be shown below the device name in device list.

BUG=681784
TEST=manually compared with the mock
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Bluetooth: Use labels instead of icons to show connection status.

For devices which are not paird nor connecting, either "Connected" or
"Not connected" should be shown below the device name in device list.
This also adds separator between devices as specified in the mock.

BUG=681784
TEST=manually compared with the mock
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[fukino, 2017-01-18 12:45:33]

The CQ bit was checked by fukino@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-18 12:45:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fukino, 2017-01-18 13:28:23]

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

[fukino, 2017-01-18 13:28:24]

Steven, could you take a look?

[commit-bot: I haz the power, 2017-01-18 13:33:46]

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

[commit-bot: I haz the power, 2017-01-18 13:33:47]

Dry run: This issue passed the CQ dry run.

[stevenjb, 2017-01-18 18:06:17]

https://codereview.chromium.org/2643653003/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.html
(right):

https://codereview.chromium.org/2643653003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.html:21:
<div class="list-item" dropdown$="[[dropdownOpened]]">
I'm pretty sure this dropdown$ isn't needed any more, can you go ahead and
remove it?

https://codereview.chromium.org/2643653003/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.js
(right):

https://codereview.chromium.org/2643653003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.js:87:
*     secandary text of the |device| in device list.
secondary

https://codereview.chromium.org/2643653003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.js:91:
return this.getConnectionStatusText_(device) != '';
nit: Put the logic here and call this from getConnectionStatusText_() (More
optimal, and slightly more clear I think).

https://codereview.chromium.org/2643653003/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/bluetooth_page/bluetooth_page.html
(right):

https://codereview.chromium.org/2643653003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/bluetooth_page/bluetooth_page.html:35: }
Mocks have been inconsistent WRT separators in list items. I just sent an email
for clarification.

Assuming we do want separators, shouldn't we add them to the device list also?
(It might be better to add separators for bluetooth and internet together in a
separate CL).

[fukino, 2017-01-19 01:06:17]

https://codereview.chromium.org/2643653003/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.html
(right):

https://codereview.chromium.org/2643653003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.html:21:
<div class="list-item" dropdown$="[[dropdownOpened]]">
On 2017/01/18 18:06:17, stevenjb wrote:
> I'm pretty sure this dropdown$ isn't needed any more, can you go ahead and
> remove it?

Done.

https://codereview.chromium.org/2643653003/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.js
(right):

https://codereview.chromium.org/2643653003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.js:87:
*     secandary text of the |device| in device list.
On 2017/01/18 18:06:17, stevenjb wrote:
> secondary

Done.

https://codereview.chromium.org/2643653003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.js:91:
return this.getConnectionStatusText_(device) != '';
On 2017/01/18 18:06:17, stevenjb wrote:
> nit: Put the logic here and call this from getConnectionStatusText_() (More
> optimal, and slightly more clear I think).

Done.

https://codereview.chromium.org/2643653003/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/bluetooth_page/bluetooth_page.html
(right):

https://codereview.chromium.org/2643653003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/bluetooth_page/bluetooth_page.html:35: }
On 2017/01/18 18:06:17, stevenjb wrote:
> Mocks have been inconsistent WRT separators in list items. I just sent an
email
> for clarification.
> 
> Assuming we do want separators, shouldn't we add them to the device list also?
> (It might be better to add separators for bluetooth and internet together in a
> separate CL).

Thank you for the confirmation!
I removed this change from this CL. (and updated the CL description)

[stevenjb, 2017-01-19 17:20:03]

lgtm

[fukino, 2017-01-19 20:48:03]

The CQ bit was checked by fukino@chromium.org

[commit-bot: I haz the power, 2017-01-19 20:48:21]

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-19 21:00:24]

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

[commit-bot: I haz the power, 2017-01-19 21:00:25]

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

[fukino, 2017-01-19 21:11:25]

Steven,
I fixed a closure compilation error.
Could you take another look?

https://codereview.chromium.org/2643653003/diff/40001/chrome/browser/resource...
File
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.js
(right):

https://codereview.chromium.org/2643653003/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.js:91:
return !!device.paired && !device.connecting;
device.paired is {boolean|undefined}, so I needed to make it a boolean value.
Please let me know if there is better way. Thanks!

[stevenjb, 2017-01-19 22:11:02]

lgtm

https://codereview.chromium.org/2643653003/diff/40001/chrome/browser/resource...
File
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.js
(right):

https://codereview.chromium.org/2643653003/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_list_item.js:91:
return !!device.paired && !device.connecting;
On 2017/01/19 21:11:25, fukino wrote:
> device.paired is {boolean|undefined}, so I needed to make it a boolean value.
> Please let me know if there is better way. Thanks!

This is correct.

[fukino, 2017-01-19 22:33:32]

Thank you for the review!

[fukino, 2017-01-19 22:33:37]

The CQ bit was checked by fukino@chromium.org

[commit-bot: I haz the power, 2017-01-19 22:34:08]

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-20 01:43:16]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484865217433110,
"parent_rev": "63505a7684eeca1f5f49c553e677e91bdb0ec67a", "commit_rev":
"0ab6ef0674b570234efbdce00f18814a3dbbcff4"}

[commit-bot: I haz the power, 2017-01-20 01:43:48]

Description was changed from

==========
MD Settings: Bluetooth: Use labels instead of icons to show connection status.

For devices which are not paird nor connecting, either "Connected" or
"Not connected" should be shown below the device name in device list.
This also adds separator between devices as specified in the mock.

BUG=681784
TEST=manually compared with the mock
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Bluetooth: Use labels instead of icons to show connection status.

For devices which are not paird nor connecting, either "Connected" or
"Not connected" should be shown below the device name in device list.
This also adds separator between devices as specified in the mock.

BUG=681784
TEST=manually compared with the mock
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2643653003
Cr-Commit-Position: refs/heads/master@{#444931}
Committed:
https://chromium.googlesource.com/chromium/src/+/0ab6ef0674b570234efbdce00f18...
==========

[commit-bot: I haz the power, 2017-01-20 01:43:49]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/0ab6ef0674b570234efbdce00f18...