recent topics: Display other sender names in tooltip. #18502
Conversation
static/js/recent_topics.js
Outdated
| if (muting.is_user_muted(id)) { | ||
| other_senders_fullname_string = other_senders_fullname_string.concat("Muted user, "); | ||
| } else { | ||
| const fullname = people.get_full_name(id); |
There was a problem hiding this comment.
I think we can use people.get_display_full_names here and avoid some duplication.
There was a problem hiding this comment.
Yeah, we should definitely do that.
There was a problem hiding this comment.
Made these changes.
|
I think the positioning issue is that the tooltip is not allowed to go outside the |
Ohh, that could be possible. |
|
Would it makes sense to do this tooltip with one user per line, rather than joining the names with |
I posted on chat.zulip.org regarding this and also about first row showing bottom(if content tries to go out of table). |
|
I've made following changes so that extra senders tooltip will show, only first 10 recent extra senders with 11th line saying |
static/js/recent_topics.js
Outdated
| if (extra_sender_ids.length > MAX_EXTRA_SENDERS) { | ||
| const extra_sender_count = extra_sender_ids.length - MAX_EXTRA_SENDERS; | ||
| other_senders_fullname_string = other_senders_fullname_string.concat( | ||
| $t({defaultMessage: `<br/>and {extra_sender_count} others.`}, {extra_sender_count}), |
There was a problem hiding this comment.
The <br/> should be kept outside of the translateable string, and ideally just added via the join operation you have above.
With a bit of refactoring to collect a list (and have this block just append an extra item) and then join with <br /> items at the end, this'll read a lot more cleanly.
|
Updated this pr by refactoring how this list is collected, and then finally apply join with |
It is a follow up for #18451.
|
Merged, thanks @m-e-l-u-h-a-n! I think it'd be well worth extending the frontend tests for this module to actually cover the cases added by the new logic. |
It is a follow up for #18451 and #18480.
Testing plan: Manually on dev server.
As can be seen in the gif, when tooltip is shown with max allowed width(300) and is on the first row, it is not shown in the top. While it's position is as expected(top), when it is not in the first row. On searching more about this on tiipyjs repo issues I found it is a popper.js problem. I'm not sure how to debug that, so I am trying to find a fix about that.
Other than that this work is ready for review.
GIFs or screenshots:
