-
Notifications
You must be signed in to change notification settings - Fork 160
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
Feature/fga/push subscribe to room #3257
Conversation
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3257 +/- ##
===========================================
- Coverage 76.13% 76.11% -0.02%
===========================================
Files 1647 1647
Lines 38787 38801 +14
Branches 7530 7533 +3
===========================================
+ Hits 29529 29534 +5
- Misses 5347 5355 +8
- Partials 3911 3912 +1 ☔ View full report in Codecov by Sentry. |
} | ||
val client = matrixClientProvider.getOrRestore(notifiableEvent.sessionId).getOrNull() ?: return@launch | ||
client.getRoom(notifiableEvent.roomId)?.use { room -> | ||
room.subscribeToSync() |
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.
I think that might be instantiating a timeline too?
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.
Yes the goal is also to create and cache a timeline before clicking the notification
After enabling the developer option I see no |
I don't know if it's quick enough to wait for the app to be in foreground. Do you see improvements on your side? |
Actually my bad, I thought this PR was about opportunistic syncs when push notifications were received, so we could get updates for other subscribed rooms too, but it's only about optimising loading times for the timeline, right? To be honest, I see no difference with or without the developer option in the debug build. Maybe it'll be more noticeable in the nightlies. I'll approve it so we can test it there. |
Ok, lets see and remove it if it's useless |
Content
Add a feature flag so we can try subscribing to room whenever we receive a notifiable push.
Motivation and context
To improve time to see timeline when opening the app from push notification.
Screenshots / GIFs
Tests
Tested devices
Checklist