988392 - Allow Loop to be added to the contact details

Status: RESOLVED FIXED

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

Description

[Maria Angeles Oteo (:oteo)]

Currently, Contacts Application is using directly MozTelephony API instead of any activity. This means that if any other way to dial-in a phone number is installed on the device, the user will not be offered to choose that alternative. This affects, for instance, the Loop client.
In order to make Loop more discoverable, this is obviously a quick-win.
The suggested approach here is making the Contacts App use the Dial Activity. The problem with that approach is that current Dial Activity, requires the user to click on the "Dial Button" for security reasons.  In order to avoid the user clicking on the "Dial Button" once on the dialpad for this particular case, we suggest to use a HMAC validation mechanism in the Activity so that the Dialer can check if the request is coming from contacts, and in that case, dial the number automatically so the user does not need to click on the green button.

Comment 1

[Fernando Jiménez Moreno [:ferjm]]

Attachment: WIP Gaia branch

The approach mentioned in the bug description is not valid anymore. Instead of that we opted for building a comms provider discovery mechanism based on the DataStore API, where each comms provider that wants to be listed in the Contacts app details view needs to expose a 'comms-provider' DS containing some meta data describing the provided service.

I've started to work on this approach at https://github.com/ferjm/gaia/tree/loop-contacts In that tree you can see how two different buttons corresponding to two different providers ("Comms Provider Test" and "Comms Provider Test 2") are added to the contacts details views. Note, that you need to open the comms provider apps before being able to see the buttons in the contacts app.

Francisco, this patch still needs quite some work, but it would be great if you could give us some feedback about the overall approach. Thanks!

Comment 2

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8413859 [details] [diff] [review]
WIP Gaia branch

Looking pretty promising!

Nice job, I think is pretty straight forward to follow and understand.

Left some comments on the github branch, but basically the resume is:

- Try to put into the helper (lazy loaded) as much functions as possible, so the less javascript we have to parse in details.js the better.
- Let's use Promises, now that we are working with datastores, based on promises, in the helper we can take advance of that to perform for example several queries to the different datastores in a easy way (Promise.all)

A part from that ... it's looking pretty nice :)

Thanks for your work!

Comment 3

[Maria Angeles Oteo (:oteo)]

Removing the dependency with bug 995147 as it was agreed with Vivien that, for this 1.0 Loop mobile version in 2.0 FxOS, we will use MozApps to check if Loop application is installed instead of DataStore.

Comment 4

[Borja Salguero [:borjasalguero]]

Attachment: Pull request

Pull request based on the proposal in bug https://bugzilla.mozilla.org/show_bug.cgi?id=1017420, so for testing in the device we need to wait until this patch will be reviewed & landed.

Comment 5

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8430833 [details] [review]
Pull request

First round done, looking pretty promising!

Great work Borja, left some comments on github.

Couldn't try on the phone, waiting for the blocking bug. Also have one concern, is about the use of a webactivity to send the contact, we could leak contacts information to non privileged apps. Will comment on the gecko patch.

Thanks!

Comment 6

[Francisco Jordano [:arcturus] [:francisco]]

