-
Notifications
You must be signed in to change notification settings - Fork 106
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
Read Receipts sheet + enabled RR by default #2123
Conversation
Generated by 🚫 Danger Swift against 23463b6 |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #2123 +/- ##
===========================================
- Coverage 71.91% 71.91% -0.01%
===========================================
Files 509 511 +2
Lines 34597 34716 +119
Branches 16640 16671 +31
===========================================
+ Hits 24882 24965 +83
- Misses 9099 9137 +38
+ Partials 616 614 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
LGTM.
Please can we fix the grouping too before enabling the feature flag though?
ElementX/Sources/Screens/RoomScreen/View/ReadReceipts/ReadReceiptCell.swift
Outdated
Show resolved
Hide resolved
ElementX/Sources/Screens/RoomScreen/View/ReadReceipts/ReadReceiptsSummaryView.swift
Show resolved
Hide resolved
ElementX/Sources/Screens/RoomScreen/View/Supplementary/TimelineReadReceiptsView.swift
Show resolved
Hide resolved
@amshakal Are we okay in removing the grouping when a message has RRs? EDIT: |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Added a sheet that displays read receipts of that message in more detal when they are tapped
fixes #1053
Also enabled the FF for Read Receipts as true by default. If no problems are detected anymore we can safely then remove this FF once and for all.