read_receipts: Disable showing read receipts for Notification Bot messages.

[Riken-Shah]

The implementation is simple, we just check if the message sender is a notification bot to decide if we should show the read receipts list.

Fixes #22905

Screenshots and screen captures:

Self-review checklist

• Self-reviewed the changes for clarity and maintainability
  (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

• Explains differences from previous plans (e.g., issue description).
• Highlights technical choices and bugs encountered.
• Calls out remaining decisions and concerns.
• Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

• Each commit is a coherent idea.
• Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

• Visual appearance of the changes.
• Responsiveness and internationalization.
• Strings and tooltips.
• End-to-end functionality of buttons, interactions and flows.
• Corner cases, error conditions, and easily imagined bugs.

[Riken-Shah]

Instead of only applying the disable logic for the notification bot I have applied the logic for all the bots.

@alya Is this fine?

Also updated suggested text from,

Read receipts are not available for Notification any Bot messages.

[alya]

Hm, could we just apply the logic to Notification Bot? Other bots don't send messages that the app automatically marks as read.

[Riken-Shah]

Hm, could we just apply the logic to Notification Bot? Other bots don't send messages that the app automatically marks as read.

Cool Done!

[juliaBichler01]

The changes to the code look good to me, but one question/suggestion about how the popup window looks:
Is it on purpose that there is a lot of empty space below the text? I think it would look better if this would be a bit less.

[Riken-Shah]

Thanks for the review!

Is it on purpose that there is a lot of empty space below the text? I think it would look better if this would be a bit less.

We show the simar space if there are no read receipts.

I also agree it'd look better if we remove extra space. @alya What do you think?

[timabbott]

The design challenge with the popover window is that we show it while the loading indicator is pinning, with a certain height, and then its height potentially changes when the data contents are added; it feels not great. So I think we have a minimum height set in the CSS for what will look good if there's at least one user shown, for example, read receipts for a private message.

One possible solution would be to just not have the modal have a loading indicator -- just only open it once we've received a response from the server (if we're making one); that way, you never see the thing's size change shortly after it opens.

The downside of that approach would be that you get no feedback that your click was processed properly..

[Riken-Shah]

We don't make any API call for notification bot messages, so for this case, we can remove the minimum height rule.

[Riken-Shah]

We don't make any API call for notification bot messages, so for this case, we can remove the minimum height rule.

@alya Should I implement this?

[alya]

We don't make any API call for notification bot messages, so for this case, we can remove the minimum height rule.

@alya Should I implement this?

Yes, that sounds good to me!

[Riken-Shah]

@alya & @timabbott This is ready for another round of review!

[alya]

Hmm, I think we went too far with reducing the height required -- the modal looks cut off on the bottom.

Can we try leaving as much space on the bottom as we do on the top?

[alya]

I didn't see any other issues in manual testing.

[Riken-Shah]

Can we try leaving as much space on the bottom as we do on the top?

On top we have 10px margin, at bottom we had 8px padding I have updated it to 10px.
Let me know if it looks alright or if I need to add more spacing.

P.S I have updated the screenshot on the top comment.

[alya]

Hmm, it seems like there must be additional margin coming from somewhere -- I'm still seeing more space on top than on the bottom.

[alya]

@timabbott do you want to take a look?

[Riken-Shah]

I'm still seeing more space on top than on the bottom.

Hmm, maybe I can add one more px to compensate for hr border, but not sure that will create any significant difference.

[timabbott]

I think Alya had a different existing padding as what to match -- there's 16px of padding above the heading, and 10px below; so we should just do the 16px, not 10px. It lgtm changing from 10px to 16px:

Merged with that tweak, thanks @Riken-Shah!

[timabbott]

Merged, with a bonus commit to move the read receipts CSS next to the related modal CSS.