element-ios
element-ios copied to clipboard
Background audio messages info (PSB-180)
Ticket -> https://element-io.atlassian.net/browse/PSB-180?atlOrigin=eyJpIjoiM2QxY2RmZWM1MDcxNDAwN2E1YWJiYjU3OWM2MmZkMzUiLCJwIjoiaiJ9
This PR
- Hides the information about the message/room in the background audio player
- Introduces the new
BuildSetting.allowBackgroundAudioMessagePlaybackto disable the background audio feature completely
| Before | After |
|---|---|
![]() |
![]() |
:iphone: Scan the QR code below to install the build for this PR. :lock: This build is for internal testing purpose. Only devices listed in the ad-hoc provisioning profile can install Element Alpha.
If you can't scan the QR code you can install the build via this link: https://i.diawi.com/miQ3Ad
Codecov Report
Base: 11.63% // Head: 11.63% // Decreases project coverage by -0.00% :warning:
Coverage data is based on head (
ac655c4) compared to base (380ad94). Patch coverage: 0.00% of modified lines in pull request are covered.
Additional details and impacted files
@@ Coverage Diff @@
## develop #6870 +/- ##
===========================================
- Coverage 11.63% 11.63% -0.01%
===========================================
Files 1559 1559
Lines 155250 155255 +5
Branches 62667 62668 +1
===========================================
- Hits 18060 18057 -3
- Misses 136578 136586 +8
Partials 612 612
| Impacted Files | Coverage Δ | |
|---|---|---|
| Config/BuildSettings.swift | 43.07% <ø> (ø) |
|
| ...s/Room/VoiceMessages/VoiceMessageAudioPlayer.swift | 0.00% <0.00%> (ø) |
|
| ...iceMessages/VoiceMessageMediaServiceProvider.swift | 0.00% <0.00%> (ø) |
|
| ...ojiPicker/Data/EmojiMart/EmojiItem+EmojiMart.swift | 95.34% <0.00%> (-0.31%) |
:arrow_down: |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
The code looks fine to me but I'm going to question the decision to remove useful info from the now playing screen on Element. We may have users that are used to having this and I can't really see any justification for removing it for regular users.
Could that change perhaps also be behind a build flag?
That sounds reasonable to me. But how are we going to do it? Adding a second flag?
- Flag 1: background audio on/off
- Flag 2: less info in the player on/off
What do you think @Johennes?
We had discussed removing the possibly sensitive info on the lock screen with product before and reached this decision due to privacy concerns. This does seem like a relatively granular setting so my personal tendency is to pick a sensible default rather than adding another configuration option. @jakewb-b maybe you could expand on this?
Another option I was thinking is to have 3-states build setting like this:
extension BuildSetting {
enum BackgroundAudioPlayerBehaviour {
case off
case `default`
case anonymous
}
static let backgroundAudioPlayerBehaviour: BackgroundAudioPlayerBehaviour = .default
}
But probably it's better to understand with @jakewb-b if this should be or not a runtime setting.
As a rule I’d much prefer to avoid additional settings and choose a sensible default, as Johannes suggests. In this case, if we’re going with a default we should bias for privacy and I’m content to remove the information from the lock screen as I think it’s of relatively low utility and, while any change may be a surprise to users, it’s not something that seems likely to cause huge issues in using the feature.
I think there could be a case in the future for a runtime setting that bundles a few other items into one privacy choice to, for example, ‘minimise display of room and user information on lock screen’ but we’d need to work out if there are, in practice, any other items that could usefully be bundled with this.
In the short term, this mustn’t block us progressing so let’s go with hiding the information for now and then review.
As a rule I’d much prefer to avoid additional settings and choose a sensible default, as Johannes suggests. In this case, if we’re going with a default we should bias for privacy and I’m content to remove the information from the lock screen as I think it’s of relatively low utility and, while any change may be a surprise to users, it’s not something that seems likely to cause huge issues in using the feature.
I think there could be a case in the future for a runtime setting that bundles a few other items into one privacy choice to, for example, ‘minimise display of room and user information on lock screen’ but we’d need to work out if there are, in practice, any other items that could usefully be bundled with this.
In the short term, this mustn’t block us progressing so let’s go with hiding the information for now and then review.
Thanks Jake, @pixlwave, still needing your approval if we are happy with the code. ;-)









