Skip to content

change (preferences) : new moderation and safety settings #4574

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Apr 11, 2025
Merged

change (preferences) : new moderation and safety settings #4574

merged 10 commits into from
Apr 11, 2025

Conversation

ganfra
Copy link
Member

@ganfra ganfra commented Apr 10, 2025

Content

  • Move timeline preview setting related from developper to advanced settings, and add new value to show only in private rooms
  • Also add new setting to hide avatars from invites.

Motivation and context

Closes #4436

Screenshots / GIFs

See recorded screenshots.

Tests

  • Step 1
  • Step 2
  • Step ...

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

@ganfra ganfra changed the title Feature/fga/advanced settings moderation and safety change (preferences) : new moderation and safety settings Apr 11, 2025
@ganfra ganfra added PR-Change For updates to an existing feature Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. labels Apr 11, 2025
@github-actions github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Apr 11, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 11, 2025 •
edited
Loading

📱 Scan the QR code below to install the build (arm64 only) for this PR.
QR code
If you can't scan the QR code you can install the build via this link: https://i.diawi.com/K2dRNj

@ganfra ganfra marked this pull request as ready for review April 11, 2025 09:41
@ganfra ganfra requested a review from a team as a code owner April 11, 2025 09:41
@ganfra ganfra requested review from bmarty and removed request for a team April 11, 2025 09:41
@codecov Codecov
Copy link

codecov bot commented Apr 11, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 82.35294% with 24 lines in your changes missing coverage. Please review.

Project coverage is 80.07%. Comparing base (84ff284) to head (0964b7b).
Report is 13 commits behind head on develop.

Files with missing lines Patch % Lines
...eferences/impl/store/DefaultAppPreferencesStore.kt 0.00% 6 Missing ⚠️
.../preferences/impl/advanced/AdvancedSettingsView.kt 82.14% 4 Missing and 1 partial ⚠️
...references/impl/advanced/AdvancedSettingsEvents.kt 0.00% 2 Missing ⚠️
...erences/impl/advanced/AdvancedSettingsPresenter.kt 80.00% 2 Missing ⚠️
...id/libraries/matrix/api/media/MediaPreviewValue.kt 83.33% 0 Missing and 2 partials ⚠️
...es/preferences/test/InMemoryAppPreferencesStore.kt 75.00% 2 Missing ⚠️
...pl/notifications/DefaultNotifiableEventResolver.kt 0.00% 0 Missing and 2 partials ⚠️
...nt/android/features/joinroom/impl/JoinRoomState.kt 50.00% 0 Missing and 1 partial ⚠️
...ndroid/features/joinroom/impl/di/JoinRoomModule.kt 0.00% 1 Missing ⚠️
...libraries/designsystem/components/avatar/Avatar.kt 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #4574      +/-   ##
===========================================
- Coverage    80.08%   80.07%   -0.01%     
===========================================
  Files         2087     2088       +1     
  Lines        55121    55214      +93     
  Branches      6876     6889      +13     
===========================================
+ Hits         44144    44213      +69     
- Misses        8617     8633      +16     
- Partials      2360     2368       +8

☔ 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.

bmarty
bmarty approved these changes Apr 11, 2025
View reviewed changes
Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. A few minor remarks

...ures/joinroom/impl/src/main/kotlin/io/element/android/features/joinroom/impl/JoinRoomView.kt Outdated
@@ -385,13 +387,14 @@ private fun JoinRoomContent(
Column(horizontalAlignment = Alignment.CenterHorizontally) {
val inviteSender = (contentState.joinAuthorisationStatus as? JoinAuthorisationStatus.IsInvited)?.inviteSender
if (inviteSender != null) {
InviteSenderView(inviteSender = inviteSender)
InviteSenderView(inviteSender = inviteSender, hideAvatarImage = shouldHideAvatars)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of shouldHideAvatars, why not using hideAvatarImage? Should has to be used when the behavior is optional.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should first meaning is about obligation, not optionality, so this makes sense. But I can change if you really want.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure? For obligation, we use Must. But OK to keep it like that, not a big deal.

...c/test/snapshots/images/features.preferences.impl.advanced_AdvancedSettingsView_Day_0_en.png Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use @PreviewWithLargeHeight for this preview?

...ain/kotlin/io/element/android/libraries/preferences/impl/store/DefaultAppPreferencesStore.kt
@@ -31,7 +32,8 @@ private val Context.dataStore: DataStore<Preferences> by preferencesDataStore(na
private val developerModeKey = booleanPreferencesKey("developerMode")
private val customElementCallBaseUrlKey = stringPreferencesKey("elementCallBaseUrl")
private val themeKey = stringPreferencesKey("theme")
private val hideImagesAndVideosKey = booleanPreferencesKey("hideImagesAndVideos")
private val hideInviteAvatarsKey = booleanPreferencesKey("hideInviteAvatars")
private val timelineMediaPreviewValueKey = stringPreferencesKey("timelineMediaPreviewValue")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe create a AppMigration08 class to ensure that if previous key hideImagesAndVideos was set to true, hideInviteAvatars is set to true and timelineMediaPreviewValue is set to Private?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed this internally and we said to not bother with migration

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(it's a quick thing to do, but OK)

@ganfra ganfra added the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Apr 11, 2025
@github-actions github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Apr 11, 2025
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed 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 SonarQube Cloud

@ganfra ganfra merged commit 62a2c2f into develop Apr 11, 2025
31 checks passed
@ganfra ganfra deleted the feature/fga/advanced_settings_moderation_and_safety branch April 11, 2025 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bmarty bmarty bmarty approved these changes

Assignees
No one assigned
Labels
PR-Change For updates to an existing feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Task] Advanced settings override for hide invite avatars and timeline media
3 participants
@ganfra @bmarty @ElementBot