849729 - [Contacts] [vCard] Allow the user to import multiple contacts from vCard files

Status: RESOLVED FIXED

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

Description

[Kai-Chih Hu [:khu]]

Because of conformance.

Comment 1

[Kai-Chih Hu [:khu]]

Will mark it as leo+ for now. Can definitely leo? or - if someone has different view on this.

Comment 2

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Let me do some studies first. Start from google://vcard spec 2.1

Comment 3

[Dominic Kuo [:dkuo]]

Kevin, do we have any ux spec or user stories of supporting vCard? thanks.

Comment 4

[Joe Cheng [:jcheng] (please needinfo)]

change to leo? as product is confirming this is out of scope

Comment 5

[Kai-Chih Hu [:khu]]

(In reply to Dominic Kuo [:dkuo] from comment #3)
> Kevin, do we have any ux spec or user stories of supporting vCard? thanks.

Yes, it would be great, but so far, it looks like this feature may be not required.

Comment 6

[Lucas Adamski [:ladamski]]

Sorry, off by 1.

Comment 7

[Robert Kaiser]

Bug 823492 and bug 838121 are connected to this as well.

Comment 11

[Fernando Campo (:fcampo)]

A little late on the game (as the other was already resolved), but doesn't this bug collide with bug 920533 ?

Comment 12

[Julien Wajsberg [:julienw]]

We can support both mime types when receiving a MMS, right ? :)

Comment 13

[Fernando Campo (:fcampo)]

That's why I see a 'little' collision. From my pov, either we support it all across the system, or we don't ;)
Supporting it on certain apps and not in other seems clumsy. Of course we 'can' do it, but 'should' we do that?

Comment 14

[Julien Wajsberg [:julienw]]

The rule should be: permissive with what we receive, strict with what we send.

For the Messages app, we should imo target the maximum compatibility, and therefore we may need to send our vcard attachments as text/x-vcard ? This is all the Messages app business and should not impact any other app...

Comment 15

[Joe Cheng [:jcheng] (please needinfo)]

added to 1.5 targeted

Comment 16

[Steve Chung [:steveck]]

Hi Omega, we will need your help to define how to deal with vCcard attachment. Basically it should open contact/new contact page when user click on the vCard, but a wireframe or protptype would be peceft for feature implemetation, thanks.

Comment 17

[Omega Feng [:Omega] [:馮於懋]]

Attachment: [1.5 Messages] vCard v1.0.pdf

Here is the UX spec. Let me know your feedback if any.

Comment 18

[Julien Wajsberg [:julienw]]

Needs Francisco's feedback as well.

Comment 19

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

Hi folks,

Thanks Julien ;P

A couple of things, in contacts we are able to read any vcard format from 2.0 to 4.0, but recently we downgraded our writter from 4.0 to 2.0, the reason was compatibility with android. Modern android devices were not able to read the vcards that we were generating with vcard format 4.0.

Now we have a parser to create vcard in format 2.0 and apparently we get ride of the compatibility problems.

Regarding the wireframes by Omega, in the contacts app we already have an activity to import vcards, since we had this use case when importing a vcard from bluetooth, but the way it works is different.

You could see the experience in action if you send a contact from an android phone to a firefox os via bluetooth or just send a vcard with several contacts via bluetooth file transfer.

In the case of a single contact, we open the list, import (you see a spinner for importing) and then move to the contact detail (with the contact already imported).

In the case of several contacts, we open the list, import (see the spinner), and stay on the list.

That was flow specified for the bluetooth transfer, IMHO, we should be consistent, either have the old experience or the new one.

I think we can have a quick win by just using current code (activity and flow), and adding a better follow up, specially when haida is ready, will be easier, since we could point to specific parts of the app (like going directly to the contact form).

Thanks.

Comment 20

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Small correction: We downgraded the generation of vCard to 3.0, not 4.0.

Comment 21

[Julien Wajsberg [:julienw]]

BTW, I noticed there are 2 different ways of receiving a vcard:

* either as a MMS attachment (iPhone is doing this, possibly some Android phones)
* or as a SMS text (this is done by feature phones)

Given our target market, I think we should do both.

BTW, Stock Android 4.3 does not support any of them; there are applications in the Play Store to support vCard as SMS, but I found nothing to support vCard as MMS. We can't even display the content!

Don't know for Androd 4.4.

Comment 22

