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)

defect

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)

When a user leaves the room, a message should be displayed in chat, along with a timestamp, indicating this event.
Suggested string: Your friend has left.
Blocks: 1240517
Whiteboard: [triage]
Rank: 28
Priority: -- → P2
Whiteboard: [triage]
Sevaan: I believe there's some UX needed for this? Layout/style for the text chat pane?
Flags: needinfo?(sfranks)
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.
Flags: needinfo?(sfranks) → needinfo?(b.pmm)
Here's the design for this. I'll upload a visual spec and the icon shortly.
Flags: needinfo?(b.pmm)
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)
Attached file Assets.zip
Here's the icon.
Flags: needinfo?(b.pmm)
Attached image In-chatNotification.png (obsolete) —
Visual Spec for this in-chat notification.
Attached image link-generator.png (obsolete) —
Attachment #8717455 - Flags: ui-review?(b.pmm)
Attached image link-clicker.png (obsolete) —
Attachment #8717456 - Flags: ui-review?(b.pmm)
Vidhuran can you confirm the space between the icon and the text is 10px, please?
Flags: needinfo?(vidhuran2012)
(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)
(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)
that font displayed...*
(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)
Ok. Here's the final spec. Thanks!
Attachment #8714337 - Attachment is obsolete: true
Attachment #8717455 - Attachment is obsolete: true
Attachment #8717455 - Flags: ui-review?(b.pmm)
Attachment #8717456 - Attachment is obsolete: true
Attachment #8717456 - Flags: ui-review?(b.pmm)
Attached image link-clicker.png
Attachment #8717495 - Flags: ui-review?(b.pmm)
Attached image link-generator.png
Attachment #8717496 - Flags: ui-review?(b.pmm)
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.
Attachment #8717495 - Flags: ui-review?(b.pmm) → ui-review+
Attachment #8717496 - Flags: ui-review?(b.pmm) → ui-review+
Vidhuran: I was in the area so I added a couple of comments on the PR for you.
Attachment #8717452 - Flags: review?(standard8)
(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.
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 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+
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.

Attachment

General

Created:
Updated:
Size: