190.04 KB, image/png
480.88 KB, application/pdf
378.48 KB, image/jpeg
55.23 KB, image/png
46 bytes, text/x-github-pull-request
Jose Manuel Cantera: review+
|Details | Review | Splinter Review|
7.60 MB, video/3gpp
Follow-up of bug 849729. Adrian, please ask for a ui-review from Fang and then we will know whether we would need visual tweaks or the current implementation is good enough. thanks
Created attachment 8567002 [details] Screenshot of new view.
Attachment #8567002 - Flags: ui-review?(fshih)
Comment on attachment 8567002 [details] Screenshot of new view. Thanks for the screenshot. I've checked with UX designer and UX spec. The original design is using the layout of contact list view. We might miss out the search field and index here. Also the upper right button "import" should be "import all" like the UX spec. Thanks!
Attachment #8567002 - Flags: ui-review?(fshih) → ui-review-
Created attachment 8568421 [details] UX_Spec_vcard_multiple.png Attached the screen of the UX spec. Please let me know if you need anything else. Thank you!
Fang, Carrie, Although I'm sympathetic to the UX design, the reality is that there are not going to be vcards with so many contacts that would justify the necessity of the searchbox or the alphascroll. Could we live with the current design if we change 'Import' by 'Import All' ? thanks!
I added a PR which shows how we could integrate the side index. What I found is that the component is dangerously coupled to the main contact list so it would require first to decouple it in an independent module to avoid a handful of related bugs and conflicts. I have to side here with Jose Manuel and add this to the argument. Maybe a simpler view without search or alpha scrolling would be enough here? Thanks.
Hi, I've discussed with Omega (he did the spec) and both of us agree that this is a nice to have item. So we are fine with removing it (the search field). Thanks!
Attachment #8569148 - Attachment is obsolete: true
Comment on attachment 8569902 [details] [review] [gaia] ADLR-es:vcard-button-text > mozilla-b2g:master Button changed to "Import all" instead of "Import"
Attachment #8569902 - Flags: review?(jmcf)
Created attachment 8569923 [details] multiple-vcard-view.png Two screenshots added, one for several contacts without photo and another one for contacts with photo.
Attachment #8569902 - Flags: review?(jmcf) → review+
Comment on attachment 8569923 [details] multiple-vcard-view.png Visually looks fine, but I'm not sure about showing the phone number in the list, ni? Carrie for UX issue. Thanks!
Attachment #8569923 - Flags: ui-review?(fshih) → ui-review-
Hi guys, Can you attach the vcard spec for me as a reference? Thanks!
Created attachment 8571802 [details] [1.5 Messages] vCard v1.0.pdf Atttached UX spec delivered by Omega in bug 849729
Hi guys, I wonder why the numbers are listed underneath the contacts' names? In Contacts or contact picker, it will display only the company name (if the contact has a company name set) as the second line. So I'd say we should follow the same rule here. Thanks!
Created attachment 8573873 [details] 2015-03-06-06-23-05.png Sorry about that. The issue is fixed. Right now the organization appears below the contact name as it should. New screenshot to ask for ui-review.
Comment on attachment 8573873 [details] 2015-03-06-06-23-05.png Currently the divider is passed over the contact photo. It should be shorter. Also the contact photo is a bit offset. It should move 0.7rem to the right. So the spacing can be equal on both sides. Thank you!
Attachment #8573873 - Flags: ui-review?(fshih) → ui-review-
Created attachment 8574559 [details] import_vcard.jpg Attached the spac for your reference, Please let me know if you need more information. Thanks
Created attachment 8575907 [details] 2015-02-25-02-53-20.png Patch updated. New screenshot submitted for ui-review.
Comment on attachment 8575907 [details] 2015-02-25-02-53-20.png Looks good! Thanks!
Attachment #8575907 - Flags: ui-review?(fshih) → ui-review+
Thanks for your help polishing this one, let's see if we can merge it into master.
Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/407e611a0bef56bbdbaa5a2fba1f7afa51a7a733
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
status-b2g-master: affected → fixed
Target Milestone: --- → 2.2 S8 (20mar)
Backed out for Gaia unit test failures. Note that they were present in your Gaia Try runs as well. Please be sure to double-check that before merging in the future. Master: https://github.com/mozilla-b2g/gaia/commit/0864c13f863a0348defab74135aa0210e81e521f https://treeherder.mozilla.org/logviewer.html#?job_id=1520664&repo=b2g-inbound
Status: RESOLVED → REOPENED
status-b2g-master: fixed → affected
Resolution: FIXED → ---
Comment on attachment 8578118 [details] [review] [gaia] ADLR-es:vcard-button-text > mozilla-b2g:master Sorry, I will. I forgot to update the unit tests
Attachment #8578118 - Flags: review?(jmcf)
Attachment #8569902 - Attachment is obsolete: true
Comment on attachment 8578118 [details] [review] [gaia] ADLR-es:vcard-button-text > mozilla-b2g:master thanks Adrian. please check the tiny nit reported on GH
Attachment #8578118 - Flags: review?(jmcf) → review+
Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/d5fb945b48069a6745c60617e3203e3f9ca3ea01
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago → 3 years ago
Resolution: --- → FIXED
status-b2g-master: affected → fixed
Target Milestone: 2.2 S8 (20mar) → 2.2 S9 (3apr)
This isn't going to land on 2.2 right? Asking this from localization perspective, because 2.2 string freeze is on April 6th. thanks
(In reply to Delphine Lebédel [:delphine - in SA for l10n events until April 7th, expect delay in answering] from comment #27) > This isn't going to land on 2.2 right? Asking this from localization > perspective, because 2.2 string freeze is on April 6th. thanks Hi Delphine, Multiple contacts vcards handling didn't land in 2.2 branch and it's not planned to be supported it in that branch so status-b2g-v2.2 is unaffected. Thanks for asking!
status-b2g-v2.2: --- → unaffected
This bug has been verified as "pass" on latest Nightly build of Flame v3.0 and Nexus5 v3.0. STR： 1.Receive a mail with contacts '.vcf' files. 2.Download and open the contacts '.vcf' file. 3.Import these contacts. **Results: a. It shows "Import all" instead of "Import". b. The contacts list is alphabetical in "Import all" view. c. It doesn't show numbers underneath the contacts' names, and shows "false" or company. d. The divider spacing about Contact photo is correct. e. It imports all contacts successfully. See attachment: verified_v3.0.3gp Reproduce rate: 0/10 Device: Flame v3.0 build(Verified) Build ID 20150624080416 Gaia Revision eb0d4aefa62b20420d6fa0642515a110daca5d97 Gaia Date 2015-06-24 01:48:14 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/4cdc1a95a672 Gecko Version 41.0a1 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150624.153831 Firmware Date Wed Jun 24 15:38:44 EDT 2015 Bootloader L1TC000118D0 Device: Nexus5 v3.0 build(Verified) Build ID 20150624010204 Gaia Revision 311c4e59936a407e64509f54fecb440d8a78e3c8 Gaia Date 2015-06-20 20:21:42 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/be81b8d6fae9 Gecko Version 41.0a1 Device Name hammerhead Firmware(Release) 5.1 Firmware(Incremental) eng.cltbld.20150624.043116 Firmware Date Wed Jun 24 04:31:35 EDT 2015 Bootloader HHZ12f
Status: RESOLVED → VERIFIED
status-b2g-master: fixed → verified
Opened bug 1178418 to tracked the issue reported in Comment 29 (Result c) about importing a multiple contacts vcard, if a contact does not have Company field, "false" is shown in the import contacts list
See Also: → bug 1178418
You need to log in before you can comment on or make changes to this bug.