-
Notifications
You must be signed in to change notification settings - Fork 172
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
Add ability to swipe between media when opened from the timeline. #4205
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 #4205 +/- ##
========================================
Coverage 80.18% 80.19%
========================================
Files 2023 2026 +3
Lines 53280 53378 +98
Branches 6451 6469 +18
========================================
+ Hits 42723 42804 +81
- Misses 8339 8346 +7
- Partials 2218 2228 +10 ☔ 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.
Thanks, good job!
See some of my remarks, otherwise LGTM.
We'll probably iterate later to improve with event cache!
interface MediaTimeline { | ||
suspend fun getTimeline(): Result<Timeline> | ||
val cache: GroupedMediaItems? | ||
fun orCache(data: GroupedMediaItems): GroupedMediaItems |
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'm not sure to understand this method
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 maybe not needed but it's here to protect a Timeline that would not return the expected Event.
Originally I though that mediaTimeline
would return faster, with an empty list of Event, but it seems that it actually return only when the expected Event has been retrieved. This is why there is this code now: groupedMediaItemsFlow.emit(AsyncData.Success(cache))
at the beginning of method TimelineMediaGalleryDataSource.start()
.
So the method orCache
is probably not needed, but I prefer to keep it as a safety measure.
.../impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaTimeline.kt
Outdated
Show resolved
Hide resolved
.../impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaTimeline.kt
Outdated
Show resolved
Hide resolved
- io.element.android.libraries.mediaviewer.impl.datasource - package io.element.android.libraries.mediaviewer.impl.model
Quality Gate passedIssues Measures |
Content
Add ability to swipe between media when open from the timeline. Previously it was only possible to swipe when the media was opened from the media gallery.
Motivation and context
Closes #4174
Closes #1484
Screenshots / GIFs
MediaViewerSwipe.mp4
Tests
Tested devices
Checklist