Natural media viewer swiping order

[bxdxnn]

Content

Change the swiping order in the media viewer and update tests:

• Swipe from right to left goes to a newer media
• Swipe from left ro right goes to an older media

Motivation and context

Fixes #6350
Fixes #4296

Right now, the swiping order in the media viewer is wrong:

• Swiping from right to left goes to an older media
• Swiping from left to right goes to a newer media

The new bevahior in this PR is more natural and a standard behavior in most of the apps.

Screenshots / GIFs

Tests

• Swipe from right to left goes to a newer media
• Swipe from left ro right goes to an older media

Tested devices

• Physical
• Emulator
• OS version(s):

Checklist

• Changes have been tested on an Android device or Android emulator with API 24
• UI change has been tested on both light and dark themes
• Accessibility has been taken into account. See https://github.com/element-hq/element-x-android/blob/develop/CONTRIBUTING.md#accessibility
• Pull request is based on the develop branch
• Pull request title will be used in the release note, it clearly define what will change for the user
• Pull request includes screenshots or videos if containing UI changes
• You've made a self review of your PR

[github-actions]

Thank you for your contribution! Here are a few things to check in the PR to ensure it's reviewed as quickly as possible:

• Your branch should be based on origin/develop, at least when it was created.
• The title of the PR will be used for release notes, so it needs to describe the change visible to the user.
• The test pass locally running ./gradlew test.
• The code quality check suite pass locally running ./gradlew runQualityChecks.
• If you modified anything related to the UI, including previews, you'll have to run the Record screenshots GH action in your forked repo: that will generate compatible new screenshots. However, given Github Actions limitations, it will prevent the CI from running temporarily, until you upload a new commit after that one. To do so, just pull the latest changes and push an empty commit.

[jmartinesp]

I think this addresses #6350 or #4296, right? I'm not sure if reversing the order is the right call when opening the media viewer from both the timeline and the gallery though.

@bmarty I see you assigned to one of these issues, WDYT?

[bxdxnn]

I think this addresses #6350 or #4296, right? I'm not sure if reversing the order is the right call when opening the media viewer from both the timeline and the gallery though.

@bmarty I see you assigned to one of these issues, WDYT?

Correct, this PR fixes these issues. Just added.

[jmartinesp]

I wanted some feedback from @bmarty since it seems like he planned to work on a related task, but maybe I should ask @mxandreas or @americanrefugee instead: should we just reverse the order of items in the media viewer, or maybe change it conditionally depending on where this screen is started from?:

• From selecting a media item in the timeline: items to the left are older, items to the right are newer.
• From opening some media in the gallery of a room: items to the left are newer, items to the right are older.

---

Looking at other clients:

• Whatsapp keeps a consistent order: left is older, right is newer. It's what's done in this PR.
• Telegram on the other hand does what I proposed above. This makes more sense to me, but it's a subjective preference.

[americanrefugee]

This is a tricky one...

I agree that when viewing media from the timeline that right=newer / left=older makes total sense for both platforms.

The sticky bit is when viewing the Media Browser on iOS vs. Android.

• On iOS, the gallery shows the most recent at the bottom of the list. Therefore, right=newer / left=older still makes sense
• But on Android, the gallery list items are reversed - the most recent is at the top of the list. Therefore, right=older / left=newer would be more natural

So my personal opinion is:

• Both platforms have a common swipe before for viewing media from the timeline (right=news / left=older)
• When viewing from the Media Browser, the swipe pattern depends on whether most recent items are at the bottom of the list (iOS) vs. the top (Android)

Would love to get thoughts from @mxandreas

[americanrefugee]

To clarify, when I say for example "right=newer / left=older" I mean...

If viewing an image, the image to the right is newer and the image to the left is older. To see the image to the right you swipe left. To see the image to the left you swipe right.

Hopefully that makes sense 😅

[mxandreas]

When viewing from the Media Browser, the swipe pattern depends on whether most recent items are at the bottom of the list (iOS) vs. the top (Android)

Why is the ordering different in Android gallery? If we changed that order we'd have straightforward solution exactly like we currently have on iOS.

[americanrefugee]

Why is the ordering different in Android gallery? If we changed that order we'd have straightforward solution exactly like we currently have on iOS.

Because that's the native pattern on Android in general. To make them match would be a refactoring of the media gallery on Android. I'm not opposed to that, but I'm also not sure what the reaction would be from Android users.

In general we try and follow native patterns when possible.

[mxandreas]

Because that's the native pattern on Android in general.

Ok, if we do not want to change this, then I have no better ideas than what we discussed:

1. Opening from timeline should certainly follow the ascending order, e.g. older images on the left. It would be very weird for this not to be the case.
2. Opening from the gallery - it could follow the gallery order (e.g. older on the right), but I have a slight preference of doing the same as for timeline. Because once the user has the image open they probably forget about the previous context and if sometimes swiping on the same direction gives you newer and sometimes older image, then I think this may be more confusing.

[americanrefugee]

FWIW I agree with you @mxandreas

[jmartinesp]

So, just to confirm: are we ok with the behaviour change in this PR, reversing the order in both cases?

[jmartinesp]

Ok, this implementation was verified by product and design and works fine. Let me merge this into a branch of mine for some minor tweaks (I'd upload it to your forked repo, but I can't 🫤, maybe you need to enable this) and then it can be merged.

[jmartinesp]

Actually, a rebase that fixes the failing screenshot tests would probably be good enough to get it merged. Let's see if it works automatically.

[bxdxnn]

Ok, this implementation was verified by product and design and works fine. Let me merge this into a branch of mine for some minor tweaks (I'd upload it to your forked repo, but I can't 🫤, maybe you need to enable this) and then it can be merged.

I have this enabled.
Another maintainer couldn't upload his changes too, but he solved it by disabling Git LFS push.

[codecov]

Codecov Report

❌ Patch coverage is 80.00000% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.13%. Comparing base (5b3f91f) to head (250e000).
⚠️ Report is 5 commits behind head on develop.

Files with missing lines	Patch %	Lines
...es/mediaviewer/impl/viewer/MediaViewerPresenter.kt	66.66%	0 Missing and 4 partials ⚠️
...s/mediaviewer/impl/viewer/MediaViewerDataSource.kt	92.30%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #6431      +/-   ##
===========================================
- Coverage    81.14%   81.13%   -0.01%     
===========================================
  Files         2623     2623              
  Lines        72806    72821      +15     
  Branches      9418     9428      +10     
===========================================
+ Hits         59079    59086       +7     
- Misses       10254    10260       +6     
- Partials      3473     3475       +2     

☔ 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.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.