Add tooltips to all @mentions

RESOLVED FIXED in Instantbird 45

Status

Chat Core
Twitter
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: aleth, Assigned: aleth)

Tracking

trunk
Instantbird 45
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

2 years ago
Currently @mentions of users who are not participants (i.e. just referenced in a tweet) don't get tooltips. This is inconsistent and feels broken.
(Assignee)

Comment 1

2 years ago
Created attachment 8678513 [details] [diff] [review]
twitteraddmoretooltips.diff
Attachment #8678513 - Flags: review?(clokep)
(Assignee)

Updated

2 years ago
Depends on: 1213908
(Assignee)

Comment 2

2 years ago
Created attachment 8678599 [details] [diff] [review]
twitteraddmoretooltips.diff v2

Typo fix.
Assignee: nobody → aleth
Attachment #8678513 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8678513 - Flags: review?(clokep)
Attachment #8678599 - Flags: review?(clokep)
Comment on attachment 8678599 [details] [diff] [review]
twitteraddmoretooltips.diff v2

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

::: chat/protocols/twitter/twitter.js
@@ +308,5 @@
>          entArray = entArray.concat(entities.user_mentions.map(um => ({
>            start: um.indices[0],
>            end: um.indices[1],
>            str: "@" + um.screen_name,
> +          text: '@<span class="ib-person">' + um.screen_name + "</span>",

Can we reuse ib-nick here? If we're adding new ones, they should likely be im, not ib.
Attachment #8678599 - Flags: feedback+
(Assignee)

Comment 4

2 years ago
(In reply to Patrick Cloke [:clokep] from comment #3)
> > +          text: '@<span class="ib-person">' + um.screen_name + "</span>",
> 
> Can we reuse ib-nick here? If we're adding new ones, they should likely be
> im, not ib.

We can't really reuse ib-nick (added by conversation.xml for participants) as that gets styled with a little bubble and the participant nick colour. I don't think that makes sense here. 

I'm OK with im though.
Comment on attachment 8678599 [details] [diff] [review]
twitteraddmoretooltips.diff v2

Seems sane. I'd like Florian to take a quick look.
Attachment #8678599 - Flags: review?(florian)
Attachment #8678599 - Flags: review?(clokep)
Attachment #8678599 - Flags: review+
Attachment #8678599 - Flags: review?(florian) → review+
(Assignee)

Comment 6

2 years ago
https://hg.mozilla.org/comm-central/rev/ced21c95ce612c7f093de3418afe2a97a301c8ec
Bug 1218127 - Add tooltips to all twitter @mentions. r=clokep,florian a=aleth chat/ patch on CLOSED TREE
(Assignee)

Comment 7

2 years ago
(In reply to aleth [:aleth] from comment #4)
> I'm OK with im though.

On second thoughts, I decided to stick with ib for consistency with ib-sender (which already exists and is in use by TB too), as if two of those class names are ib- and one is im- it's just going to be confusing and lead to typos.
(Assignee)

Updated

2 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Instantbird 45
(Assignee)

Updated

2 years ago
Depends on: 1226559
You need to log in before you can comment on or make changes to this bug.