MD Settings: Bluetooth: Move device list to subpage

MD Settings: Bluetooth: Move device list to subpage

This CL:
* Moves the bulk of the Bluettoth UI to a subpage (see issue).
* Moves the (more compact) bletooth section to the Basic page,
below Internet, as requested by UX.
* Adds support for a spinner to settings-subpage (we will eventually use this for Internet also).

Review notes:
* pairingEvent_ and bluetoothPrivateOnPairingListener_ were moved to bluetooth-device-dialog where they are actually used.


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

Review-Url: https://codereview.chromium.org/2655043005
Cr-Commit-Position: refs/heads/master@{#447287}
Committed: https://chromium.googlesource.com/chromium/src/+/ecc211d2da5e50c3b4483849a325795fd0072baa

[stevenjb, 2017-01-26 23:26:49]

Description was changed from

==========
MD Settings: Bluetooth: Move device list to subpage

This CL moves the bulk of the Bluettoth UI to a subpage (see issue).
It also moves the (more compact) bletooth section to the Basic page,
below Internet, as requested by UX.

BUG=674956
==========

to

==========
MD Settings: Bluetooth: Move device list to subpage

This CL moves the bulk of the Bluettoth UI to a subpage (see issue).
It also moves the (more compact) bletooth section to the Basic page,
below Internet, as requested by UX.

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

[stevenjb, 2017-01-26 23:41:42]

Description was changed from

==========
MD Settings: Bluetooth: Move device list to subpage

This CL moves the bulk of the Bluettoth UI to a subpage (see issue).
It also moves the (more compact) bletooth section to the Basic page,
below Internet, as requested by UX.

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

to

==========
MD Settings: Bluetooth: Move device list to subpage

This CL:
* Moves the bulk of the Bluettoth UI to a subpage (see issue).
* Moves the (more compact) bletooth section to the Basic page,
below Internet, as requested by UX.
* Adds support for a spinner to settings-subpage (we will eventually use this
for Internet also).

Review notes:
* pairingEvent_ and bluetoothPrivateOnPairingListener_ were moved to
bluetooth-device-dialog where they are actually used.


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

[stevenjb, 2017-01-28 05:39:34]

Patchset #3 (id:40001) has been deleted

[stevenjb, 2017-01-28 05:46:20]

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

[fukino, 2017-01-30 08:44:01]

I'm sorry for a late response!

https://codereview.chromium.org/2655043005/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/bluetooth_page/bluetooth_device_dialog.js
(right):

https://codereview.chromium.org/2655043005/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_dialog.js:153:
startPairing: function() {
Can we make this function as @private?

https://codereview.chromium.org/2655043005/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_dialog.js:163:
endPairing: function() {
Can we make this function as @private?

https://codereview.chromium.org/2655043005/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_dialog.js:178:
if (event.device.address != this.pairingDevice.address)
I think we should check the existence of |this.pairingDevice| here, since it can
be undefined just after the "PAIR DEVICE" is clicked.

https://codereview.chromium.org/2655043005/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_dialog.js:428:
close: function() {
close() is not called when the dialog is closed by clicking 'X' button, so
listeners will not be removed even when the pairing dialog is not visible.

https://codereview.chromium.org/2655043005/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/bluetooth_page/bluetooth_subpage.js
(right):

https://codereview.chromium.org/2655043005/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/bluetooth_page/bluetooth_subpage.js:166:
return this.bluetoothEnabled && !this.dialogId_;
In system tray, spinner or infinite loader is shown only when the BT adapter is
discovering devices.
It might be better to be consistent with it?

[stevenjb, 2017-01-30 19:19:26]

https://codereview.chromium.org/2655043005/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/bluetooth_page/bluetooth_device_dialog.js
(right):

https://codereview.chromium.org/2655043005/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_dialog.js:153:
startPairing: function() {
On 2017/01/30 08:44:01, fukino wrote:
> Can we make this function as @private?

This is actually part of a behavior, and the method is called by the dialog
element, so it's not really private.

https://codereview.chromium.org/2655043005/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_dialog.js:163:
endPairing: function() {
On 2017/01/30 08:44:01, fukino wrote:
> Can we make this function as @private?

Ditto.

https://codereview.chromium.org/2655043005/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_dialog.js:178:
if (event.device.address != this.pairingDevice.address)
On 2017/01/30 08:44:01, fukino wrote:
> I think we should check the existence of |this.pairingDevice| here, since it
can
> be undefined just after the "PAIR DEVICE" is clicked.

Yeah, there are some new edge cases here that I didn't fully consider. Fixed
this, did some more careful testing, and made a few other fixes too, thanks.

https://codereview.chromium.org/2655043005/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/bluetooth_page/bluetooth_device_dialog.js:428:
close: function() {
On 2017/01/30 08:44:01, fukino wrote:
> close() is not called when the dialog is closed by clicking 'X' button, so
> listeners will not be removed even when the pairing dialog is not visible.

Right, thanks. Fixed.

https://codereview.chromium.org/2655043005/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/bluetooth_page/bluetooth_subpage.js
(right):

https://codereview.chromium.org/2655043005/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/bluetooth_page/bluetooth_subpage.js:166:
return this.bluetoothEnabled && !this.dialogId_;
On 2017/01/30 08:44:01, fukino wrote:
> In system tray, spinner or infinite loader is shown only when the BT adapter
is
> discovering devices.
> It might be better to be consistent with it?

Yeah, now that I think about it, the spinner doesn't really make sense here at
all. This is consistent with the previous behavior (except we hide it with the
dialog now because that looks terrible, it wasn't visible with the dialog
before). We can address this when we reconsider how we do 'add devices'.

[stevenjb, 2017-01-30 19:20:30]

Description was changed from

==========
MD Settings: Bluetooth: Move device list to subpage

This CL:
* Moves the bulk of the Bluettoth UI to a subpage (see issue).
* Moves the (more compact) bletooth section to the Basic page,
below Internet, as requested by UX.
* Adds support for a spinner to settings-subpage (we will eventually use this
for Internet also).

Review notes:
* pairingEvent_ and bluetoothPrivateOnPairingListener_ were moved to
bluetooth-device-dialog where they are actually used.


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

to

==========
MD Settings: Bluetooth: Move device list to subpage

This CL:
* Moves the bulk of the Bluettoth UI to a subpage (see issue).
* Moves the (more compact) bletooth section to the Basic page,
below Internet, as requested by UX.
* Adds support for a spinner to settings-subpage (we will eventually use this
for Internet also).

Review notes:
* pairingEvent_ and bluetoothPrivateOnPairingListener_ were moved to
bluetooth-device-dialog where they are actually used.


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

[fukino, 2017-01-31 01:58:12]

lgtm. Thanks!

[stevenjb, 2017-01-31 16:43:19]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2017-01-31 16:43:39]

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-31 17:02:12]

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

[commit-bot: I haz the power, 2017-01-31 17:02:14]

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...)

[stevenjb, 2017-01-31 17:54:54]

The CQ bit was checked by stevenjb@chromium.org

[stevenjb, 2017-01-31 17:54:55]

The patchset sent to the CQ was uploaded after l-g-t-m from fukino@chromium.org
Link to the patchset: https://codereview.chromium.org/2655043005/#ps120001
(title: "Fix clang, ES6")

[commit-bot: I haz the power, 2017-01-31 17:55:25]

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-31 19:08:21]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1485885294778310,
"parent_rev": "a436fd0e674c0f13a3334f4c2bfbd04773b3372c", "commit_rev":
"ecc211d2da5e50c3b4483849a325795fd0072baa"}

[commit-bot: I haz the power, 2017-01-31 19:08:56]

Description was changed from

==========
MD Settings: Bluetooth: Move device list to subpage

This CL:
* Moves the bulk of the Bluettoth UI to a subpage (see issue).
* Moves the (more compact) bletooth section to the Basic page,
below Internet, as requested by UX.
* Adds support for a spinner to settings-subpage (we will eventually use this
for Internet also).

Review notes:
* pairingEvent_ and bluetoothPrivateOnPairingListener_ were moved to
bluetooth-device-dialog where they are actually used.


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

to

==========
MD Settings: Bluetooth: Move device list to subpage

This CL:
* Moves the bulk of the Bluettoth UI to a subpage (see issue).
* Moves the (more compact) bletooth section to the Basic page,
below Internet, as requested by UX.
* Adds support for a spinner to settings-subpage (we will eventually use this
for Internet also).

Review notes:
* pairingEvent_ and bluetoothPrivateOnPairingListener_ were moved to
bluetooth-device-dialog where they are actually used.


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

Review-Url: https://codereview.chromium.org/2655043005
Cr-Commit-Position: refs/heads/master@{#447287}
Committed:
https://chromium.googlesource.com/chromium/src/+/ecc211d2da5e50c3b4483849a325...
==========

[commit-bot: I haz the power, 2017-01-31 19:08:58]

Committed patchset #6 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/ecc211d2da5e50c3b4483849a325...