From 00994dc3d7f705d7322f66b8a8bd6585f9e9aa74 Mon Sep 17 00:00:00 2001 From: Alfonso Grillo Date: Fri, 3 Nov 2023 11:39:22 +0100 Subject: [PATCH] Add deferred spinner for voice messages (#2018) * Add delayedLoaderPlaybackState * Add more tests * Fix Xcode 14 build error * Refactor delayedLoaderPlaybackState -> playerButtonPlaybackState * Fix runtime issue --- .../VoiceMessage/VoiceMessageButton.swift | 4 +- .../View/VoiceMessagePreviewComposer.swift | 2 +- .../Audio/Player/AudioPlayerState.swift | 31 ++++++++++++-- .../RoomTimelineController.swift | 6 ++- .../VoiceMessageRoomPlaybackView.swift | 2 +- UnitTests/Sources/AudioPlayerStateTests.swift | 40 +++++++++++++++++++ 6 files changed, 75 insertions(+), 10 deletions(-) diff --git a/ElementX/Sources/Other/VoiceMessage/VoiceMessageButton.swift b/ElementX/Sources/Other/VoiceMessage/VoiceMessageButton.swift index 6ff3d41ae..d4b6c7adc 100644 --- a/ElementX/Sources/Other/VoiceMessage/VoiceMessageButton.swift +++ b/ElementX/Sources/Other/VoiceMessage/VoiceMessageButton.swift @@ -88,13 +88,13 @@ private struct VoiceMessageButtonStyle: ButtonStyle { } extension VoiceMessageButton.State { - init(state: AudioPlayerPlaybackState) { + init(_ state: AudioPlayerPlaybackState) { switch state { case .loading: self = .loading case .playing: self = .playing - default: + case .stopped, .error, .readyToPlay: self = .paused } } diff --git a/ElementX/Sources/Screens/ComposerToolbar/View/VoiceMessagePreviewComposer.swift b/ElementX/Sources/Screens/ComposerToolbar/View/VoiceMessagePreviewComposer.swift index e69fa4934..f181019ea 100644 --- a/ElementX/Sources/Screens/ComposerToolbar/View/VoiceMessagePreviewComposer.swift +++ b/ElementX/Sources/Screens/ComposerToolbar/View/VoiceMessagePreviewComposer.swift @@ -43,7 +43,7 @@ struct VoiceMessagePreviewComposer: View { var body: some View { HStack { HStack { - VoiceMessageButton(state: .init(state: playerState.playbackState), + VoiceMessageButton(state: .init(playerState.playerButtonPlaybackState), size: .small, action: onPlayPause) Text(timeLabelContent) diff --git a/ElementX/Sources/Services/Audio/Player/AudioPlayerState.swift b/ElementX/Sources/Services/Audio/Player/AudioPlayerState.swift index a6ee2f94d..53789fedb 100644 --- a/ElementX/Sources/Services/Audio/Player/AudioPlayerState.swift +++ b/ElementX/Sources/Services/Audio/Player/AudioPlayerState.swift @@ -37,11 +37,15 @@ class AudioPlayerState: ObservableObject, Identifiable { let duration: Double let waveform: EstimatedWaveform @Published private(set) var playbackState: AudioPlayerPlaybackState + /// It's similar to `playbackState`, with the a difference: `.loading` + /// updates are delayed by a fixed amount of time + @Published private(set) var playerButtonPlaybackState: AudioPlayerPlaybackState @Published private(set) var progress: Double @Published private(set) var showProgressIndicator: Bool private weak var audioPlayer: AudioPlayerProtocol? - private var cancellables: Set = [] + private var audioPlayerSubscription: AnyCancellable? + private var playbackStateSubscription: AnyCancellable? private var displayLink: CADisplayLink? /// The file url that the last player attached to this object has loaded. @@ -63,6 +67,8 @@ class AudioPlayerState: ObservableObject, Identifiable { self.progress = progress showProgressIndicator = false playbackState = .stopped + playerButtonPlaybackState = .stopped + setupPlaybackStateSubscription() } deinit { @@ -99,7 +105,7 @@ class AudioPlayerState: ObservableObject, Identifiable { func detachAudioPlayer() { audioPlayer?.stop() stopPublishProgress() - cancellables = [] + audioPlayerSubscription = nil audioPlayer = nil playbackState = .stopped showProgressIndicator = false @@ -112,7 +118,7 @@ class AudioPlayerState: ObservableObject, Identifiable { // MARK: - Private private func subscribeToAudioPlayer(audioPlayer: AudioPlayerProtocol) { - audioPlayer.actions + audioPlayerSubscription = audioPlayer.actions .receive(on: DispatchQueue.main) .sink { [weak self] action in guard let self else { @@ -122,7 +128,6 @@ class AudioPlayerState: ObservableObject, Identifiable { await self.handleAudioPlayerAction(action) } } - .store(in: &cancellables) } private func handleAudioPlayerAction(_ action: AudioPlayerAction) async { @@ -175,6 +180,24 @@ class AudioPlayerState: ObservableObject, Identifiable { private func restoreAudioPlayerState(audioPlayer: AudioPlayerProtocol) async { await audioPlayer.seek(to: progress) } + + private func setupPlaybackStateSubscription() { + playbackStateSubscription = $playbackState + .map { state in + switch state { + case .loading: + return Just(state) + .delay(for: .seconds(2), scheduler: RunLoop.main) + .eraseToAnyPublisher() + case .playing, .stopped, .error, .readyToPlay: + return Just(state) + .eraseToAnyPublisher() + } + } + .switchToLatest() + .removeDuplicates() + .weakAssign(to: \.playerButtonPlaybackState, on: self) + } } extension AudioPlayerState: Equatable { diff --git a/ElementX/Sources/Services/Timeline/TimelineController/RoomTimelineController.swift b/ElementX/Sources/Services/Timeline/TimelineController/RoomTimelineController.swift index fbd443e3d..a7e978a7f 100644 --- a/ElementX/Sources/Services/Timeline/TimelineController/RoomTimelineController.swift +++ b/ElementX/Sources/Services/Timeline/TimelineController/RoomTimelineController.swift @@ -409,8 +409,10 @@ class RoomTimelineController: RoomTimelineControllerProtocol { guard let playerState = mediaPlayerProvider.playerState(for: .timelineItemIdentifier(timelineItem.id)) else { continue } - playerState.detachAudioPlayer() - mediaPlayerProvider.unregister(audioPlayerState: playerState) + Task { @MainActor in + playerState.detachAudioPlayer() + mediaPlayerProvider.unregister(audioPlayerState: playerState) + } } newTimelineItems.append(timelineItem) diff --git a/ElementX/Sources/Services/Timeline/TimelineItems/Items/Messages/VoiceMessages/VoiceMessageRoomPlaybackView.swift b/ElementX/Sources/Services/Timeline/TimelineItems/Items/Messages/VoiceMessages/VoiceMessageRoomPlaybackView.swift index de8ad7f87..d97f6094b 100644 --- a/ElementX/Sources/Services/Timeline/TimelineItems/Items/Messages/VoiceMessages/VoiceMessageRoomPlaybackView.swift +++ b/ElementX/Sources/Services/Timeline/TimelineItems/Items/Messages/VoiceMessages/VoiceMessageRoomPlaybackView.swift @@ -43,7 +43,7 @@ struct VoiceMessageRoomPlaybackView: View { var body: some View { HStack { HStack { - VoiceMessageButton(state: .init(state: playerState.playbackState), + VoiceMessageButton(state: .init(playerState.playerButtonPlaybackState), size: .medium, action: onPlayPause) Text(timeLabelContent) diff --git a/UnitTests/Sources/AudioPlayerStateTests.swift b/UnitTests/Sources/AudioPlayerStateTests.swift index 4a722dbb0..57e60d517 100644 --- a/UnitTests/Sources/AudioPlayerStateTests.swift +++ b/UnitTests/Sources/AudioPlayerStateTests.swift @@ -69,6 +69,46 @@ class AudioPlayerStateTests: XCTestCase { XCTAssertFalse(audioPlayerState.showProgressIndicator) } + func testDelayedState() async throws { + audioPlayerState.attachAudioPlayer(audioPlayerMock) + + XCTAssert(audioPlayerState.isAttached) + XCTAssertEqual(audioPlayerState.playbackState, .loading) + XCTAssertEqual(audioPlayerState.playerButtonPlaybackState, .stopped) + + let deferred = deferFulfillment(audioPlayerState.$playerButtonPlaybackState) { output in + switch output { + case .loading: + return true + default: + return false + } + } + try await deferred.fulfill() + + XCTAssertEqual(audioPlayerState.playerButtonPlaybackState, .loading) + } + + func testOtherActionsAreNotDelayed() async throws { + audioPlayerState.attachAudioPlayer(audioPlayerMock) + XCTAssertEqual(audioPlayerState.playbackState, .loading) + XCTAssertEqual(audioPlayerState.playerButtonPlaybackState, .stopped) + + let deferred = deferFulfillment(audioPlayerState.$playerButtonPlaybackState) { output in + switch output { + case .playing: + return true + default: + return false + } + } + + audioPlayerActionsSubject.send(.didStartPlaying) + try await deferred.fulfill() + XCTAssertEqual(audioPlayerState.playbackState, .playing) + XCTAssertEqual(audioPlayerState.playerButtonPlaybackState, .playing) + } + func testReportError() async throws { XCTAssertEqual(audioPlayerState.playbackState, .stopped) audioPlayerState.reportError(AudioPlayerError.genericError)