1003330 - Mobile identity UI

Status: RESOLVED FIXED

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

Description

[Fernando Jiménez Moreno [:ferjm]]

Bug 988469 is going to expose an API to allow privileged apps to obtain a verified phone number. We need to design and build some chrome UI bits for the following cases:

- Ask permission to the user to let the app get her phone number.
- Provide the user the ability to give a phone number that doesn't belong to a SIM inserted in the device (this is specially required for the Desktop case and might not be needed for Firefox OS).
- Provide the user the ability to select the desired SIM or phone number in a multi-SIM scenario.
- Ask the user for the verification code in case that the chosen phone number does not belong to any of the inserted SIMs or the code is not received in the device after a certain timeout.
- Allow the user to cancel the verification process.

Comment 1

[Fernando Jiménez Moreno [:ferjm]]

Attachment: High level overview flow

Comment 2

[Fernando Jiménez Moreno [:ferjm]]

Attachment: WIP

Comment 3

[Fernando Jiménez Moreno [:ferjm]]

Attachment: WIP

Comment 4

[Borja Salguero [:borjasalguero]]

Attachment: Pull Request

Patch ready to review!

Comment 5

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8434130 [details] [review]
Pull Request

Thanks Borja! Excellent work! :)

I did a first review pass and left a few comments in Github.

Also, apart from what I mentioned in the PR, while testing this patch I found a few issues:

1- The progress bar for the verification timeout is not working.

2- Error alerts are not working (at least on the device).

3- The input field error dance is not working.

4- Cannot re-launch the dialog after clicking in the home button.

5- I can manually scroll from the SIM options to the manual phone number input.

6- The phone number input is too long and doesn't fit in the screen.

Comment 6

[Borja Salguero [:borjasalguero]]

Comment on attachment 8434130 [details] [review]
Pull Request

All issues detected were related with your suggestions, so now everything should work as expected! r?

Comment 7

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8434130 [details] [review]
Pull Request

Great job as usual! :)

I left a few more comments in the PR, minor issues only.

Since this is adding a new system dialog, we need to get sr? from a module owner. Thanks in advance Alive!

Comment 8

[Fernando Jiménez Moreno [:ferjm]]

Oh, and remember to run the tools/png_recompresh.sh script if you haven't done it already, please.

Comment 9

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8434130 [details] [review]
Pull Request

I only check the dialog and its manager.

Basically what's being implemented in current system app is:
Things about the iframe belong to one dialog / one window should be done in the dialog / window module.
The XXXX manager's load will be less if the instances managed by him doing its own work well.

Also, if next time you want to communicate from an embedded iframe to system app, window.parent.XXXX is somewhat bad coupling. Try dispatch event like 'fxa-xxxxxx' to window.parent and ask some one to caught it.

Comment 10

[Borja Salguero [:borjasalguero]]

https://github.com/borjasalguero/gaia/commit/9e164703a195517d1b9de5c1c346285673ab5dd6
https://github.com/mozilla-b2g/gaia/commit/270a833e262dfbaf855cc6954d23a55f92c60db5

R+ and I've addressed all your suggestions. I'll move to events when adding the new errors in a follow-up, once the whole list will be available. Thanks!

Comment 11

[Francesco Lodolo [:flod]]

Note for English: "Ok" should be spelled "OK".

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Reverted for TBPL bustage.
Master: https://github.com/mozilla-b2g/gaia/commit/2e5636e852a9354a5f8072b179cf16f72647cfd6

https://tbpl.mozilla.org/php/getParsedLog.php?id=41274847&tree=B2g-Inbound

Comment 13

[Julien Wajsberg [:julienw]]

Looks like TBPL doesn't like unicode file names ?

Also, is this file expected: https://github.com/borjasalguero/gaia/commit/9e164703a195517d1b9de5c1c346285673ab5dd6#diff-20 ?

Comment 14

[Borja Salguero [:borjasalguero]]

(In reply to Julien Wajsberg [:julienw] from comment #13)
> Looks like TBPL doesn't like unicode file names ?
> 
> Also, is this file expected:
> https://github.com/borjasalguero/gaia/commit/
> 9e164703a195517d1b9de5c1c346285673ab5dd6#diff-20 ?

Thanks Julien for pointing out where the problem was. It's an old file (actually it was a test for the gif animation), and I dont know why it was in the PR. Removing it and waiting for re-merging.

Comment 15

[Borja Salguero [:borjasalguero]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #12)
> Reverted for TBPL bustage.
> Master:
> https://github.com/mozilla-b2g/gaia/commit/
> 2e5636e852a9354a5f8072b179cf16f72647cfd6
> 
> https://tbpl.mozilla.org/php/getParsedLog.php?id=41274847&tree=B2g-Inbound

Hi Ryan, my fault, it seems an old file was included by error in the PR. I've removed it and it's in a new PR:
https://github.com/mozilla-b2g/gaia/pull/20184

Waiting Travis to re-merge this. Sorry for this issue!

Thanks

Comment 16

[Borja Salguero [:borjasalguero]]

Attachment: Patch fixing integration error

There was an old file included in the PR, so Integration tests were failing. I've remove the file and waiting Travis to merge.

Comment 17

[Borja Salguero [:borjasalguero]]

(In reply to Francesco Lodolo [:flod] from comment #11)
> Note for English: "Ok" should be spelled "OK".

As I had to re-push it, I've added this.
Thanks!
https://github.com/mozilla-b2g/gaia/pull/20184/files#diff-08417443aed477a5faa9e4ffd0f5b628R22

Comment 18

[Borja Salguero [:borjasalguero]]

https://github.com/borjasalguero/gaia/commit/02825a20c72472ac9d7fe129889a14490e0532dc
https://github.com/mozilla-b2g/gaia/commit/c006aaa4eadc7ea56bc244ad63a1862a37133fbe
Merged (I hope this time with no issue!) :)