Display message in chat indicating that a user has left the session

RESOLVED FIXED

Status

Hello (Loop)
Client
P2
normal
Rank:
28
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: sevaan, Assigned: Vidhuran Harichandra Babu)

Tracking

Firefox Tracking Flags

(Not tracked)

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 attachments, 3 obsolete attachments)

(Reporter)

Description

2 years ago
When a user leaves the room, a message should be displayed in chat, along with a timestamp, indicating this event.
(Reporter)

Comment 1

2 years ago
Suggested string: Your friend has left.
(Reporter)

Updated

2 years ago
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.
(Reporter)

Updated

2 years ago
Flags: needinfo?(sfranks) → needinfo?(b.pmm)

Comment 4

2 years ago
Created attachment 8713203 [details]
User has left message.png

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)

Comment 6

2 years ago
Created attachment 8714336 [details]
Assets.zip

Here's the icon.
Flags: needinfo?(b.pmm)

Comment 7

2 years ago
Created attachment 8714337 [details]
In-chatNotification.png

Visual Spec for this in-chat notification.
Created attachment 8717452 [details] [review]
[loop] Vidhuran:textchat-notify-peerdisconnect > mozilla:master
(Assignee)

Comment 9

2 years ago
Created attachment 8717455 [details]
link-generator.png
Attachment #8717455 - Flags: ui-review?(b.pmm)
(Assignee)

Comment 10

2 years ago
Created attachment 8717456 [details]
link-clicker.png
Attachment #8717456 - Flags: ui-review?(b.pmm)

Comment 11

2 years ago
Vidhuran can you confirm the space between the icon and the text is 10px, please?
Flags: needinfo?(vidhuran2012)
(Assignee)

Comment 12

2 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

2 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

2 years ago
that font displayed...*
(Assignee)

Comment 15

2 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

2 years ago
Created attachment 8717470 [details]
In-chatNotification Spec

Ok. Here's the final spec. Thanks!
Attachment #8714337 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Attachment #8717455 - Attachment is obsolete: true
Attachment #8717455 - Flags: ui-review?(b.pmm)
(Assignee)

Updated

2 years ago
Attachment #8717456 - Attachment is obsolete: true
Attachment #8717456 - Flags: ui-review?(b.pmm)
(Assignee)

Comment 17

2 years ago
Created attachment 8717495 [details]
link-clicker.png
Attachment #8717495 - Flags: ui-review?(b.pmm)
(Assignee)

Comment 18

2 years ago
Created attachment 8717496 [details]
link-generator.png
Attachment #8717496 - Flags: ui-review?(b.pmm)
(Assignee)

Comment 19

2 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

2 years ago
Attachment #8717495 - Flags: ui-review?(b.pmm) → ui-review+

Updated

2 years ago
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.
(Assignee)

Updated

2 years ago
Attachment #8717452 - Flags: review?(standard8)
(Assignee)

Comment 21

2 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.
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
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.