(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #5)

> 
> Couldn't try on the phone, waiting for the blocking bug. Also have one
> concern, is about the use of a webactivity to send the contact, we could
> leak contacts information to non privileged apps. Will comment on the gecko
> patch.

As I comment in github, I'm a bit slow, but that's what the current contacts chooser does, anyway, we can have the case of other apps implementing that activity too.

Comment 7

[Rafael Rebolleda]

Hey folks, 

I think it'd be nice to enhance the UX a little bit with what I believe would be a small eng investment.

Right now Loop buttons are displayed if Loop app is installed and the contact has either a number or an email. If the Loop app is not installed or the contact doesn't have either a number or an email, the buttons are not displayed. I find the latter case to be a bit misleading.

We can enhance this by showing the buttons disabled when the app is installed but the contact doesn't have neither numbers nor emails.

Wdyt?

Comment 8

[Rafael Rebolleda]

Attachment: Loop Buttons on Contact Details

Comment 9

[Rafael Rebolleda]

Attachment: Disabled Loop buttons for Contact Details

Comment 10

[Francisco Jordano [:arcturus] [:francisco]]

One more thing, Fabrice and me were talking in IRC about the web activity names, better not to associate the name with 'loop'.

Comment 11

[Borja Salguero [:borjasalguero]]

Comment on attachment 8430833 [details] [review]
Pull request

All comments addressed and last request from UX added. Removed naming due to the branding is not ready yet.

Comment 12

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8431532 [details]
Disabled Loop buttons for Contact Details

Hei Carrie, could you give some feedback about a contact without email and phone, so loop shouldn't be available.

Should we disable the buttons like in this screenshot or should we hide the options?

IMHO, we shouldn't show even the options.

Comment 13

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8430833 [details] [review]
Pull request

Pretty cool Borja,

code wise looking to me perfect, I just created an app with a webrtc-call web activity to check it, you can see it here:

http://people.mozilla.org/~fjordano/loop_test/

You can install from the browser.

I found a couple of things:
   - Visibility is not working at least with the b2g from m-c to try with the new activities goodies.
   - Requested ui-review for the disable option that is showing the buttons disabled for contacts without email or phone.
   - When editing a contact, it doesn't get refresh, for example adding a phone to a contact that didn't have either phone or email.

We are pretty, pretty close.

Thanks a lot for the work!

Comment 14

[Rafael Rebolleda]

(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #12)
> Should we disable the buttons like in this screenshot or should we hide the
> options?
> 
> IMHO, we shouldn't show even the options.

Just to be clear, the goal is to distinguish between:
* Loop not installed => No buttons
* Loop installed, a particular contact with no email nor number => disabled buttons

Otherwise, I think it's a bit confusing seeing different contact details with and without this sections. Having it disabled feels clearer.

Comment 15

[Borja Salguero [:borjasalguero]]

Comment on attachment 8430833 [details] [review]
Pull request

Tested and now working as expected, just a boolean not set properly (and now fixed!). r?

Comment 16

[Francisco Jordano [:arcturus] [:francisco]]

(In reply to Rafael Rebolleda [:rafaelrebolleda] from comment #14)

> 
> Otherwise, I think it's a bit confusing seeing different contact details
> with and without this sections. Having it disabled feels clearer.

Contacts already works like that, we hide any buttons for the information not present.

Anyway, it's something I won't block on it since it's a simple detail and we can fix it through the stabilisation phase if needed.

Thanks Rafa!
F.

Comment 17

[Rafael Rebolleda]

I agree this is fine detail that shouldn't block development.

Thanks folks!

Comment 18

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8430833 [details] [review]
Pull request

Hi Borja,

both Vivien and me got the clarification about going on with the activities query solution.

Just went again through the patch and tried it, and it's working like charm.

Again, won't block on the visibility of the buttons right now, since it can be done later when we decided.

Great job here, congrats.

Comment 19

[Carrie Wang [New account: [e-mail]]]

Comment on attachment 8431532 [details]
Disabled Loop buttons for Contact Details

I'd say, if there is no email and no MSISDN, we shall remove the buttons. We only display these two buttons when either email or the phone numbers exists (or both of them exist, of course).

In addition, I still think we shall move these two buttons above the email section so that the similar actions will be close to each other (normal calls/ audio/ video calls).
Thanks!

Comment 20

[Romain Testard [:RT]]

Agreed with Carrie regarding not displaying any button if the contact has no MSISDN and no email (that should be pretty rare anyway...).

Comment 21

[Borja Salguero [:borjasalguero]]

Hi Carrie & Romain!
As I've seen there are some discrepancies about how & where show these buttons within the details of the Contact. However, the goal of this bug is adding a new feature, which is detecting if the 'Loop' App is installed or not, and then show the actions related through Contacts.

I know that there are some things to improve (branding, visibility of the buttons, locales, position within the information of a Contact), and I would like to open a discussion in [1], taking into account your suggestions and the requirements from Product.

It's important to share the same concept, because having a good integration of Loop in the core Communication Apps is the key for the success of the product, and Carrie and the rest of the UX team are playing a really important role here!

So Carrie & Romain, I'm gonna add a ni? to [1] in order to retrieve your feedback and proposals, and we will work together in order to implement them within the Contact Details in the best way.

谢谢! ;)

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1017756