1007569 - [NFC] Connect to BT-earphone dialog is different from UX spec and after pressed "paired confirm", screen will hang.

Status: VERIFIED FIXED

(Firefox OS Graveyard :: NFC, defect)

Description

[Alison Shiue]

Gaia      347d0517f0a77122c876d5f62c0942006a7a0bfe
Gecko     https://hg.mozilla.org/mozilla-central/rev/8be0e21fd300
BuildID   20140507160203
Version   32.0a1

A phone with NFC, and a NFC-capable bluetooth earphone

STR:
1. Enable NFC in settings
2. Tap the phone with bluetooth earphone
3. After NFC detected, click paired confirm

Expected result:
Connect NFC earphone successfully without any problem

Actual result:
Screen hang on unless click home button

Comment 1

[Ken Chang[:kenkai]]

We need Arno's help.

Comment 2

[Alison Shiue]

Attachment: dialog when BT is off

Comment 3

[Alison Shiue]

Attachment: dialog when BT is on

Comment 4

[Alison Shiue]

Attachment: [NFC] BT Pairing_20140318.pdf

The confirm dialog(both BT on and off) is different from UX design. Please refer page 6 and 7.

Comment 5

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

There are two issues in this bug
1. The UX is different from spec, as Alison mentioned in Comment 2 ~ Comment 4.
2. After the confirmation dialog, the screen will hang, but we can still drag the notification bar and press Home key, as Alison mentioned in Comment 0.

Right now only this dialog will have the 'hang' problem, so we suspect that the 'Pairing Dialog' has some bugs in the event handling and UI.

Comment 6

[Kamil Leszczuk [:kamituel]]

Hi!
Regarding those two issues:
1. UX: I've fixed it - I believe it matches the spec now. Allison, could you review it? I'll post screenshots in a sec.
2. Screen hang: issue was that SystemDialog.hide() was overriden and thus, not called. This meant "system-dialog-hide" event was never dispatched and SystemDialogManager never removed "dialog" class from div#screen.

Here's my current work: https://github.com/kamituel/gaia/commit/b899f7ebe495eac5c4ca78b478a12c426e7ebe1e.
Once UX gets r+, I'll mark code for review as well.

Comment 7

[Kamil Leszczuk [:kamituel]]

Attachment: bt-off-name-known

Comment 8

[Kamil Leszczuk [:kamituel]]

Attachment: bt-on-name-unknown

Comment 9

[Kamil Leszczuk [:kamituel]]

Attachment: bt-on-name-known

Comment 10

[Kamil Leszczuk [:kamituel]]

Attachment: bt-on-name-unknown

Comment 11

[Kamil Leszczuk [:kamituel]]

Comment on attachment 8420541 [details]
bt-on-name-unknown

Marking you, @ashiue, r? for one of the attachments.

Comment 12

[Kamil Leszczuk [:kamituel]]

Improved test cases a bit: https://github.com/kamituel/gaia/commit/cff2df84d3ba977c001bce6849f8bc4a955af54e

Comment 13

[Alison Shiue]

Comment on attachment 8420541 [details]
bt-on-name-unknown

Please update the wording on buttons. According to UX doc, bt-on wording should be "No" and "Yes". Thank you!

Comment 14

[Alison Shiue]

Hi Kamil,
Could you update the wording on button? 
According to UX doc, bt-off wording should be "Cancel" and "Turn on", and bt-on wording should be "No" and "Yes". 
Except those buttons wording, the dialog looks well! Thank you.

Comment 15

[Kamil Leszczuk [:kamituel]]

Attachment: Fixed button labels.

Hi, thanks for the review :) I've fixed labels. In attachment you'll find two cases, for BT on (left) and off (right) (I've concatenated them to avoid multiple attachments).

Comment 16

[Kamil Leszczuk [:kamituel]]

Attachment: Fix for both UI and hang issues.

Comment 17

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8420879 [details] [review]
Fix for both UI and hang issues.

Alive's r+ is enough.

Comment 18

[Axel Hecht [:Pike]]

Comment on attachment 8420879 [details] [review]
Fix for both UI and hang issues.

The Yes and No are already re-used, and they're re-used in the same context, also visually.

These should be fine to use again.

Side-note, I wish we'd use data-l10n-id changes and mozL10n.translate(), that'd open up a host of hackability for testing and fixing of localizations.

Comment 19

[Kamil Leszczuk [:kamituel]]

I've fixed L10n remarks:
- "yes"/"no" labels are no re-used.
- I used mozL10n.translate() and mozL10n.localize() instead of .get()

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/a13d42ab5240008e042d0c61bf9c9d05174e70e4

Comment 21

[Alison Shiue]

Verified on
Gaia      101c500903a2477f9de1ea5ce523b9e0be4d45d0
Gecko     https://hg.mozilla.org/mozilla-central/rev/41a54c8add09
BuildID   20140518040203
Version   32.0a1

Comment 22

[Shally Li（leave from Mozilla）]

Per comment 21,I clear "verifyme".