[MM-13274] Show a tooltip with the full channel name when hovering over group message channels in the sidebar

[marianunez]

Summary

Added a tooltip to show group message channel full display name at the sidebar.
Included new test for Group Message channel case to include new tooltip in test snapshot.

Ticket Link

Fixes mattermost/mattermost#10787

[jasonblais]

@marianunez Really appreciate the contribution!! This is a great user improvement.

I noticed you had a question earlier about whether this should be for both webapp and desktop - only webapp is needed, so you've taken the right approach there 👍 Sorry for missing the question earlier.

Unfortunately the generated test server above doesn't load so I'm unable to test the changes at the moment - nothing on your end, it's something for us to resolve (@cpanato can you help take a look next week?)

@marianunez Finally, welcome to the Mattermost community! Looking forward to test the changes soon. If you haven't joined our Contributors community channel, would love to see you there 🚀

[cpanato]

@jasonblais fixed

[marianunez]

@marianunez Finally, welcome to the Mattermost community! Looking forward to test the changes soon. If you haven't joined our Contributors community channel, would love to see you there 🚀

Thanks @jasonblais! I'm already enjoying being able to contribute at least this little piece :) I'm @mariale at the community server.

[jasonblais]

Tested the following:

1. iOS 12, iPhone 5s, Safari browser: no tooltip displayed when clicking on channel, as expected
2. Mac, Chrome: tooltip displayed, both on narrow and wide screens, as expected.
3. Mac, Desktop App v4.2.1: tooltip not displayed, not expected

@hmhealey or @deanwhillier would you know why the tooltip would not be displayed for the desktop app? I thought the code for the sidebar would be shared across the web and desktop apps.

PS: I also found a regression, which is not related to this PR. Filed a separate ticket for that: https://mattermost.atlassian.net/browse/MM-15534

[deanwhillier]

@jasonblais, I'll take a look and get back!

[deanwhillier]

Okay so, without getting too much into the weeds, there is a conditional in the webapp code when it is running in the Desktop app that shows a contextual menu when right clicking on a channel in the sidebar to allow for copying a channel's link when using the Desktop app. This contextual menu appears to be stopping propagation of mouse events (didn't verify this explicitly), preventing the hover from working as expected on Desktop.

So we either need to figure out how to allow the hover events through, or perhaps move the tooltip element inside the contextual menu element. I quickly tried the latter locally and it works for me. Unfortunately that means some refactoring of the channel item markup might be needed.

@marianunez, let me know if you'd like to talk through any of this!

[marianunez]

@deanwhillier I can take a look. Thanks!

[marianunez]

Looking into the issue, as @deanwhillier suspected, the context menu is a custom component and is swallowing the event handlers, see: react-bootstrap/react-bootstrap#2208

I applied the workaround suggested there with the extra div and it is now working on both desktop and web app. Let me know your thoughts @deanwhillier @hmhealey

[jasonblais]

Thank you @marianunez! Nice work.

Tested the following:

• iOS 12, iPhone 5s, Safari browser: no tooltip displayed when clicking on channel, as expected
• Mac, Chrome: tooltip displayed, both on narrow and wide screens, as expected.
• Mac, Desktop App v4.2.1: tooltip displayed, both on narrow and wide screens, as expected.

@asaadmahmood I'm requesting your help with review from UX side, particularly when moving the mouse UP on the sidebar with multiple group message channels. I want to ensure the tooltip doesn't feel like it's hindering the mouse movement.

[asaadmahmood]

@jasonblais I just had a look, and it looks fine to me. The tooltip position would normally be centered above, however the position is dynamic, it may change based on where the item is.
So for me, the tooltip dissapears shortly after I move my mouse up, so I think that's sort of expected and fine.

[deanwhillier]

The workaround is definitely working for me – thanks @marianunez! 🎉

I posed a question regarding UX on the Jira ticket, just waiting for an answer before I finish my review. 🙂

[marianunez]

@deanwhillier I had your same concern when doing this change regarding if the other channels also needed the tooltip.

However, when I tried it, the channel creation UI is limiting the channel name so the sidebar shows all characters of the longest permitted length. It seems that in other channels the truncated name case currently would not be possible.

[deanwhillier]

Ya, I saw the limited characters as well, however, when there are new messages in a channel and the name is bolded, it is possible for a normal channel name to truncate. Check out the Developers: Performace channel screenshot I posted in the Jira ticket. 🙂

@asaadmahmood, can you please have a look at my question on the Jira ticket when you have a minute?

[asaadmahmood]

@deanwhillier Had a look, I think any channel that goes into the truncation mode should show the tooltip, though I guess it would be hard to check that? Especially considering the translations as well.

[deanwhillier]

Thanks @asaadmahmood! It does seem possible to check. I know you can get the overall width of the text using element.scrollWidth and the width of the text container using element.offsetWidth or element.clientWidth. 'Simply' checking to see if the element.scrollWidth is greater than element.offsetWidth or element.clientWidth (possibly with a bit of buffer) should be a reliable indicator of truncation being used in this case. I did a quick vanilla JavaScript POC to test latest Chrome, Safari, Firefox, Edge and IE 11 (IE11 seems to need element.clientWidth, while the others can use element.offsetWidth) and they all worked. I didn't do a test in our app using React though.

@marianunez, this is perhaps a bit more complex than what you have already implemented, are you interested in giving it a try?

[deanwhillier]

@marianunez, we'll create a new help-wanted ticket for this additional effort, so this one can be closed with your existing fix for what the ticket actually asked for. If you would like to give the additional functionality a go, by all means feel free to take the new ticket as well! 🙂

Thanks for your contribution on this one! 🎉

[deanwhillier]

One more thing @marianunez, could you please merge master into your branch to update the PR when you have a minute?

[marianunez]

@marianunez, this is perhaps a bit more complex than what you have already implemented, are you interested in giving it a try?

@deanwhillier For sure! Can you mention me on the new ticket so I can take a look?

One more thing @marianunez, could you please merge master into your branch to update the PR when you have a minute?

It should be up to date now. Thanks!