[Ralph Daub [:rdaub]]

For tracking purposes - 
A user on the English SUMO forums is asking about this feature: https://support.mozilla.org/en-US/questions/1000847

Comment 24

[Julien Wajsberg [:julienw]]

David, I think you'll want to split the tasks in several subtasks (notably SMS and MMS-based vcards, sending and receiving).

Please ping me if you have any question!

Comment 27

[Jose Manuel Cantera]

After a talk with Francisco, we are going to proceed progressively. 

As a first step in bug 974589 we are going to implement an activity that will allow to list vCard content and import the content (all or nothing) if the user decides to do so. Later, we will consider contact selection, after we see how it works or if it is necessary.

Comment 29

[Julien Wajsberg [:julienw]]

Hey josé, is this bug implemented as part of other bugs now?

Comment 30

[Jose Manuel Cantera]

(In reply to Julien Wajsberg [:julienw] from comment #29)
> Hey josé, is this bug implemented as part of other bugs now?
Hey, 

Currently we are no actively working on it but the plan is to implement it in the near future as a new activity view in order not to pollute the current contact list selection UI. My colleague Adrian will work on it once he finishes some 2.2 blockers he is assigned to right now.

Comment 31

[Adrián de la Rosa]

Attachment: mozilla-b2g:master...ADLR-es:vcard-import-multiple.patch

Asking for feedback about the first iteration of this new feature. When opening a vCard with multiple contacts, it shows a list of all contacts included, with photos if contacts have it and it allows the user to import all contacts. Then it will check for duplicates and tell the user how many contacts were imported before closing the activity.

Comment 32

[GitHub Autolander Bot]

Attachment: [PullReq] ADLR-es:vcard-import-multiple to mozilla-b2g:master

Comment 33

[Jose Manuel Cantera]

Comment on attachment 8559733 [details] [diff] [review]
mozilla-b2g:master...ADLR-es:vcard-import-multiple.patch

I have left some comments on GH and I have given some hints on how to continue with the work

It goes well Adrian

thanks!

Comment 34

[Adrián de la Rosa]

Comment on attachment 8559750 [details] [review]
[PullReq] ADLR-es:vcard-import-multiple to mozilla-b2g:master

PR updated with the changes suggested.

Comment 35

[Jose Manuel Cantera]

Comment on attachment 8559750 [details] [review]
[PullReq] ADLR-es:vcard-import-multiple to mozilla-b2g:master

thanks Adrian,

a new round is needed

best

Comment 36

[Adrián de la Rosa]

Comment on attachment 8559750 [details] [review]
[PullReq] ADLR-es:vcard-import-multiple to mozilla-b2g:master

New f?. If everything goes well, I'll just add tests and ask for review.

Thanks!

Comment 37

[Jose Manuel Cantera]

Comment on attachment 8559750 [details] [review]
[PullReq] ADLR-es:vcard-import-multiple to mozilla-b2g:master

thanks Adrian,

good work, but we need additional refinements before we craft tests

best

Comment 38

[Adrián de la Rosa]

Comment on attachment 8559750 [details] [review]
[PullReq] ADLR-es:vcard-import-multiple to mozilla-b2g:master

All comments addressed once more! I added some tests this time, all of them passing and code working on device. I'll finish tests later today.

Thanks, great suggestions.

Comment 39

[Jose Manuel Cantera]

Comment on attachment 8559750 [details] [review]
[PullReq] ADLR-es:vcard-import-multiple to mozilla-b2g:master

Thanks Adrian,

More nits spotted and some substantive comments on the unit tests. We would need integration tests as well

Next stage would be r?

Comment 40

[Adrián de la Rosa]

Comment on attachment 8559750 [details] [review]
[PullReq] ADLR-es:vcard-import-multiple to mozilla-b2g:master

Everything pointed out about unit tests has been fixed, and a Marionette test to check if all contacts are imported has been added as well.

Let's ask for r? this time ;)

Comment 41

[Jose Manuel Cantera]

Comment on attachment 8559750 [details] [review]
[PullReq] ADLR-es:vcard-import-multiple to mozilla-b2g:master

Hi Adrian,

Overall looks good but I've left some substantive comments on GH. Apart from that comments we need as well two very important things:

A/ Ask for a UI review to Fang. Show to her two different cases: When the contact has a photo and when not. 

B/ I'm worried about the flashing of the UI when the activity is launched. I think is very important to fix that issue. You need to  investigate how to tackle that problem, through a transtion or other mechanism

thanks for the work!

Comment 42

[Adrián de la Rosa]

Comment on attachment 8559750 [details] [review]
[PullReq] ADLR-es:vcard-import-multiple to mozilla-b2g:master

Asking for review since everything has been fixed.
Also asking for ui-review since we are implementing a new view. The new view can be seen here: http://i.imgur.com/p4tm0vN.png

Comment 43

[Jose Manuel Cantera]

Comment on attachment 8559750 [details] [review]
[PullReq] ADLR-es:vcard-import-multiple to mozilla-b2g:master

UI review should be done over screen capture

Comment 44

[Jose Manuel Cantera]

Comment on attachment 8559750 [details] [review]
[PullReq] ADLR-es:vcard-import-multiple to mozilla-b2g:master

r- as the flashing effect is still there. 

On the other hand the file name should not include the 'sms-attachments' string, as I guess this is an internal folder which does not provide any useful information to the user.

Comment 45

[Jose Manuel Cantera]

I would suggest that in order to overcome the flashing issue, We just simply perform a transition for the import view from the bottom to the top, similar to the one we perform when there is only one contact.

Comment 46

[Adrián de la Rosa]

Attachment: Screenshot of new view.

New ui-review because last one was not made properly.

Comment 47

[Adrián de la Rosa]

Comment on attachment 8559750 [details] [review]
[PullReq] ADLR-es:vcard-import-multiple to mozilla-b2g:master

Transition from bottom to top when opening activity is working. There was a css conflict with a class that was overriding the correct transition property.

Comment 48

[Julien Wajsberg [:julienw]]

With those transitions, be careful that we can't trigger some action twice by double tapping a button (tapping once will trigger the activity, but you can actually tap a second time because while the transition is happening the new panel is not hiding the action button.

Comment 49

[Adrián de la Rosa]

The transition is done once Contacts app is launched so buttons are immediately disabled. I tested this and I wasn't able to trigger the activity twice.

Comment 50

[Jose Manuel Cantera]

Comment on attachment 8559750 [details] [review]
[PullReq] ADLR-es:vcard-import-multiple to mozilla-b2g:master

thanks Adrian,

very nice work. We are going to land this despite not having the UI review for Fang. Adrian, please open a follow-up for the visual tweaks of this new view.

Comment 51

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/0ef70b47ef37bcc1dff4b3ea980dcdcbf8a3f9b6

Comment 52

[Adrián de la Rosa]

Comment on attachment 8565402 [details]
Screenshot of new view.

The ui review has been moved to a follow up (bug 1134995).

Comment 53

[Stephany Wilkes]

Fang is out on Chinese New Year, like most of the rest of the Taipei office. I'm flagging Peter La to see if he can take this quick review for Fang during CNY.

Comment 54

[Stephany Wilkes]

Comment on attachment 8559750 [details] [review]
[PullReq] ADLR-es:vcard-import-multiple to mozilla-b2g:master

Adding Peter La on ui-review? in lieu of Fang during CNY. This is, however, landing anyway, but should have gotten review from Fang.

Comment 55

[Jose Manuel Cantera]

(In reply to Stephany Wilkes from comment #54)
> Comment on attachment 8559750 [details] [review]
> [PullReq] ADLR-es:vcard-import-multiple to mozilla-b2g:master
> 
> Adding Peter La on ui-review? in lieu of Fang during CNY. This is, however,
> landing anyway, but should have gotten review from Fang.

Stephanie,

We landed the bug but we have open a follow-up for the visual tweaks (bug 1134995). 

thanks

Comment 56

[Fang Shih [:fang]]

I've replied the review on bug 1134995, Thanks!

Comment 57

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

Comment on attachment 8559750 [details] [review]
[PullReq] ADLR-es:vcard-import-multiple to mozilla-b2g:master

We are adding a lot of code changes here and would like to know what we are doing.

Flagging myself to get that understanding.

Comment 58

[Maria Angeles Oteo (:oteo)]

I am changing the bug title because the original one is not correct. The patch provided in this bug allow importing multiple contacts vcards but the user is not able to select which contacts he wants to import or not to his device so there are only two options:
- Import all the contacts (in case of duplicated contacts, passive merging rules are applied)
- Cancel the import option