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

RESOLVED FIXED in Firefox OS master

Status

RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: khu, Assigned: hola)

Tracking

({feature, verifyme})

unspecified
2.2 S6 (20feb)
ARM
Gonk (Firefox OS)
feature, verifyme
Dependency tree / graph

Firefox Tracking Flags

(tracking-b2g:backlog, b2g-master fixed)

Details

(Whiteboard: ux-most-wanted[priority])

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

6 years ago
Because of conformance.
(Reporter)

Updated

6 years ago
blocking-b2g: --- → leo?
(Reporter)

Comment 1

6 years ago
Will mark it as leo+ for now. Can definitely leo? or - if someone has different view on this.
blocking-b2g: leo? → leo+
Let me do some studies first. Start from google://vcard spec 2.1
Assignee: nobody → timdream

Comment 3

6 years ago
Kevin, do we have any ux spec or user stories of supporting vCard? thanks.
change to leo? as product is confirming this is out of scope
blocking-b2g: leo+ → leo?
(Reporter)

Comment 5

6 years ago
(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.
blocking-b2g: leo? → koi+
Sorry, off by 1.
blocking-b2g: koi+ → -

Comment 7

6 years ago
Bug 823492 and bug 838121 are connected to this as well.
Keywords: feature
No longer blocks: 840063
Assignee: timdream → nobody
Duplicate of this bug: 911064
Duplicate of this bug: 885683
Component: Gaia → Gaia::SMS
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → ARM
Duplicate of this bug: 924242
A little late on the game (as the other was already resolved), but doesn't this bug collide with bug 920533 ?
We can support both mime types when receiving a MMS, right ? :)
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?
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...

Updated

5 years ago
blocking-b2g: - → 1.5?
Blocks: 985429
Flags: needinfo?(ofeng)
added to 1.5 targeted
blocking-b2g: 1.5? → backlog
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.

Updated

5 years ago
Whiteboard: ux-most-wanted

Updated

5 years ago
Blocks: 994991
Created attachment 8413668 [details]
[1.5 Messages] vCard v1.0.pdf

Here is the UX spec. Let me know your feedback if any.
Flags: needinfo?(ofeng)
Needs Francisco's feedback as well.
Flags: needinfo?(francisco.jordano)
Whiteboard: ux-most-wanted → ux-most-wanted[priority]
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.
Flags: needinfo?(francisco.jordano)
Small correction: We downgraded the generation of vCard to 3.0, not 4.0.
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

4 years ago
For tracking purposes - 
A user on the English SUMO forums is asking about this feature: https://support.mozilla.org/en-US/questions/1000847
Duplicate of this bug: 1030677
Assignee: nobody → david.garciaparedes
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!
Duplicate of this bug: 974276
Status: NEW → ASSIGNED
Blocks: 974276
Depends on: 1059715
Duplicate of this bug: 1064817
Depends on: 974589

Updated

4 years ago
Assignee: david.garciaparedes → mri

Updated

4 years ago
Summary: To support vCard 2.1 (mime-type: text/x-vCard) → [Contacts] [vCard] Allow the user to select which contacts must be imported from vCard files

Updated

4 years ago
Assignee: mri → nobody

Comment 27

4 years ago
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.
Depends on: 1083225
Depends on: 1083226
No longer depends on: 1083225
No longer depends on: 1083226
Depends on: 1084225
(Assignee)

Updated

4 years ago
Depends on: 1087402
Component: Gaia::SMS → Gaia::Contacts
Duplicate of this bug: 1103846
Status: ASSIGNED → NEW
Hey josé, is this bug implemented as part of other bugs now?
Flags: needinfo?(jmcf)

Comment 30

4 years ago
(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.
Flags: needinfo?(jmcf)
(Assignee)

Updated

4 years ago
Assignee: nobody → hola
(Assignee)

Comment 31

4 years ago
Created attachment 8559733 [details] [diff] [review]
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.
Attachment #8559733 - Flags: feedback?(jmcf)
Created attachment 8559750 [details] [review]
[PullReq] ADLR-es:vcard-import-multiple to mozilla-b2g:master

Comment 33

4 years ago
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!
Attachment #8559733 - Attachment is obsolete: true
Attachment #8559733 - Flags: feedback?(jmcf)
(Assignee)

Comment 34

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

PR updated with the changes suggested.
Attachment #8559750 - Flags: feedback?(jmcf)

Comment 35

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

thanks Adrian,

a new round is needed

best
Attachment #8559750 - Flags: feedback?(jmcf)
(Assignee)

Comment 36

4 years ago
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!
Attachment #8559750 - Flags: feedback?(jmcf)

Comment 37

4 years ago
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
Attachment #8559750 - Flags: feedback?(jmcf)
(Assignee)

Comment 38

4 years ago
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.
Attachment #8559750 - Flags: feedback?(jmcf)

Comment 39

4 years ago
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?
Attachment #8559750 - Flags: feedback?(jmcf) → feedback+
(Assignee)

Comment 40

4 years ago
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 ;)
Attachment #8559750 - Flags: review?(jmcf)

Comment 41

4 years ago
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!
Attachment #8559750 - Flags: review?(jmcf)
(Assignee)

Comment 42

4 years ago
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
Attachment #8559750 - Flags: ui-review?(fshih)
Attachment #8559750 - Flags: review?(jmcf)

Comment 43

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

UI review should be done over screen capture
Attachment #8559750 - Flags: ui-review?(fshih)

Comment 44

4 years ago
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.
Attachment #8559750 - Flags: review?(jmcf) → review-

Comment 45

4 years ago
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.
(Assignee)

Comment 46

4 years ago
Created attachment 8565402 [details]
Screenshot of new view.

New ui-review because last one was not made properly.
Attachment #8565402 - Flags: ui-review?(fshih)
(Assignee)

Comment 47

4 years ago
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.
Attachment #8559750 - Flags: review- → review?(jmcf)
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.
(Assignee)

Comment 49

4 years ago
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

4 years ago
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.
Attachment #8559750 - Flags: review?(jmcf) → review+

Updated

4 years ago
Keywords: checkin-needed

Updated

4 years ago
Keywords: checkin-needed
(Assignee)

Updated

4 years ago
Blocks: 1134990

Updated

4 years ago
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED

Updated

4 years ago
Blocks: 1134995

Updated

4 years ago
Keywords: verifyme
(Assignee)

Comment 52

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

The ui review has been moved to a follow up (bug 1134995).
Attachment #8565402 - Attachment is obsolete: true
Attachment #8565402 - Flags: ui-review?(fshih)

Updated

4 years ago
status-b2g-master: --- → fixed
Target Milestone: --- → 2.2 S6 (20feb)

Comment 53

4 years ago
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.
Flags: needinfo?(pla)

Comment 54

4 years ago
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.
Attachment #8559750 - Flags: ui-review?(pla)

Comment 55

4 years ago
(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

4 years ago
I've replied the review on bug 1134995, Thanks!
Flags: needinfo?(pla)

Updated

4 years ago
Attachment #8559750 - Flags: ui-review?(pla) → ui-review-
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.
Attachment #8559750 - Flags: review?(francisco)
Attachment #8559750 - Flags: review?(francisco)

Updated

4 years ago
Blocks: 1142185
blocking-b2g: backlog → ---
tracking-b2g: --- → backlog
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
Summary: [Contacts] [vCard] Allow the user to select which contacts must be imported from vCard files → [Contacts] [vCard] Allow the user to import multiple contacts from vCard files
See Also: → bug 1178975
You need to log in before you can comment on or make changes to this bug.