1184819 - Migrate bluetooth switches to use gaia-switch

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Settings, defect)

Description

[Kevin Grandon :kgrandon]

In preparation for moving to web components.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] KevinGrandon:bug_1184819_migrate_bluetooth_switches > mozilla-b2g:master

Comment 2

[Kevin Grandon :kgrandon]

Comment on attachment 8635092 [details] [review]
[gaia] KevinGrandon:bug_1184819_migrate_bluetooth_switches > mozilla-b2g:master

Hi Fred and Shawn - 

I was hoping that you could help me for some reviews for settings and bluetooth. This code looked similar, so I tried to make the same change in both places. 

I was actually not sure how to exactly trigger the checkboxes in the bluetooth app (or if they are surfaced to the user at all). Please take a look and let me know what you think. Thanks!

Comment 3

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Sorry, I'm not the right person to review gaia code, gecko bluetooth is my field. Fred, would you please help the change for bluetooth app?

Comment 4

[[e-mail]]

Thanks for the patch. Bluetooth/module is a mirror with Settings/js/module/bluetooth and the paring UI will be used when BT transfer is triggered.

I'm PTO this afternoon & will do the review next Monday.

Comment 5

[[e-mail]]

Comment on attachment 8635092 [details] [review]
[gaia] KevinGrandon:bug_1184819_migrate_bluetooth_switches > mozilla-b2g:master

Test on device and works fine.

The loadtime also looks fine after the patch.

Origin

Metric                            Mean      Median    Min       Max       StdDev   p95
--------------------------------  --------  --------  --------  --------  -------  --------
coldlaunch.contentInteractive     994.000   983.000   889.000   1119.000  50.194   1096.000
coldlaunch.navigationLoaded       1134.333  1125.000  1053.000  1241.000  48.918   1238.000
coldlaunch.navigationInteractive  1134.433  1125.500  1053.000  1241.000  48.908   1238.000
coldlaunch.startupPathEnd         2185.567  2149.500  2030.000  2441.000  118.607  2425.000
coldlaunch.visuallyLoaded         2750.667  2702.500  2574.000  2994.000  122.982  2964.000
coldlaunch.fullyLoaded            5288.967  5258.500  5154.000  5491.000  98.473   5467.000
coldlaunch.pss                    22.550    22.600    22.100    23.000    0.220    22.900
coldlaunch.rss                    38.113    38.100    37.700    38.500    0.208    38.500
coldlaunch.uss                    18.940    19.000    18.600    19.300    0.182    19.300

After Patch

Metric                            Mean      Median    Min       Max       StdDev   p95
--------------------------------  --------  --------  --------  --------  -------  --------
coldlaunch.contentInteractive     990.233   981.000   914.000   1131.000  53.202   1086.000
coldlaunch.navigationLoaded       1129.800  1110.000  1048.000  1334.000  66.759   1237.000
coldlaunch.navigationInteractive  1129.867  1110.000  1048.000  1334.000  66.769   1237.000
coldlaunch.startupPathEnd         2173.600  2137.500  2027.000  2435.000  122.276  2423.000
coldlaunch.visuallyLoaded         2737.500  2671.000  2546.000  3082.000  154.903  3016.000
coldlaunch.fullyLoaded            5275.033  5229.500  5172.000  5561.000  109.635  5488.000
coldlaunch.pss                    22.483    22.500    22.200    22.700    0.132    22.700
coldlaunch.rss                    38.027    38.000    37.800    38.200    0.129    38.200
coldlaunch.uss                    18.853    18.900    18.700    19.000    0.102    19.000

Comment 6

[[e-mail]]

And please also update settings/elements/bluetooth_v2.html

Comment 7

[[e-mail]]

Comment on attachment 8635092 [details] [review]
[gaia] KevinGrandon:bug_1184819_migrate_bluetooth_switches > mozilla-b2g:master

Sorry I found there's more places need to be addressed, so I'll take off r+.

`panels/bluetooth/panel.js` still use `bluetooth-status input` selector, which seems need to be update to '#bluetooth-status gaia-switch'

Since Bluetooth API just get tremendously update, we have to maintain compatibility between APIv1/v2 at this stage. Will deprecate the old v1 API compatibility layer in gaia when BT APIv2 is stabled.

Please update the PR and set review again.

Comment 8

[Kevin Grandon :kgrandon]

Comment on attachment 8635092 [details] [review]
[gaia] KevinGrandon:bug_1184819_migrate_bluetooth_switches > mozilla-b2g:master

Hi Fred,

Thank you for helping me with this patch. I've made some updates to it to fix disabled states. Having both v1 and v2 markup for bluetooth in the codebase was a bit confusing, but it seems to be working well on the flame. Please take a look if you get a chance, thanks!

Comment 9

[[e-mail]]

Comment on attachment 8635092 [details] [review]
[gaia] KevinGrandon:bug_1184819_migrate_bluetooth_switches > mozilla-b2g:master

Looks good! Please remove debug log before landing.

Comment 10

[Kevin Grandon :kgrandon]

Thank you for the great review Fred!

Comment 11

[Kevin Grandon :kgrandon]

In master: https://github.com/mozilla-b2g/gaia/commit/5c87f4001b599e8e91130641a1ba9b4469d7c425

Comment 12

[Kevin Grandon :kgrandon]

I've pushed a follow-up which removes references to pack-* elements in lists.css. This is helpful for grepping/narrowing down the list of remaining components to port.

https://github.com/mozilla-b2g/gaia/commit/bf8565e0c3ad216ccb3f109c17f8a2eb2c42f6b8