Skip to content
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

[AND-401] Fix audio recordings not paused #5685

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from

Conversation

VelikovPetar
Copy link
Contributor

@VelikovPetar VelikovPetar commented Mar 18, 2025

🎯 Goal

A playing audio recording attachment in the Message List is not paused/stopped in the following scenarios:

  • Minimize app
  • Open a screen on top of the Message List
  • Delete a message with a playing audio recording

🛠 Implementation details

ui-common

  • Standardise the way the audioHash is created for both XML and Compose SDKs. On the XML SDK, the audioHash was generated as Attachment::hashCode, while on the Compose SDK the audioHash was generated as (attachment.assetUrl ?: attachment.upload?.toUri()?.toString()).hashCode(). This is now unified in the Attachment::audioHash extension, and the same is used in both SDK. This is needed for the change in MessageListController:
  • Before calling chatClient.deleteMessage(), in the MessageListController, we check if the deleted message has a playing audio recording -> by comparing the chatClient.audioPlayer.currentPlayingId with the message.attachment.audioHash. If the message currently being deleted has a playing audio, pause it before launching delete request.

ui-components

  • Add a LifecycleObserver in the audio recording attachment view holder, which will pause any running audio in onPause.
  • Additionally, add a similar observer in MessageListView, to handle the case when the audio recording viewHolder is recycled (when scrolling away) which causes the item-level observer to be removed.
  • In theory, only the MessageListView observer is enough to fix the bug, but I added the one on the Item for the case when a customer would use the MediaAttachmentsViewHolder independently form the MessageListView

compose

  • Add LifecycleEventEffect in AudioRecordAttachmentContent and AudioRecordAttachmentPreviewContent, which pause any running audio in onPause.
  • Additionally, add a similar observer in MessageList, to handle the case when the audio recording item composable is disposed (when scrolling away) which causes the item-level effect to be removed.
  • In theory, only the MessageListView observer is enough to fix the bug, but I added the one on the Item for the case when a customer would use the AudioRecordAttachmentContent independently form the MessageList

🎨 UI Changes

Before After
compose-before.mp4
compose-after.mp4
xml-before.mp4
xml-after.mp4

🧪 Testing

  1. Open Compose/XML SDK
  2. Open Channel
  3. Send, or scroll to an audio recording message
  4. Play it
  5. Open a screen on top of it / minimise the app / delete the active message
  6. The audio recording should be paused

Copy link
Contributor

github-actions bot commented Mar 18, 2025

SDK Size Comparison 📏

SDK Before After Difference Status
stream-chat-android-client 3.18 MB 3.18 MB 0.00 MB 🟢
stream-chat-android-offline 3.39 MB 3.39 MB 0.00 MB 🟢
stream-chat-android-ui-components 7.89 MB 7.89 MB 0.00 MB 🟢
stream-chat-android-compose 9.90 MB 9.90 MB 0.00 MB 🟢

@VelikovPetar VelikovPetar changed the title Bug/fix audio recordings not paused [AND-401] Fix audio recordings not paused Mar 20, 2025
@VelikovPetar VelikovPetar linked an issue Mar 20, 2025 that may be closed by this pull request
@VelikovPetar VelikovPetar marked this pull request as ready for review March 20, 2025 19:11
@VelikovPetar VelikovPetar requested a review from a team as a code owner March 20, 2025 19:11
Comment on lines 238 to 239
val audioPlayer = ChatClient.instance().audioPlayer
audioPlayer.pause()
Copy link
Member

Choose a reason for hiding this comment

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

Our UI should't use the ChatClient itself.
Could we move this logic to the ViewModel?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will try to re-wire it to go through the VM.

Comment on lines 651 to 655
private val pauseAudioPlayerListener = object : DefaultLifecycleObserver {
override fun onPause(owner: LifecycleOwner) {
ChatClient.instance().audioPlayer.pause()
}
}
Copy link
Member

Choose a reason for hiding this comment

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

The same case here, ChatClient shouldn't be used within our UI

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
21.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

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.

Audio message does not stop playing in certain scenarios
2 participants