Skip to content

feat(sdk,ui): Unset the unread flag when sending unthreaded receipts#5055

Merged
Hywan merged 12 commits intomatrix-org:mainfrom
zecakeh:auto-unset-unread
May 21, 2025
Merged

feat(sdk,ui): Unset the unread flag when sending unthreaded receipts#5055
Hywan merged 12 commits intomatrix-org:mainfrom
zecakeh:auto-unset-unread

Conversation

@zecakeh
Copy link
Copy Markdown
Collaborator

@zecakeh zecakeh commented May 19, 2025

This allows to follow these recommendations from the spec automatically:

Clients SHOULD reset the unread marker by setting unread to false when opening a room to display its timeline.

Clients that offer functionality to mark a room as read by sending a read receipt for the last event, SHOULD reset the unread marker simultaneously.

Note that there was a function in the bindings doing a similar thing that was removed in #3119, but it doesn't seem that it was necessary to fix the bug that it was trying to fix.

@zecakeh zecakeh requested a review from a team as a code owner May 19, 2025 10:10
@zecakeh zecakeh requested review from Hywan and removed request for a team May 19, 2025 10:10
zecakeh added a commit to zecakeh/matrix-rust-sdk that referenced this pull request May 19, 2025
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
@codecov
Copy link
Copy Markdown

codecov Bot commented May 19, 2025

Codecov Report

Attention: Patch coverage is 96.77419% with 1 line in your changes missing coverage. Please review.

Project coverage is 85.84%. Comparing base (4d2c261) to head (90c6c8b).
Report is 34 commits behind head on main.

Files with missing lines Patch % Lines
crates/matrix-sdk-ui/src/timeline/mod.rs 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5055      +/-   ##
==========================================
+ Coverage   85.83%   85.84%   +0.01%     
==========================================
  Files         325      325              
  Lines       35924    35954      +30     
==========================================
+ Hits        30836    30866      +30     
  Misses       5088     5088              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@Hywan Hywan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Apart from the minor feedback, we are good!

Comment on lines +609 to +615
if !receipts.is_empty() {
self.room().send_multiple_receipts(receipts).await?;
} else if self.room().is_marked_unread() {
self.room().set_unread_flag(false).await?;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add let room = self.room(); to avoid calling self.room() multiple time please?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment on lines +635 to +637
if self.room().is_marked_unread() {
// Unset the read marker.
self.room().set_unread_flag(false).await?;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add let room = self.room(); to avoid calling self.room() multiple time please?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the removal of the condition, there is only one call here now.

Comment on lines +611 to +612
} else if self.room().is_marked_unread() {
self.room().set_unread_flag(false).await?;
Copy link
Copy Markdown
Contributor

@bnjbvr bnjbvr May 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of specializing each call site of Room::set_unread_flag(), consider making Room::set_unread_flag a no-op (aka return early without sending the request) if !self.is_marked_unread()?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, done.

@zecakeh zecakeh requested a review from Hywan May 20, 2025 07:35
zecakeh added 11 commits May 20, 2025 09:35
…Room

Updates the unread flag or the room in `Room::send_single_receipt()` and
`Room::send_multiple_receipts()` if the room is marked as unread and the
receipts are unthreaded.

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
…imeline

Updates the unread flag or the room in `Timeline::send_single_receipt()`
and `Timeline::send_multiple_receipts()` if the room is marked as unread
and the receipts are unthreaded.

Updates it also in `Timeline::mark_as_read()`, even if there is no
latest event ID.

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
…g already has the wanted value

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
…ry time

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
@zecakeh zecakeh force-pushed the auto-unset-unread branch from 6ce071d to 380f78d Compare May 20, 2025 07:39
@zecakeh
Copy link
Copy Markdown
Collaborator Author

zecakeh commented May 20, 2025

Rebased on main to solve conflicts.

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Copy link
Copy Markdown
Member

@Hywan Hywan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks!

@Hywan Hywan merged commit 9f196be into matrix-org:main May 21, 2025
42 checks passed
Hywan pushed a commit that referenced this pull request May 21, 2025
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
@zecakeh zecakeh deleted the auto-unset-unread branch May 21, 2025 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants