1378823 - Implement tooltips for Matrix participants

Status: RESOLVED FIXED

(Chat Core :: Matrix, enhancement)

Description

[Pavan Karthik [:matrixisreal]]

Add necessary info to tooltip, from USer Obj
http://matrix-org.github.io/matrix-js-sdk/0.7.5/module-models_user-User.html

Comment 1

[Pavan Karthik [:matrixisreal]]

Attachment: tooltip.patch

Turns out, there is much less showable info compared to IRc.

Currently, just 3 items:
Display NAme,
Presence Status Message
Last Activity

Comment 2

[Patrick Cloke [:clokep]]

Comment on attachment 8883973 [details] [diff] [review]
tooltip.patch

Review of attachment 8883973 [details] [diff] [review]:
-----------------------------------------------------------------

Looks like a good start! We should probably also show the level? That seems to be included in riot.im.

::: chat/locales/en-US/matrix.properties
@@ +21,5 @@
> +#    from the "User" object of that user.
> +#    The human readable name of the user.
> +tooltip.displayName= Display name
> +tooltip.presenceStatusMsg= Status message
> +tooltip.lastActiveAgo= Last activity

Please remove the space before all of these.

::: chat/protocols/matrix/matrix.js
@@ +5,5 @@
>  var {classes: Cc, interfaces: Ci, results: Cr, utils: Cu} = Components;
>  
>  Cu.import("resource:///modules/imXPCOMUtils.jsm");
>  Cu.import("resource:///modules/jsProtoHelper.jsm");
> +Cu.import("resource:///modules/imServices.jsm");

Please keep these in alphabetical order!

@@ +248,5 @@
> +      return EmptyEnumerator;
> +
> +    // Convert timespan in milli-seconds into a human-readable form.
> +    let getNormalizedTime = function(aTime) {
> +      let valuesAndUnits = DownloadUtils.convertTimeUnits(aTime/1000);

Nit: spaces on either side of /.

@@ +261,5 @@
> +
> +    if (user.displayName)
> +      tooltipInfo.push(new TooltipInfo(_("tooltip.displayName"), user.displayName));
> +
> +    // We probably need to set this as Away Message?

This needs to be set the status properly: https://dxr.mozilla.org/comm-central/source/chat/protocols/irc/irc.js#1247-1260

Is this actually just a message? Is there a concept of away vs. idle?

Comment 3

[Patrick Cloke [:clokep]]

Attachment: Patch v2

This un-bitrots attachment 8883973 [details] [diff] [review] and updates it to handle a bit more of the logic for tooltips (e.g. to properly handle icons).

I tested this by connecting to Matrix and hovering over some Matrix IDs.

This doesn't work in all situations for all Matrix participants, but it is a good start and I wanted to finish up this partially done work.

Comment 4

[Paul Morris [:pmorris]]

Comment on attachment 9122839 [details] [diff] [review]
Patch v2

Review of attachment 9122839 [details] [diff] [review]:
-----------------------------------------------------------------

The code additions seem fine as a good first step.  I applied the patch and could see the tooltips as expected.  Good to see progress towards full matrix support.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/fdc16e6a8006
Implement tooltips for Matrix. r=pmorris