Add developer setting to hide images in the timeline #3592

bmarty · 2024-10-03T09:52:45Z

Content

Add a switch to the developer settings screen to prevent preview of images, videos and sticker to be rendered automatically in the timeline.
When enabled:

the blurhash (if available) will be rendered by default, or a color surface will be rendered.
A button exist to render the image
The image is not clickable, it must be rendered first
The thumbnail (for replyied image) is not rendered, expect if the original image has been rendered.
The image does not render in the notification

Motivation and context

Closes #3386

Screenshots / GIFs

Waiting for the final design before recording the screenshot.

Tests

Navigate to the developer setting to enable the new option
Go to a room and observe that the images are not rendered automatically

Tested devices

Physical
Emulator
OS version(s):

Checklist

Changes have been tested on an Android device or Android emulator with API 23
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
Pull request includes a sign off
You've made a self review of your PR

github-actions · 2024-10-03T10:11:51Z

📱 Scan the QR code below to install the build (arm64 only) for this PR.

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/qVBAbQ

jmartinesp

LGTM once Konsist tests pass, I added a couple of suggestions too.

By the way, this feature seems more like something that would fit in the advanced settings since some users may want this as the default instead. I saw it was decided to add it in developer settings instead, but maybe we'll iterate on that in the future?

jmartinesp · 2024-10-04T06:16:40Z

...kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt

                timelineController.invokeOnCurrentTimeline {
                    val replyToDetails = loadReplyDetails(draftType.eventId).map(permalinkParser)
-                    run { messageComposerContext.composerMode = MessageComposerMode.Reply(replyToDetails) }
+                    run {


Is this run needed?

Was needed, but not anymore with 6ee0dd32c32f49f1f3fcbc3277efbc01f6fc9d4f.

jmartinesp · 2024-10-04T06:19:45Z

...ent/android/features/messages/impl/timeline/components/event/TimelineItemEventContentView.kt

@@ -35,6 +35,8 @@ import io.element.android.libraries.architecture.Presenter
 @Composable
 fun TimelineItemEventContentView(
    content: TimelineItemEventContent,
+    hideContent: Boolean,


hideMediaContent?

Better, yes: 0bb2fee3fb523664bd052979f937e9a2b7f2899d.

jmartinesp · 2024-10-04T06:21:50Z

...io/element/android/features/messages/impl/timeline/protection/TimelineProtectionPresenter.kt

+) : Presenter<TimelineProtectionState> {
+    @Composable
+    override fun present(): TimelineProtectionState {
+        val hideContent by appPreferencesStore.doesHideImagesAndVideosFlow().collectAsState(initial = false)


hideMediaContent?

Better, yes: 0bb2fee3fb523664bd052979f937e9a2b7f2899d.

jmartinesp · 2024-10-04T06:24:08Z

...io/element/android/features/messages/impl/timeline/protection/TimelineProtectionPresenter.kt

+    @Composable
+    override fun present(): TimelineProtectionState {
+        val hideContent by appPreferencesStore.doesHideImagesAndVideosFlow().collectAsState(initial = false)
+        var allowedEvents by remember { mutableStateOf<Set<EventId>>(setOf()) }


Should this be an immutable Set? I'm not sure if it might affect recompositions in any way, probably not.

I think it is simpler to leave it like that.

jmartinesp · 2024-10-04T06:26:53Z

...ages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt

@@ -72,6 +74,7 @@ import kotlinx.coroutines.launch
 @Composable
 fun TimelineView(
    state: TimelineState,
+    timelineProtectionState: TimelineProtectionState,


Maybe this could be used as a CompositionLocalProvider? This way we don't have it to explicitly pass it around through so many classes and functions, but it's ok as it is too.

Add it to TimelineRoomInfo as it's already passed everywhere?

Not easy at the state is computed in MessagesPresenter as I need the state for the MessageComposer. I may revisit this later.

Hide images, videos and stickers in the timeline. Disable click on hidden content. It must be revealed first. Add preview without BlurHash. Also hide image in thumbnails.

…enabled.

… remove `run` block.

sonarcloud · 2024-10-04T15:23:38Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

codecov · 2024-10-04T15:40:01Z

Codecov Report

Attention: Patch coverage is 84.07643% with 50 lines in your changes missing coverage. Please review.

Project coverage is 82.67%. Comparing base (98d9abe) to head (1f7b05a).
Report is 18 commits behind head on develop.

Files with missing lines	Patch %	Lines
.../messages/impl/timeline/protection/TimelineItem.kt	0.00%	22 Missing and 1 partial ⚠️
...imeline/components/TimelineItemGroupedEventsRow.kt	33.33%	4 Missing ⚠️
...eferences/impl/store/DefaultAppPreferencesStore.kt	0.00%	4 Missing ⚠️
...pl/notifications/DefaultNotifiableEventResolver.kt	66.66%	1 Missing and 3 partials ⚠️
...ries/matrix/ui/messages/reply/InReplyToMetadata.kt	40.00%	0 Missing and 3 partials ⚠️
...ssages/impl/timeline/components/TimelineItemRow.kt	71.42%	1 Missing and 1 partial ⚠️
...e/components/event/TimelineItemEventContentView.kt	66.66%	2 Missing ⚠️
...meline/components/event/TimelineItemStickerView.kt	92.59%	0 Missing and 2 partials ⚠️
...l/timeline/components/TimelineItemStateEventRow.kt	50.00%	1 Missing ⚠️
...timeline/components/event/TimelineItemImageView.kt	94.44%	0 Missing and 1 partial ⚠️
... and 4 more

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3592      +/-   ##
===========================================
- Coverage    82.68%   82.67%   -0.02%     
===========================================
  Files         1731     1737       +6     
  Lines        41014    41214     +200     
  Branches      4973     5003      +30     
===========================================
+ Hits         33912    34073     +161     
- Misses        5342     5372      +30     
- Partials      1760     1769       +9

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

bmarty added the PR-Feature For a new feature label Oct 3, 2024

bmarty changed the title ~~Hide images in the timeline~~ Add developer setting to hide images in the timeline Oct 3, 2024

bmarty force-pushed the feature/bma/hideImages branch from f6466a1 to dd8a6d5 Compare October 3, 2024 10:06

bmarty added the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Oct 3, 2024

github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Oct 3, 2024

bmarty force-pushed the feature/bma/hideImages branch from b598afe to 057943b Compare October 3, 2024 16:54

bmarty marked this pull request as ready for review October 3, 2024 16:55

bmarty requested a review from a team as a code owner October 3, 2024 16:55

bmarty requested review from jmartinesp and removed request for a team October 3, 2024 16:55

jmartinesp approved these changes Oct 4, 2024

View reviewed changes

bmarty added the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Oct 4, 2024

github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Oct 4, 2024

bmarty and others added 16 commits October 4, 2024 17:01

Add settings to hide images and videos in the timeline.

dd2a1b3

Hide images, videos and stickers in the timeline. Disable click on hidden content. It must be revealed first. Add preview without BlurHash. Also hide image in thumbnails.

Improve extension to fix a copy paste issue.

fd142c1

Do not render images in notification if the setting to hide image is …

f544f31

…enabled.

Fix test compilation

0244b71

Fix test

d2bd1b7

Add unit test on TimelineProtectionPresenter

8ffdfff

Add unit test on ProtectedView and TimelineProtectionState

73bdf0e

Remove dead code.

a465b0c

Fix test name.

5285696

Hide images: iterate on design.

8b282f2

Update screenshots

70c6bed

SuppressWarnings ModifierClickableOrder

a3e2457

Let invokeOnCurrentTimeline lambda param return Unit so that we can…

fafd5d4

… remove `run` block.

Rename hideContent to hideMediaContent

b889e86

Fix preview name.

a72c406

Do not inject the TimelineProtectionPresenter

1fad4d4

bmarty force-pushed the feature/bma/hideImages branch from c9ef67d to 1fad4d4 Compare October 4, 2024 15:03

bmarty added the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Oct 4, 2024

github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Oct 4, 2024

Update screenshots

1f7b05a

bmarty added the Run-Maestro Starts a Maestro Cloud session to run integration tests label Oct 4, 2024

bmarty enabled auto-merge October 4, 2024 15:26

github-actions bot removed the Run-Maestro Starts a Maestro Cloud session to run integration tests label Oct 4, 2024

bmarty merged commit 779c6db into develop Oct 4, 2024
30 checks passed

bmarty deleted the feature/bma/hideImages branch October 4, 2024 15:57

frebib mentioned this pull request Oct 5, 2024

Fix black-on-black status bars with hidden media #3611

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add developer setting to hide images in the timeline #3592

Add developer setting to hide images in the timeline #3592

bmarty commented Oct 3, 2024

github-actions bot commented Oct 3, 2024 •

edited

Loading

jmartinesp left a comment

jmartinesp Oct 4, 2024

bmarty Oct 4, 2024 •

edited

Loading

jmartinesp Oct 4, 2024

bmarty Oct 4, 2024 •

edited

Loading

jmartinesp Oct 4, 2024

bmarty Oct 4, 2024 •

edited

Loading

jmartinesp Oct 4, 2024

bmarty Oct 4, 2024

jmartinesp Oct 4, 2024

ganfra Oct 4, 2024

bmarty Oct 4, 2024

sonarcloud bot commented Oct 4, 2024

codecov bot commented Oct 4, 2024 •

edited

Loading

Add developer setting to hide images in the timeline #3592

Add developer setting to hide images in the timeline #3592

Conversation

bmarty commented Oct 3, 2024

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Checklist

github-actions bot commented Oct 3, 2024 • edited Loading

jmartinesp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmarty Oct 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmarty Oct 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmarty Oct 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Oct 4, 2024

Quality Gate passed

codecov bot commented Oct 4, 2024 • edited Loading

Codecov Report

github-actions bot commented Oct 3, 2024 •

edited

Loading

bmarty Oct 4, 2024 •

edited

Loading

bmarty Oct 4, 2024 •

edited

Loading

bmarty Oct 4, 2024 •

edited

Loading

codecov bot commented Oct 4, 2024 •

edited

Loading