Format mentions (without avatars) in the message list #805

danxuliu · 2018-04-23T15:14:55Z

This pull request adds formatted mentions to the message list (but not formatted mentions nor auto-completion in messages being composed); it is based on #517, although in this case avatars are no longer included in the mention (as decided in #517).

It is based on both the mentions shown in the Comments app and the rich formatting of notifications. The backend uses the Comments API to store the messages, so when receiveMessages is called the Comments API is used to extract the mentions from the plain text message originally sent. However, unlike the Comments app, the mentions are not returned to the client in its own attribute, but in a generic way using a rich object string. Thanks to this approach it should be easier to extend the formatting in the future with support for other definition types.

In any case, as the mentions are got using the Comments API this system has its same limitations, like not supporting mentions to users with a space in the name.

Note that links are still formatted in the client using OCP.Comments.plainToRich, which formats the plain text links found in the message; this has not been modified, although for consistency it would be good to move the links too to the rich object string system (something for another pull request, though).

Before:

After (outdated; now the display name is shown instead of the user name):

Formatted mentions are just span elements with a font-weight: bold style; they include the user name, but not the display name (it is the same as in GitHub, for example). Should the display name be shown too? Note that showing it in a tooltip may not be a good option, as (pending, to be done in another pull request) the contacts menu will be shown when clicking on the mention (like done in the Comments app), and the tooltip and the menu would look bad at the same time. they include @ followed by the display name.

The RichMessageHelper replaces the enrichable references in a plain text message with their equivalent rich object string parameter; currently only mentions are taken into account. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

nickvergessen · 2018-04-23T15:46:13Z

js/richobjectstringparser.js

+
+	OCA.SpreedMe.RichObjectStringParser = {
+
+		_userLocalTemplate: '<span class="mention-user">@{{id}}</span>',


We should use name here, so we display the user displayname, not the uid

I think it is better to show @nickvergesen than @joas Schilling or Joas Schilling, as the @ would make easier to spot mentions (I guess that in the future there could be other bold texts), and the @ prepended to the display name feels strange. Not a strong opinion on this, though; @nextcloud/designers ?

But @some sha for ldap users is not usable...

It rather is @597ae2f6-16a6-1027-98f4-d28b5365dc14 😉

Since names are bold, I think it should be fine with an @.
I checked WhatsApp and Threema are showing you the "real"name, Telegram shows the id.
However just see the LDAP thing above and I guess everyone knows we shouldn't use IDs ;)

Since names are bold, I think it should be fine with an @.

Maybe instead of just using bold text we could use a coloured background like in Riot. But, for the time being, I think that just bold is fine ;-)

However just see the LDAP thing above and I guess everyone knows we shouldn't use IDs ;)

Agreed ;-) I will change it to use the real name.

danxuliu · 2018-04-23T15:52:14Z

Oops, I will fix the tests.

nickvergessen · 2018-04-24T09:54:45Z

One final thing:

spreed/js/views/chatview.js

Lines 378 to 384 in 8821165

    
           $el.find('.avatar').each(function() { 
        
           	var avatar = $(this); 
        
           	var strong = $(this).next(); 
        
           	var appendTo = $(this).parent(); 
        
           	$.merge(avatar, strong).contactsMenu(avatar.data('user'), 0, appendTo); 
        
           });

This is not working anymore. I also failed to quickly move it over to the new code. Can you have a look? (It's the user menu which should be shown when you click on a mention)

danxuliu · 2018-04-24T10:04:14Z

There are several issues with the contacts menu unrelated to the mentions themselves, so I intended to fix them and make the contacts menu appear when clicking on a mention in a different pull request.

nickvergessen · 2018-04-24T10:32:23Z

lib/Controller/ChatController.php

 			return [
 				'id' => $comment->getId(),
 				'token' => $token,
 				'actorType' => $comment->getActorType(),
 				'actorId' => $comment->getActorId(),
 				'actorDisplayName' => $displayName,
 				'timestamp' => $comment->getCreationDateTime()->getTimestamp(),
-				'message' => $comment->getMessage()
+				'message' => $message,
+				'messageParameters' => $messageParameters,


Should adjust the docs/api-v1.md

Do you prefer me to explicitly document all the possible parameters or just point to the RichMessageHelper.php file?

No, just that there is messageParameters
See rich subject of activity: https://github.com/nextcloud/activity/blob/master/docs/endpoint-v2.md#activity-element
Maybe you can also link to nextcloud/server#1706

Done; I shamelessly ripped it from the Notifications app

jancborchardt · 2018-04-24T11:19:29Z

For now this seems good. We need to have a broader discussion regarding showing usernames vs real names (or both). Usually we show real names (if present) everywhere in sharing, and as name next to the avatar.

MorrisJobke · 2018-04-24T12:18:02Z

For now this seems good. We need to have a broader discussion regarding showing usernames vs real names (or both). Usually we show real names (if present) everywhere in sharing, and as name next to the avatar.

I don't think there is much to discuss here. Username is purely internally. All the user cares about is the login name (could be username, email or any other attribute in case of LDAP) and the display name. The username (our internal representation of a user) could be highly artificial - like @597ae2f6-16a6-1027-98f4-d28b5365dc14 which is the default for every LDAP setup.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

The RichObjectStringParser was copied from the Notifications app and adapted to be used in the chat (support for file references was removed, "-" is taken into account too in parameter IDs, only local users are taken into account, and if the display name of a mention is empty the user ID is used instead). Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Signed-off-by: Joas Schilling <coding@schilljs.com>

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

In some cases (like when LDAP is being used) a user can not be easily identified through its user name (as it is just a hash), so formatted mentions now show the display name in all cases (and if the server does not provide a display name the "name" parameter of the template falls back to the user name). Note that the acceptance tests do not need to be updated, as the display name of the mentioned user is the same as its user name. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

The contacts menu uses both the "bubble" and "contactsmenu-popover" CSS classes, and the specific rules for ".bubble" elements used in Talk should not affect the contacts menu. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

The border-box sizing is set for apps in the server, but the CSS rules for the icons of the contacts menu (also set in the server!) assume that content-box sizing is used. Therefore the proper size should be forced for the icons to be visible. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

jancborchardt · 2018-04-24T20:46:41Z

@MorrisJobke yeah, that was kinda my point. So we should not use the username here, but the real name, right? (However it needs to fall back to username if no real name is set.)

nickvergessen · 2018-04-24T21:24:02Z

Yeah it's fixed now, the fallback is done by the server, the display name automatically returns the uid when no name is set

The contacts menu expects its parent element to be its positioning context, so the mention must have a relative position. Besides that, the default CSS rules from the server set the position of the contacts menu assuming that the parent element is an avatar, so the rules have to be overriden to position it on the text. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

A pointer cursor is now used on mentions to make the user aware of a possible interaction. As the contacts menu is not shown for mentions of the current user the default cursor is kept in that case. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

danxuliu · 2018-04-25T05:41:41Z

I have fixed the failing acceptance tests and also added rich-strings to the capabilities (as I supposed that it had to be added; if not just remove the last commit ;-) ).

Backport is ready in https://github.com/nextcloud/spreed/tree/stable13-805-format-mentions-without-avatars-in-the-message-list (no pull request until this one is merged, though).

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

nickvergessen

Works fine

danxuliu added 3. to review design enhancement feature: chat 💬 Chat and system messages labels Apr 23, 2018

danxuliu added this to the 4.0 (Nextcloud 14) milestone Apr 23, 2018

danxuliu requested review from nickvergessen and Ivansss April 23, 2018 15:14

danxuliu mentioned this pull request Apr 23, 2018

Format mentions in the message list #517

Closed

nickvergessen reviewed Apr 23, 2018

View reviewed changes

danxuliu force-pushed the format-mentions-without-avatars-in-the-message-list branch from c6853e6 to 9d07c73 Compare April 23, 2018 16:34

nickvergessen reviewed Apr 24, 2018

View reviewed changes

nickvergessen added the backport-request label Apr 24, 2018

danxuliu and others added 8 commits April 24, 2018 17:47

Return rich messages when calling "receiveMessages"

1652466

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Add integration tests for receiving rich messages

b526c14

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Copy fix for undefined placeholders from activity app

80d955a

Signed-off-by: Joas Schilling <coding@schilljs.com>

Add acceptance tests for formatted messages

dd13cf9

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Fix position of right end of contacts menu

a52f7cb

The contacts menu uses both the "bubble" and "contactsmenu-popover" CSS classes, and the specific rules for ".bubble" elements used in Talk should not affect the contacts menu. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

danxuliu force-pushed the format-mentions-without-avatars-in-the-message-list branch from 7feedda to 47ed718 Compare April 24, 2018 15:47

danxuliu added 2 commits April 25, 2018 07:06

Do not show the contacts menu for mentions of the current user

7dc7274

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

danxuliu force-pushed the format-mentions-without-avatars-in-the-message-list branch from 47ed718 to 1811440 Compare April 25, 2018 05:17

Adjust capability docs

07d6e7b

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

nickvergessen force-pushed the format-mentions-without-avatars-in-the-message-list branch from 1811440 to 07d6e7b Compare April 25, 2018 08:21

nickvergessen approved these changes Apr 25, 2018

View reviewed changes

nickvergessen merged commit f24177e into master Apr 25, 2018

nickvergessen deleted the format-mentions-without-avatars-in-the-message-list branch April 25, 2018 08:53

danxuliu mentioned this pull request Apr 25, 2018

[stable13] Format mentions without avatars in the message list #810

Merged

danxuliu removed the backport-request label Apr 25, 2018

nickvergessen mentioned this pull request May 2, 2018

Update changelog with current patches #739

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Format mentions (without avatars) in the message list #805

Format mentions (without avatars) in the message list #805

danxuliu commented Apr 23, 2018 •

edited

Loading

nickvergessen Apr 23, 2018

danxuliu Apr 23, 2018

nickvergessen Apr 23, 2018

MorrisJobke Apr 23, 2018

nickvergessen Apr 24, 2018

danxuliu Apr 24, 2018

danxuliu commented Apr 23, 2018

nickvergessen commented Apr 24, 2018

danxuliu commented Apr 24, 2018

nickvergessen Apr 24, 2018

danxuliu Apr 24, 2018

nickvergessen Apr 24, 2018

danxuliu Apr 24, 2018

jancborchardt commented Apr 24, 2018

MorrisJobke commented Apr 24, 2018

jancborchardt commented Apr 24, 2018

nickvergessen commented Apr 24, 2018

danxuliu commented Apr 25, 2018

nickvergessen left a comment


		OCA.SpreedMe.RichObjectStringParser = {

		_userLocalTemplate: '<span class="mention-user">@{{id}}</span>',

Format mentions (without avatars) in the message list #805

Format mentions (without avatars) in the message list #805

Conversation

danxuliu commented Apr 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danxuliu commented Apr 23, 2018

nickvergessen commented Apr 24, 2018

danxuliu commented Apr 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jancborchardt commented Apr 24, 2018

MorrisJobke commented Apr 24, 2018

jancborchardt commented Apr 24, 2018

nickvergessen commented Apr 24, 2018

danxuliu commented Apr 25, 2018

nickvergessen left a comment

Choose a reason for hiding this comment

danxuliu commented Apr 23, 2018 •

edited

Loading