-
Notifications
You must be signed in to change notification settings - Fork 358
Read Receipts: Allow Timelines to be configured to hide read receipts on state events. #5900
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
3470137 to
2ce1262
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5900 +/- ##
==========================================
- Coverage 88.66% 88.66% -0.01%
==========================================
Files 363 363
Lines 104580 104628 +48
Branches 104580 104628 +48
==========================================
+ Hits 92730 92771 +41
- Misses 7501 7508 +7
Partials 4349 4349 ☔ View full report in Codecov by Sentry. |
CodSpeed Performance ReportMerging #5900 will not alter performanceComparing Summary
|
fb8002e to
2266d0d
Compare
bd3b58b to
4b903d0
Compare
Hywan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clean code, nice feature, great test, good documentation, what can I ask more? A tiny feedback to remove a PartialEq implementation, but otherwise it's really great, kudos 👏 and thanks for the great contribution!
| AnySyncTimelineEvent::State(_) => settings.state_events_can_show_read_receipts, | ||
| AnySyncTimelineEvent::MessageLike(_) => true, | ||
| AnySyncTimelineEvent::State(_) => { | ||
| settings.track_read_receipts == TimelineReadReceiptTracking::AllEvents |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably better to use matches!(settings.track_read_receipts, TimelineReadReceiptTracking::AllEvents) here, so that you can remove the PartialEq implementation on this enum. Less code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4b903d0 to
fdc7479
Compare
…ts on state events. # Conflicts: # bindings/matrix-sdk-ffi/CHANGELOG.md
fdc7479 to
d0eb5d6
Compare
|
Force pushed to fix a conflict on the changelog. |
This is an mvp attempt at #3065 to fix element-hq/element-x-ios#2338.
The PR adds a new
TimelineReadReceiptTrackingenum which is used in place of thetrack_read_receiptsboolean to decide whether to enable tracking for all events, just for message-like events or to disable tracking.When tracking is configured for message-like only, any read receipts on state events are bundled up into the previous event, using the same logic that receipts from filtered events are handled with.