From 3eeb2816b2bbc49183f5cf87844c3741a03cb36c Mon Sep 17 00:00:00 2001 From: "Kevin .M. Gitonga" Date: Thu, 2 Feb 2023 03:45:03 +0300 Subject: [PATCH] Fix #1801: Audio player running after closing (#4629) ## Explanation Fix #1801 This fix adds check in the implemented playStatusLiveData to complete audioPause request if any is pending while the playerStatus keeps changing. When user clicks on pause audio icon and the UI disappears there is a delay in audio actually pausing. Adding a check in playStatusLiveData completes pause pending request fast enough to prevent it from playing. ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [ ] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only If your PR includes UI-related changes, then: - Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes - For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see [RTL guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines)) - Add a video showing the full UX flow with a screen reader enabled (see [accessibility guide](https://github.com/oppia/oppia-android/wiki/Accessibility-(A11y)-Guide)) - Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing ## Before changes demo https://user-images.githubusercontent.com/20886444/192735111-6baf46f8-3531-49f8-83d6-98277dc74c6d.mp4 ## After changes demo https://user-images.githubusercontent.com/20886444/192735968-13b7e2b5-e296-4978-9194-4df5b807897b.mp4 --------- Co-authored-by: Ben Henning --- .../app/player/audio/AudioFragmentPresenter.kt | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) mode change 100755 => 100644 app/src/main/java/org/oppia/android/app/player/audio/AudioFragmentPresenter.kt diff --git a/app/src/main/java/org/oppia/android/app/player/audio/AudioFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/player/audio/AudioFragmentPresenter.kt old mode 100755 new mode 100644 index 08804ac5817..5e5f4cd29c6 --- a/app/src/main/java/org/oppia/android/app/player/audio/AudioFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/player/audio/AudioFragmentPresenter.kt @@ -66,6 +66,8 @@ class AudioFragmentPresenter @Inject constructor( private val viewModel by lazy { getAudioViewModel() } + + private var isPauseAudioRequestPending = false private lateinit var binding: AudioFragmentBinding /** Sets up SeekBar listener, ViewModel, and gets VoiceoverMappings or restores saved state */ @@ -109,6 +111,15 @@ class AudioFragmentPresenter @Inject constructor( Observer { prepared = it != UiAudioPlayStatus.LOADING && it != UiAudioPlayStatus.FAILED binding.audioProgressSeekBar.isEnabled = prepared + + // This check will execute any pending pause request that causes issues with + // audio not being paused as the user navigates through lessons in a topic. + // Check #1801 for more details, and specifically + // https://github.com/oppia/oppia-android/pull/4629#issuecomment-1410005186 + // for notes on why this fix works. + if (prepared && isPauseAudioRequestPending) { + pauseAudio() + } } ) @@ -225,8 +236,11 @@ class AudioFragmentPresenter @Inject constructor( viewModel.loadFeedbackAudio(contentId, allowAutoPlay) fun pauseAudio() { - if (prepared) + isPauseAudioRequestPending = true + if (prepared && isPauseAudioRequestPending) { viewModel.pauseAudio() + isPauseAudioRequestPending = false + } } fun handleEnableAudio(saveUserChoice: Boolean) {