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

CB-12146: (android) Adds support for "playAudioWhenScreenIsLocked" already used in iOS #121

Closed
wants to merge 3 commits into from

Conversation

romedius
Copy link
Contributor

@romedius romedius commented Nov 14, 2016

Platforms affected

Android

What does this PR do?

Handle sound pausing and resuming on events like onPause, AudioFocus and if the phone rings.

In this patch we handle the playAudioWhenScreenIsLocked option and we must handle three cases independently

  • on Pause and onResume - pause depening if playAudioWhenScreenIsLocked is set for this audio player
  • AudioFocus (refactored)
  • phone ringing (refactored)

We have three general states:

  • No paused sounds
  • All sounds paused
  • Sounds only paused if the playAudioWhenScreenIsLocked flag is set to false

What testing has been done on this change?

Manual testing on Nexus 5

Checklist

…and if the phone rings.

In this patch we handle the playAudioWhenScreenIsLocked option and we must handle three cases independently
 * on Pause and onResume - pause depening if playAudioWhenScreenIsLocked is set for this audio player
 * AudioFocus (refactored)
 * phone ringing (refactored)

We have three general states:
 * No paused sounds
 * All sounds paused
 * Sounds only paused if the playAudioWhenScreenIsLocked flag is set to false
@cordova-qa
Copy link

Cordova CI Build has one or more failures.

Commit - Link
Dashboard - Link

Builder Name Console Output Test Report Device Logs
Windows 8.1 Store Link Link Link
Windows 10 Store Link Link Link
Windows 8.1 Phone Link Link Link
iOS Link Link Link
Android Link Link Link

@stevengill
Copy link
Contributor

Hey @romedius,

Just to confirm, this replaces your other pr. #54

@romedius
Copy link
Contributor Author

Yes, I just wanted to wait with closing the other one until there is some activity on this one :)

@romedius
Copy link
Contributor Author

I would like to change the documentation.
Now that Android and iOS are supporting this flag, should it still be listed as a quirk?

@stevengill
Copy link
Contributor

Hey @romedius,

A docs pr would be great! I would suggest adding it as a normal option and maybe adding a quirks section for windows saying that it doesn't support the option.

A first pass of the code looks good to me. I'll test it a bit more, but expect this to get merged and part of the next media plugin release!

Thanks!

@cordova-qa
Copy link

Cordova CI Build has one or more failures.

Commit - Link
Dashboard - Link

Builder Name Console Output Test Report Device Logs
Windows 8.1 Store Link Link Link
Windows 10 Store Link Link Link
Windows 8.1 Phone Link Link Link
iOS Link Link Link
Android Link Link Link

@romedius
Copy link
Contributor Author

@stevengill Hi! I will merge the functionality this week.

@stevengill
Copy link
Contributor

Hey @romedius

Can you rebase this? I'll merge it in

@romedius
Copy link
Contributor Author

New PR:
#137

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.

3 participants