1007932 - Allow to enclose a contact (serialized as vCard) as a message attachment

Status: VERIFIED FIXED

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

Description

[Adam Fónagy]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:29.0) Gecko/20100101 Firefox/29.0 (Beta/Release)
Build ID: 20140428193609

Steps to reproduce:

I've saved some contacts than I wanted to send one phone number to an other contact. I tried to find option for it also tried to attach contact card to an email.


Actual results:

I didn't find any way to send a phone number to an other contact


Expected results:

There should be an option to send contact card to a phone number or an other contact. Also it would useful to be able to attach contact to an email.

Comment 1

[Beatriz Rodríguez [:brg]]

Adding blocking-b2g to backlog, we think this is an interesting nice feature to add to FxOS. We would like Mozilla to consider it in their roadmap.

Comment 2

[Pau Masiá [:Pau]]

Attachment: [Contacts] UX Spec. Share Vcard

This spec shows how a user might be able to share a Vcard via MMS while being in Contact Details within Contacts app.

Carrie, I would need some feedback on this, please.

Thanks :).

Comment 3

[Pau Masiá [:Pau]]

Attachment: [2.0 Messages] SharevCard v1.pdf

This spec shows how a user might be able to attach a Vcard via MMS while being in a conversation within SMS app.

Jenny, I would need some feedback on this, please.

Thanks :).

Comment 4

[Maria Angeles Oteo (:oteo)]

Setting ni to Jenny according comment 3

Comment 5

[Jenny Lee]

Hi Pau,
See my comments below:
- Screen 3-4: Please refer to the share contact via BT flow (Contacts> settings> export contacts> Bluetooth) for the correct picker flow. 
- Last screen (contact detail view): we probably don't want to allow user to access the functions here, ex. call, send message, consider gray out the buttons or re-layout so that it will be "pure preview".
- What about sharing multiple contacts at the same time?

Thanks!

Comment 6

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

Hi, 

My suggestion is that maybe we can move this share contact button to the "SOCIAL NETWORKS" section. Thanks!

Comment 7

[Jose Manuel Cantera]

