Closed
Bug 1212331
Opened 9 years ago
Closed 9 years ago
Remove the Let's talk about context addition options from the panel
Categories
(Hello (Loop) :: Client, defect, P2)
Hello (Loop)
Client
Tracking
(firefox44 fixed)
Tracking | Status | |
---|---|---|
firefox44 | --- | fixed |
People
(Reporter: standard8, Assigned: mancas)
References
Details
(Whiteboard: [web sharing])
User Story
Acceptance criteria: - "Let's talk about:" in the panel is removed from the panel. - All associated css removed. - All redundant strings removed. - Any relevant views removed from ui-showcase. - Any redundant prefs removed from firefox.js - The backend for getting context via mozLoop stays in place as we'll need it for another part of the feature later.
Attachments
(1 file, 2 obsolete files)
28.27 KB,
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
As part of the user journey we're not going to have manual context addition. Therefore we should remove it. Depends on bug 1212074. See user story for more details.
Reporter | ||
Updated•9 years ago
|
Rank: 12
Reporter | ||
Updated•9 years ago
|
Reporter | ||
Updated•9 years ago
|
Mentor: standard8
Whiteboard: [lang=js] → [web sharing]
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → b.mcb
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8671763 -
Flags: review?(standard8)
Assignee | ||
Comment 2•9 years ago
|
||
Patch ready to be reviewed. Notice that I've kept the creation of the room with a context since we will need this code in the future.
Assignee | ||
Comment 3•9 years ago
|
||
Attachment #8671763 -
Attachment is obsolete: true
Attachment #8671763 -
Flags: review?(standard8)
Attachment #8671764 -
Flags: review?(standard8)
Reporter | ||
Comment 4•9 years ago
|
||
Comment on attachment 8671764 [details] [diff] [review] Remove the Let's talk about context addition options from the panel Review of attachment 8671764 [details] [diff] [review]: ----------------------------------------------------------------- This is good, I'd just prefer to tidy up the preference removal as well, so that we don't forget it later. ::: browser/app/profile/firefox.js @@ -1725,5 @@ > pref("loop.support_url", "https://support.mozilla.org/kb/group-conversations-firefox-hello-webrtc"); > pref("loop.contacts.gravatars.show", false); > pref("loop.contacts.gravatars.promo", true); > pref("loop.browserSharing.showInfoBar", true); > -pref("loop.contextInConversations.enabled", true); Well spotted. Could you go through the code and tidy up the other instances as well please: http://mxr.mozilla.org/mozilla-central/search?string=contextInConversations.enabled I think for roomViews.jsx, we can just pretend it is true.
Attachment #8671764 -
Flags: review?(standard8)
Assignee | ||
Comment 5•9 years ago
|
||
I've tidied up the code, please check the new version of the patch
Attachment #8671764 -
Attachment is obsolete: true
Attachment #8672978 -
Flags: review?(standard8)
Reporter | ||
Comment 6•9 years ago
|
||
Comment on attachment 8672978 [details] [diff] [review] Remove the Let's talk about context addition options from the panel Review of attachment 8672978 [details] [diff] [review]: ----------------------------------------------------------------- I just realised that this has the side effect of always adding context to a conversation, but I think that's fine as we're going to want something like that when we do the automatic sharing. We can figure out any additional changes to the flow when we get there. r=Standard8
Attachment #8672978 -
Flags: review?(standard8) → review+
Comment 8•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c41533f5754b
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox44:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Updated•9 years ago
|
Iteration: --- → 44.2 - Oct 19
You need to log in
before you can comment on or make changes to this bug.
Description
•