Closed
Bug 1240516
Opened 8 years ago
Closed 8 years ago
Display message in chat indicating that a user has left the session
Categories
(Hello (Loop) :: Client, defect, P2)
Hello (Loop)
Client
Tracking
(Not tracked)
RESOLVED
FIXED
Iteration:
47.2 - Feb 22
People
(Reporter: sevaan, Assigned: vidhuran2012)
References
Details
User Story
- When a user leaves the session, "Your friend has left." is added to the text chat. - An icon indicating the person has left is shown alongside the chat bubble. - A timestamp is added if it is a new minute since the last message (implementation note: this is existing functionality). - The bubbles look the same as the existing chat bubbles - no layout changes required except to layout the new text & icon. See comment 3 for tech notes.
Attachments
(6 files, 3 obsolete files)
123.02 KB,
image/png
|
Details | |
1.42 KB,
application/zip
|
Details | |
40 bytes,
text/x-github-pull-request
|
standard8
:
review+
|
Details | Review |
114.30 KB,
image/png
|
Details | |
2.99 KB,
image/png
|
Pau
:
ui-review+
|
Details |
89.97 KB,
image/png
|
Pau
:
ui-review+
|
Details |
When a user leaves the room, a message should be displayed in chat, along with a timestamp, indicating this event.
Reporter | ||
Comment 1•8 years ago
|
||
Suggested string: Your friend has left.
Updated•8 years ago
|
Whiteboard: [triage]
Updated•8 years ago
|
Rank: 28
Priority: -- → P2
Whiteboard: [triage]
Comment 2•8 years ago
|
||
Sevaan: I believe there's some UX needed for this? Layout/style for the text chat pane?
Flags: needinfo?(sfranks)
Comment 3•8 years ago
|
||
Tech note: The `activeRoomStore` already does the right thing on receiving the `RemotePeerDisconnected` action. I think we can hook up `TextChatStore` to listen for `RemotePeerDisconnected` and add a message (we'll likely need a new message type). Might be worth combining with bug 1240517, as the different string there should just be a matter of checking the peerHungup parameter.
Reporter | ||
Updated•8 years ago
|
Flags: needinfo?(sfranks) → needinfo?(b.pmm)
Comment 4•8 years ago
|
||
Here's the design for this. I'll upload a visual spec and the icon shortly.
Flags: needinfo?(b.pmm)
Comment 5•8 years ago
|
||
Pau: Please can we have the icon? Also, I'm assuming that we can use the old style of chat bubbles until such time as we've done the work for the newer style (which hasn't been scheduled yet).
Assignee: nobody → vidhuran2012
User Story: (updated)
Flags: needinfo?(b.pmm)
Comment 7•8 years ago
|
||
Visual Spec for this in-chat notification.
Comment 8•8 years ago
|
||
Assignee | ||
Comment 9•8 years ago
|
||
Attachment #8717455 -
Flags: ui-review?(b.pmm)
Assignee | ||
Comment 10•8 years ago
|
||
Attachment #8717456 -
Flags: ui-review?(b.pmm)
Comment 11•8 years ago
|
||
Vidhuran can you confirm the space between the icon and the text is 10px, please?
Flags: needinfo?(vidhuran2012)
Assignee | ||
Comment 12•8 years ago
|
||
(In reply to Pau Masiá [:Pau] from comment #11) > Vidhuran can you confirm the space between the icon and the text is 10px, > please? It is actually 8px, I will change it to 10px.
Flags: needinfo?(vidhuran2012)
Comment 13•8 years ago
|
||
(In reply to Vidhuran Harichandra Babu from comment #12) > (In reply to Pau Masiá [:Pau] from comment #11) > > Vidhuran can you confirm the space between the icon and the text is 10px, > > please? > > It is actually 8px, I will change it to 10px. I was wrong, 10px is the left side of the icon. 5px needed for the right side. Also, is there a reason for having that displayed in the message? It should always be sans serif. Thanks!
Flags: needinfo?(vidhuran2012)
Comment 14•8 years ago
|
||
that font displayed...*
Assignee | ||
Comment 15•8 years ago
|
||
(In reply to Pau Masiá [:Pau] from comment #13) In the attachments (https://bug1240516.bmoattachments.org/attachment.cgi?id=8714337) the font face was specified as Arial MT. And , I will set the right margin of the icon to be 5px.
Flags: needinfo?(vidhuran2012)
Comment 16•8 years ago
|
||
Ok. Here's the final spec. Thanks!
Attachment #8714337 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Attachment #8717455 -
Attachment is obsolete: true
Attachment #8717455 -
Flags: ui-review?(b.pmm)
Assignee | ||
Updated•8 years ago
|
Attachment #8717456 -
Attachment is obsolete: true
Attachment #8717456 -
Flags: ui-review?(b.pmm)
Assignee | ||
Comment 17•8 years ago
|
||
Attachment #8717495 -
Flags: ui-review?(b.pmm)
Assignee | ||
Comment 18•8 years ago
|
||
Attachment #8717496 -
Flags: ui-review?(b.pmm)
Assignee | ||
Comment 19•8 years ago
|
||
I didn't set the font to Lucia Grande as in the spec , it too looked different , so i just let it inherit the font face and it is now Open Sans.
Updated•8 years ago
|
Attachment #8717495 -
Flags: ui-review?(b.pmm) → ui-review+
Updated•8 years ago
|
Attachment #8717496 -
Flags: ui-review?(b.pmm) → ui-review+
Comment 20•8 years ago
|
||
Vidhuran: I was in the area so I added a couple of comments on the PR for you.
Assignee | ||
Updated•8 years ago
|
Attachment #8717452 -
Flags: review?(standard8)
Assignee | ||
Comment 21•8 years ago
|
||
(In reply to Mark Banner (:standard8) from comment #20) Standard8: Sorry for the delay, I've now addresses your comments and added dome tests and updated the ui showcase. I'm requesting a review of my PR. Thanks.
Comment 22•8 years ago
|
||
Sorry, yesterday I had a day off, and I'm short on time today, I'll get to the review tomorrow at the latest.
Comment 23•8 years ago
|
||
Comment on attachment 8717452 [details] [review] [loop] Vidhuran:textchat-notify-peerdisconnect > mozilla:master Looks great! If you can address the small comments I posted there, and add ", r=Standard8" to the commit message, that would be great. Bonus points for a rebase on latest master. Once you've done that, please needinfo me or ping me on irc to get this landed. Sorry about the delay in reviewing.
Attachment #8717452 -
Flags: review?(standard8) → review+
Comment 24•8 years ago
|
||
Thank you for working on this. https://github.com/mozilla/loop/commit/a7a03bcad2225372c4041fd6947de091ef3c7c21
Status: NEW → RESOLVED
Iteration: --- → 47.2 - Feb 22
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•