(In reply to Carrie Wang [:carrie] from comment #6)
> Hi, 
> 
> My suggestion is that maybe we can move this share contact button to the
> "SOCIAL NETWORKS" section. Thanks!

This bug is for attaching the contact in a message, but being the caller (the app which initiates the flow) the messaging app and not the contacts app. The other flow Contacts --> Msging would be a different feature.

Comment 8

[Jose Manuel Cantera]

(In reply to Jenny Lee from comment #5)
> Hi Pau,
> See my comments below:
> - Screen 3-4: Please refer to the share contact via BT flow (Contacts>
> settings> export contacts> Bluetooth) for the correct picker flow. 
> - Last screen (contact detail view): we probably don't want to allow user to
> access the functions here, ex. call, send message, consider gray out the
> buttons or re-layout so that it will be "pure preview".
> - What about sharing multiple contacts at the same time?
> 
> Thanks!

Hi all,

I think we are overcomplicating the feature. So what we are planning right now is to extend the pick contact activity to allow to send a vCard as a result. 

So the flow would be Messaging App --> Attach --> Contact --> Contact list --> User chooses a Contact --> Contact is attached. 

In the future we will be considering attaching multiple contacts but that will be a new feature for v2.2. To clarify, What we are planning to do here is a simple patch (implementing what I've described above) aimed at uplifting it to v2.1 . 

Hope this clarifies

Comment 9

[Pau Masiá [:Pau]]

(In reply to Jenny Lee from comment #5)
> Hi Pau,
> See my comments below:
> - Screen 3-4: Please refer to the share contact via BT flow (Contacts>
> settings> export contacts> Bluetooth) for the correct picker flow. 

Jenny, here I agree with Jose Manuel since I believe the Export contacts via BT flow would be much complicated for the user to get to same objective, which is simply attaching a contact in a SMS conversation.

Regarding the other changes you suggested, I'm working on them and I'll attach the new version ASAP.
Thanks!

Comment 10

[Pau Masiá [:Pau]]

Attachment: [2.0 Contacts] SharevCard v1

Here's spec updated. Carrie, can you confirm it is ok?

Thanks!

Comment 11

[Maria Angeles Oteo (:oteo)]

Setting ni to Carrie to review the spec attached by Pau in comment 10

Comment 12

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

Hi, 

I think overall it looks fine to me, but from comment 8, aren't we doing the part of attaching a vcard from message? but the updated version is sharing from contacts (I'm quite confused now). 
Also,ni? Jenny to double confirm the MMS part. Thanks!

Comment 13

[Pau Masiá [:Pau]]

Attachment: [2.0 Messages] SharevCard v1.pdf

Comment 14

[Jenny Lee]

Hi Pau,

(Comment based on attachment 8527567 [details]) Looks fine to me except for:
Screen 2 - Maybe consider moving Contact to the last on the menu since media content is more likely to be attached 
Screen 5 - "Converting to multimedia message" banner is missing, and the title should probably be "MMS"

Let's see if Carrie is ok with the contact picker flow too. Thanks!

Comment 15

[Pau Masiá [:Pau]]

Attachment: [2.0 Messages] SharevCard v1.pdf

Here I attach the new spec according to Jenny's comments. Just waiting for Carrie's approval. Thanks to both! :)

Comment 16

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

Postponing for next sprint. We have too many things in our place, will take this the next sprint.

Comment 17

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

Hi, 

Only one suggestion. We shall replace the Screen 4 with the "Contact Details View" so that users will not call/ message the contact during Vcard sharing process. Thanks!

Comment 18

[Pau Masiá [:Pau]]

Attachment: [2.0 Messages] SharevCard v1.pdf

Ok! Changes applied. Thanks!

Comment 19

[Jose Manuel Cantera]

(In reply to Pau Masiá [:Pau] from comment #18)
> Created attachment 8528230 [details]
> [2.0 Messages] SharevCard v1.pdf
> 
> Ok! Changes applied. Thanks!

The pick activity would only allow you to select a contact, i.e. you are not going to be able to see the contact detail from the activity. I think that's a reasonable assumption and not very harmful, are you ok with that?

thanks!

Comment 20

[Jose Manuel Cantera]

Attachment: Contacts Part. Pick Activity returning an vcard blob

These are the small changes proposed at Contacts App Level. At least the SMS and email apps would need to be modified as well in order to support this new kind of attachment

Comment 21

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

Comment on attachment 8529712 [details]
Contacts Part. Pick Activity returning an vcard blob

I like this approach, I though we were going to allow multiple selection, but as a first step is ok.

We reuse, the activity, since we are doing a pick with a specific mime type.

Just left a comment on github, please sync with Julien as this will require integration with SMS (and probably email)

Comment 22

[Jose Manuel Cantera]

(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #21)
> Comment on attachment 8529712 [details]
> Contacts Part. Pick Activity returning an vcard blob
> 
> I like this approach, I though we were going to allow multiple selection,
> but as a first step is ok.

Yes, UX and product agreed to initially only support one contact attachment at a time. 

> 
> We reuse, the activity, since we are doing a pick with a specific mime type.
> 
> Just left a comment on github, please sync with Julien as this will require
> integration with SMS (and probably email)

Julien, would you be happy if I implement the SMS part as well? But unfortunately that will not be possible until the week of Dec 08th, as I'm on vacation next week.

thanks

Comment 23

[Julien Wajsberg [:julienw]]

Hi,

I'm always happy if people do work I don't need to do ;)

I wonder if your change in the pull request is enough?

Comment 24

[Jose Manuel Cantera]

(In reply to Julien Wajsberg [:julienw] from comment #23)
> Hi,
> 
> I'm always happy if people do work I don't need to do ;)
> 
> I wonder if your change in the pull request is enough?

oh no, it was enough to test the activity but we would need to add an icon in the attachments section of the sms app to indicate that we are enclosing a contact ... I think we would need some visual design input as well

Comment 25

[Jose Manuel Cantera]

Attachment: Pointer to GH PR #26912

Julien,

Please could you do a first review and feedback over the code?. 

Regarding tests, I'm planning to do a couple of integrations tests  to test the attachment part between Contacts and Messaging apps. 

Please let me know if you think I should be doing unit tests as well, any hint would be appreciated. 

thanks, best

Comment 26

[Noemí Freire (:noemi)]

Hi,

As said in comment 7 this bug will be focused on sharing a contact from Messaging app so changing the component to SMS to be more accurate. 
On the other hand, Bug 1113605 has been opened to deal with sharing a contact from "contacts detail" screen.
Thanks!

Comment 27

[Julien Wajsberg [:julienw]]

Comment on attachment 8539220 [details] [review]
Pointer to GH PR #26912

Yes, please do unit tests :) The idea is that each change you make is covered by a unit test change or a new unit test.

Please request a review from :steveck once you're ready, he knows SMIL way better than me, and he may have other phones to test (I have only Firefox OS phones).

Thanks, I really want to see this feature !

Comment 28

[Jose Manuel Cantera]

Comment on attachment 8539220 [details] [review]
Pointer to GH PR #26912

Comments from Julien addressed. I have provided both new unit and integration tests.

Comment 29

[Pau Masiá [:Pau]]

Attachment: SMMS. Vcard icon 1

Here I will attach a set of three proposals for the Vcard icon when attaching a contact via MMS.

Please Fang, could you take a look to them and send me some feedback? :)
Thanks!

Comment 30

[Pau Masiá [:Pau]]

Attachment: SMMS. Vcard icon 2

Version 2

Comment 31

[Pau Masiá [:Pau]]

Attachment: SMMS. Vcard icon 3

Version 3.

Comment 32

[Steve Chung [:steveck]]

Comment on attachment 8539220 [details] [review]
Pointer to GH PR #26912

Some suggestion on github, and you might need unit test for certain changes

Comment 33

[Steve Chung [:steveck]]

Hi Pau, just a reminder that Fang took PTO until next week, please ping Carol Huang or Peko Chen instead if you want more feedback this week.

Comment 34

[Beatriz Rodríguez [:brg]]

Thanks Steve! setting ni on Carol and Peko, please read from comment 29 and on.

Comment 35

[Carol Huang [:Carol]]

Hi Pau,
For the contact card icons, personally I like version 2 the most. because to me version 3 is more like a document of information. 
Only one comment on v2: I'm just wondering if you can simplify the shape of the card, or simplify the element of the hinge.
Thanks!!

Comment 36

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

Comment on attachment 8539220 [details] [review]
Pointer to GH PR #26912

Left some comments in github for the contacts part. No mayor issues, please address them, also we will need the r+ from Steve.

Comment 37

[Jose Manuel Cantera]

Comment on attachment 8539220 [details] [review]
Pointer to GH PR #26912

Ready for a second review round from Steve

thanks

Comment 38

[Fang Shih [:fang]]

Thanks Carol! I also like the version 2 the most. It's more straightforward and more close to the actual object. I would simplify the shape of the hinge. Since the icon may lose some detail from our device. It would be good to keep it simple and not too much element. Thanks!

Comment 39

[Steve Chung [:steveck]]

Hi Jose, I add some comments on githab, it looks good overall except some small nits. The only problem is a failed unit test. So still keeps review flag for now.

Comment 40

[Jose Manuel Cantera]

Hi Steve,

Thanks for spotting the unit test error. I have addressed your comments and now TBPL should be green.

Comment 41

[Steve Chung [:steveck]]

Comment on attachment 8539220 [details] [review]
Pointer to GH PR #26912

Looks great, thanks!

Comment 42

[Jose Manuel Cantera]

landed in master:

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

There is a pending minor improvement here, the new icon for enclosed contacts. We can leave this bug open or create a follow-up. As in this bug we already have the history of the design of the icon I'm more inclined to leave this one open.

Comment 43

[Jose Manuel Cantera]

We have open follow-up bug 1119190. So closing this. setting flag for QA verification as well.

Comment 44

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

It was landed on master before FLR so we'll have it in v2.2
Flagging to 2.2+ to get QA verification.

Comment 45

[Beatriz Rodríguez [:brg]]

Tested with today build in Flame( Gecko-7a1f501.Gaia-9946a49) there is an option to attach contact details(.vcf) when writing a MMS. Thank you everyone for your effort to finish it!

Comment 46

[Eric Chang [:ericcc] [:echang]]

Hi Francisco, 
Page 4 of [2.0 Messages] SharevCard v1.pdf https://bugzilla.mozilla.org/attachment.cgi?id=8525272, when we try to view an attached VCF, it looks like an details page with any editing text box, I didn't join the conversation until now Is that a spec change? Or we need to fix that? 
http://youtu.be/BwDImXh-sCw

Comment 47

[Jose Manuel Cantera]

(In reply to Eric Chang [:ericcc] [:echang] from comment #46)
> Hi Francisco, 
> Page 4 of [2.0 Messages] SharevCard v1.pdf
> https://bugzilla.mozilla.org/attachment.cgi?id=8525272, when we try to view
> an attached VCF, it looks like an details page with any editing text box, I
> didn't join the conversation until now Is that a spec change? Or we need to
> fix that? 
> http://youtu.be/BwDImXh-sCw

that's bug https://bugzilla.mozilla.org/show_bug.cgi?id=1120878

Comment 48

[Pau Masiá [:Pau]]

I've attached the new versions of the icon in Bug 1119190. Thanks!

Comment 49

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

Jose already replied to my ni in comment 47

Comment 50

[Shing Lyu [:shinglyu]]

Attachment: Testcases on MozTrap

Add a test case in MozTrap: https://moztrap.mozilla.org/manage/cases/?filter-id=15416

Comment 51

[Shally Li（leave from Mozilla）]

This bug has been successfully verified on Flame v2.2&3.0.
See attachment: verified_v2.2.png
Reproduce rate: 0/5.

STR:
1.Open a contact->tap Share contact icon->Select Message (or Email) to share.
**The contact can be shared as a Vcard file in Message edit page (or Email compose page).
2.Launch Messages->create a new message->add a attachment->select Communications ->select a contact.
**The contact can be added to Message app and shared as a Vcard file.

Flame v2.2 build:
Gaia-Rev        80d5b797fd0497a7e3337b7798a21b2e1219681a
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/01bf1516a65b
Build-ID        20150127002504
Version         37.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150127.035842
FW-Date         Tue Jan 27 03:58:52 EST 2015
Bootloader      L1TC000118D0

Flame 3.0 build:
Gaia-Rev        b02ec9713e6de8d96c6954d2c0dfd0442b0656ac
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/38e4719e71af
Build-ID        20150127010228
Version         38.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150127.043726
FW-Date         Tue Jan 27 04:37:39 EST 2015
Bootloader      L1TC000118D0

Comment 52

[Shally Li（leave from Mozilla）]

Attachment: verified_v2.2.png