Closed Bug 1043177 Opened 10 years ago Closed 10 years ago

Redesign Contact edit page

Categories

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

x86
macOS
defect
Not set
normal

Tracking

(feature-b2g:2.1, tracking-b2g:backlog)

VERIFIED FIXED
2.1 S3 (29aug)
feature-b2g 2.1
tracking-b2g backlog

People

(Reporter: cawang, Assigned: sergi)

References

Details

(Keywords: feature, Whiteboard: [tako])

Attachments

(6 files, 1 obsolete file)

The current Contact edit page is quite messy and the input fields are not grouped or highlighted base on their attributes. Attach the UX proposal on this issue.
[Blocking Requested - why for this release]:
blocking-b2g: --- → 2.1?
Keywords: feature
Whiteboard: [tako]
blocking-b2g: 2.1? → backlog
feature-b2g: --- → 2.1
Assignee: nobody → sergi.mansilla
Carrie we still need to find a solution for removing the photo. We cannot add the 'delete photo' to that menu since that's an activity.

Also, IMHO, the first visual proposal looks simplier to implement, do you know when we will have the visual assests?

Thanks a lot!
Flags: needinfo?(cawang)
QA Whiteboard: [COM=Gaia::Contacts]
Hi Francisco, 

I'll upload my spec here and my decision is...accepting 2 action menus :(
However is this something we can fix in the future? I mean from system side, can they make some changes to detect different scenario and provide corresponding options in the action menu?
Thanks!
Flags: needinfo?(cawang)
(In reply to Carrie Wang [:carrie] from comment #4)
> Hi Francisco, 
> 
> I'll upload my spec here and my decision is...accepting 2 action menus :(
> However is this something we can fix in the future? I mean from system side,
> can they make some changes to detect different scenario and provide
> corresponding options in the action menu?
> Thanks!

Thanks Carrie, I'm sure we can revisit this pattern for 2.2 and try to find a better approach. We can ask the system frontend team, but again it's a ux decision on the SFE team to reuse the action menu of an activity to add custom actions.
Hi again,

while reviewing part of our work for this in bug 1048160, I just remember that we have a special case for facebook contacts.

For those contacts almost all the information is not editable by contract with FB. So in the new design we will need to come with a disable state too.

Sorry for noticing this late Carrie :(
Flags: needinfo?(cawang)
Hi Carrie, could we have mockups for other fields such as Email and Address? I only need it for Mockup 1.

Thanks!
Hi Francisco, 

Not a big problem. I'll update it here. :)
Flags: needinfo?(cawang)
ni? Fang for Sergi's request.
Flags: needinfo?(fshih)
Hi Sergi, 

I'll provide the visual mock spec for contact fields, it'll also include the entire fields.
The final visual still in review progress. I will attach the spec once it's approved. Thanks!
Flags: needinfo?(fshih)
QA Contact: jlorenzo
Flags: in-moztrap?(jlorenzo)
Target Milestone: --- → 2.1 S3 (29aug)
Thanks Fang! Will you be able to provide icons and images as well?
Flags: needinfo?(fshih)
Thanks Francisco! 

Also have the spec at bug 1043166, not sure which one should we use for tracking the contacts implementation?! Thanks!
Flags: needinfo?(fshih)
This redesign has already been covered by the meta-bug: bug 1026690 comment 16.
Flags: in-moztrap?(jlorenzo) → in-moztrap+
Hi Fang,

Thanks for your comment! One question, the spec doesn't specify how it looks when the user adds a field when there is an existing one, do you think you could add that? I am referring to when the user has, for example, two or three different phones for a particular contact. I am stacking the input fields up now and I'd like to know if that's how it should look.
Flags: needinfo?(fshih)
Hi Sergi,

You may take a look at the videos I made, it covered how it looks when user adds a field when there is an existing one. https://www.dropbox.com/s/z9tzu22ogu502bs/Add_Contact_Transition.mov?dl=0   
Hope this covered what you need. Thanks!
Flags: needinfo?(fshih)
Hi Fang,

That was very helpful, thanks! One more detail: I don't see the 'Delete Contact' button in any of the screens. This button appears at the bottom of the edit screen when you edit an existing contact. Could you tell me where can I find the new version?
Flags: needinfo?(fshih)
Hi Sergi,
Attached the visual spec with the 'Delete Contact' button in it, Thanks!! : )
Flags: needinfo?(fshih)
Attached file Github Pull Request
Attachment #8481279 - Flags: review?(francisco)
Comment on attachment 8481279 [details] [review]
Github Pull Request

Hei Sergi ... wow, this work is amazing.

I did a quick test and found 3 issues:

- We always show a date empty field (commented on github where it's happening)
- We need to create a new string for the 'Edit Photo'
- When editing a FB, the disabled fields with data look a bit weird, too close to the left margin
- When a user has a photo we don't show the option to remove the photo, just change it. We will need to add an action menu there

A part from that looking pretty good!
Attachment #8481279 - Flags: review?(francisco)
I also discovered another side effect, we have some red buttons in contacts settings.

Also leaved a comment in github for this.
Comment on attachment 8481279 [details] [review]
Github Pull Request

Thanks Francisco for your detailed report! I applied the changes, tested it and it looks good. Please feel free to merge it after you give it the r+.

Thanks again!
Attachment #8481279 - Flags: review?(francisco)
Hei Sergi good job.

There is still the last patch to be applied, the one for allow photo to be removed. I'm attaching it.
Flags: needinfo?(sergi.mansilla)
Comment on attachment 8481279 [details] [review]
Github Pull Request

Also you'll need to rebase, but I checked and it's a simple conflict in the properties file.

Last thing, unit test are not passing, probably cause we changed the dom, we will need to fix that as well.

In the meantime will try to take a look to them to see if I can give you a hand.

With unit test passing I think we are ready to land this!

Will be asking for ui-review as well
Attachment #8481279 - Flags: review?(francisco)
Included remove_photo patch, rebased code and fixed unit tests.
Flags: needinfo?(sergi.mansilla)
Comment on attachment 8481279 [details] [review]
Github Pull Request

Great job Sergi, thanks for your help!
Attachment #8481279 - Flags: review+
Attached image 2014-08-30-14-27-44.png
Carrie, would you kindly review the output from Sergi's work?
Attachment #8482013 - Flags: ui-review?(cawang)
Comment on attachment 8481900 [details] [diff] [review]
remove_photo.diff

Hey guys, 

Can I review it from with  patch so that I can focus on the interaction part.
I'll leave this ui review to Fang since she's aware of the visual part. Thanks!
Flags: needinfo?(fshih)
Sure, thanks Carrie!
Hey guys, Overall it looks pretty good! : ) 
The only thing I wonder is in the back of the "edit picture" it looks like have a light gray box underneath?
And also the icon looks a bit fuzzy in the screenshot, other than that It looks great. 
Can I also get the screenshot of the one with close field? So I can see the spacing between the list. Thanks!
Flags: needinfo?(fshih)
Attachment #8482013 - Flags: ui-review?(cawang) → ui-review-
Attached image 2014-09-01-10-35-47.png
Screenshot with closed fields
As we have been speaking offline, we will land this, since Fang is not really blocking on the UI and we can address her comments in follow ups.

Thanks every one!
Landed:

https://github.com/mozilla-b2g/gaia/commit/6475adb2aabaf84510047c74d381ad5fce9f42b8

Had to run several times the integration tests, as we had some calendar and notification errors, not related to this patch.

But merged just when we got (at least) 1 green in all the builds!
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Verified landed in 
Gaia      fbb297c39aab5f17b179533d2a9a6c5166b2c197
Gecko     https://hg.mozilla.org/releases/mozilla-aurora/rev/fb5e796da813
BuildID   20140902160204
Version   34.0a2
ro.build.version.incremental=110
ro.build.date=Fri Jun 27 15:57:58 CST 2014
B1TC00011230
Status: RESOLVED → VERIFIED
Depends on: 1062003
Depends on: 1133152
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.