Skip to content

Add a setting to be able to always appear offline #5583

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

Merged
merged 24 commits into from
Apr 12, 2022

Conversation

aringenbach
Copy link
Contributor

@aringenbach aringenbach commented Mar 18, 2022

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

Presence enabling has been moved to an offline mode user setting. When enabled, user will always appear offline, even when using the application.

Motivation and context

Fixes #5582

Screenshots / GIFs

Screenshot_20220412_101940

Tests

  • Step 1: Have contacts on servers that show presence (e.g. local synapse)
  • Step 2: Toggle setting

Tested devices

  • Physical
  • Emulator
  • OS version(s): API 32

Checklist

Copy link
Contributor Author

@aringenbach aringenbach left a comment

Choose a reason for hiding this comment

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

This might require input from product team, as this is adding a setting we might not want for some reason.

@aringenbach aringenbach requested review from a team and ariskotsomitopoulos and removed request for a team March 18, 2022 16:57
@github-actions
Copy link

github-actions bot commented Mar 18, 2022

Unit Test Results

114 files  ±0  114 suites  ±0   1m 18s ⏱️ -6s
201 tests ±0  201 ✔️ ±0  0 💤 ±0  0 ±0 
674 runs  ±0  674 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit 5cd559a. ± Comparison against base commit 836a12d.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@ariskotsomitopoulos ariskotsomitopoulos left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, I added some comments

@aringenbach
Copy link
Contributor Author

This might require input from product team, as this is adding a setting we might not want for some reason.

Done. It's ok to have this setting available, and it will be added on iOS as well as part of the presence display improvement there.

Copy link
Contributor

@ariskotsomitopoulos ariskotsomitopoulos left a comment

Choose a reason for hiding this comment

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

It would be good to avoid the activity restart if possible. I added a comment regarding that

…tting

# Conflicts:
#	matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixConfiguration.kt
#	vector/src/main/java/im/vector/app/core/di/SingletonModule.kt
#	vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt
@aringenbach aringenbach changed the title Move presence enabling to a app-only preference Move presence enabling to a offline mode user setting Mar 30, 2022
@aringenbach aringenbach changed the title Move presence enabling to a offline mode user setting Move presence enabling to an offline mode user setting Mar 30, 2022
Copy link
Contributor

@ariskotsomitopoulos ariskotsomitopoulos left a comment

Choose a reason for hiding this comment

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

Thanks for the update, some comments

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.

Some minor and less minor remarks

@aringenbach aringenbach requested a review from bmarty March 31, 2022 17:40
…tting

# Conflicts:
#	vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt
…tting

# Conflicts:
#	vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt
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 for the update.

Just a last remark about the changelog.

Also the PR description is quite out of date. Can you update it please, for posterity?

) : PresenceService {

override suspend fun setMyPresence(presence: PresenceEnum, statusMsg: String?) {
lightweightSettingsStorage.setSyncPresenceStatus(SyncPresence.from(presence))
Copy link
Member

Choose a reason for hiding this comment

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

Not a big deal, but storing the value could be handled by setPresenceTask. Not sure what is best, so let's keep it like that.

@aringenbach aringenbach changed the title Move presence enabling to an offline mode user setting Add a setting to be able to always appear offline Apr 12, 2022
@aringenbach aringenbach requested a review from bmarty April 12, 2022 09:08
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, for the update. I will squash and commit.

@bmarty bmarty merged commit 047a45d into develop Apr 12, 2022
@bmarty bmarty deleted the feature/aringenbach/presence-sync-user-setting branch April 12, 2022 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move presence enabling to an offline mode user setting
3 participants