Desktop client needs the ability to edit a contact locally

VERIFIED FIXED in Firefox 34

Status

defect
P2
normal
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: enndeakin, Assigned: Paolo)

Tracking

unspecified
mozilla35
Points:
2
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify +

Firefox Tracking Flags

(firefox34 verified, firefox35 verified)

Details

(Whiteboard: [contacts, first release needed][loop-uplift][fig:wontverify])

Attachments

(1 attachment, 1 obsolete attachment)

The UI is section 6 of https://people.mozilla.org/~dhenein/labs/loop-mvp-spec/#contacts

1. The edit UI can be opened by selecting Edit Contact from the context menu. Perhaps a key shortcut should also be available. 
2. A page slides into view containing textboxes for the Name and Email. The current value of each will be filled in from the existing info in the contact list.
3. When the done button is pressed, the contact info is updated and the page slides back to the main list.

To implement:

1. Add a context menu item labeled 'Edit Contact...'
2. Implement the side page with the two textboxes with the current values prefilled.
3. Slide the page in and out as needed.
Note that some contacts cannot be edited. The Edit Contact option won't appear in the dropdown.
Priority: -- → P2
Whiteboard: [contacts, first release needed]
Target Milestone: --- → mozilla34
Flags: firefox-backlog?
Whiteboard: [contacts, first release needed] → [contacts, first release needed][loop-uplift]
Target Milestone: mozilla34 → mozilla35
Duplicate of this bug: 1038252
Summary: Implement UI for editing a contact → Desktop client needs the ability to edit a contact locally
Points: --- → 2
Flags: firefox-backlog? → firefox-backlog+
Flags: qe-verify?
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Flags: qe-verify? → qe-verify+
Posted patch The patch (obsolete) — Splinter Review
I hijacked the dropdown button for now, until the dropdown is ready. Since this is not the final button, it is not visibly disabled for imported contacts, but the edit functionality is not available there as requested.
Attachment #8495927 - Flags: review?(mdeboer)
Blocks: 1071633
No longer blocks: 1071633
Depends on: 1071633
Posted patch Updated patchSplinter Review
Now based on bug 1071633.
Attachment #8495927 - Attachment is obsolete: true
Attachment #8495927 - Flags: review?(mdeboer)
Attachment #8496841 - Flags: review?(mdeboer)
Comment on attachment 8496841 [details] [diff] [review]
Updated patch

Review of attachment 8496841 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/loop/content/js/contacts.jsx
@@ +277,5 @@
> +          if (contact.category[0] == "google") {
> +            // Imported contacts cannot be edited.  For the moment, the check
> +            // for determining whether the contact is imported is based on its
> +            // category, and also results in disabling the edit button.
> +            return;

And check will be removed as "canModify" does it now.
Blocks: 1038257
Comment on attachment 8496841 [details] [diff] [review]
Updated patch

Review of attachment 8496841 [details] [diff] [review]:
-----------------------------------------------------------------

r=me once the dropdown (bug 1071633) has landed.

::: browser/components/loop/content/js/contacts.jsx
@@ +19,5 @@
>  
>    const ContactDropdown = React.createClass({
>      propTypes: {
>        handleAction: React.PropTypes.func.isRequired,
> +      canModify: React.PropTypes.bool

nit: can you rename this to 'canEdit'? Just to have the terms used for actions kept to a minimum...
Attachment #8496841 - Flags: review?(mdeboer) → review+
(In reply to Mike de Boer [:mikedeboer] from comment #6)
> nit: can you rename this to 'canEdit'? Just to have the terms used for
> actions kept to a minimum...

Yeah, I called it differently as it controlled both "edit" and "remove", but probably this doesn't add clarity...
Iteration: --- → 35.3
https://hg.mozilla.org/mozilla-central/rev/8d456f170b1e
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
QA Contact: anthony.s.hughes
Wontverify for Fig but this will get verified for official branches.
Whiteboard: [contacts, first release needed][loop-uplift] → [contacts, first release needed][loop-uplift][fig:wontverify]
Pauly -- This is ready to be verified.
Flags: needinfo?(paul.silaghi)
Verified fixed FF 35.0a1 (2014-10-06) Win 7, Ubuntu 13.04, OS X 10.10
Status: RESOLVED → VERIFIED
Flags: needinfo?(paul.silaghi)
Comment on attachment 8496841 [details] [diff] [review]
Updated patch

Approval Request Comment
Part of the staged Loop aurora second uplift set
Attachment #8496841 - Flags: approval-mozilla-aurora?
Flags: needinfo?(paul.silaghi)
Depends on: 1083744
Verified fixed FF 34b1 OS X 10.9.5, Win 7
Flags: needinfo?(paul.silaghi)
Comment on attachment 8496841 [details] [diff] [review]
Updated patch

Already landed in aurora, approving for posterity
Attachment #8496841 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Does this have developer automation coverage?
Flags: in-testsuite?
Clearing in-testsuite requests for features that are being removed from Hello as part of te user journey work in bug 1209713.
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.