Skip to content

Commit

Permalink
Fix #1801: Audio player running after closing (#4629)
Browse files Browse the repository at this point in the history
## 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
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [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
<!-- Delete these section if this PR does not include UI-related
changes. -->
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 <[email protected]>
  • Loading branch information
KevinGitonga and BenHenning authored Feb 2, 2023
1 parent d578a96 commit 3eeb281
Showing 1 changed file with 15 additions and 1 deletion.
16 changes: 15 additions & 1 deletion app/src/main/java/org/oppia/android/app/player/audio/AudioFragmentPresenter.kt
100755 → 100644
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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()
}
}
)

Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 3eeb281

Please sign in to comment.