Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Closes #4161: Make MediaState extension methods visible for consuming apps. #4351

Merged
merged 1 commit into from
Sep 10, 2019

Conversation

pocmo
Copy link
Contributor

@pocmo pocmo commented Sep 9, 2019


Pull Request checklist

  • Quality: This PR builds and passes detekt/ktlint checks (A pre-push hook is recommended)
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry or does not need one
  • Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

After merge

  • Milestone: Make sure issues closed by this pull request are added to the milestone of the version currently in development.
  • Breaking Changes: If this is a breaking change, please push a draft PR on Reference Browser to address the breaking issues.

@pocmo pocmo added the 🕵️‍♀️ needs review PRs that need to be reviewed label Sep 9, 2019
@pocmo pocmo requested a review from sblatz September 9, 2019 17:46
@pocmo pocmo requested a review from a team as a code owner September 9, 2019 17:46
Copy link
Contributor

@sblatz sblatz left a comment

Choose a reason for hiding this comment

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

This all looks great to me 😄

I did add one more helper function in Fenix that we should consider adding here:

/* Helper function to invert a MediaState */
fun MediaState.inverse(): MediaState {
    return when (this) {
        is MediaState.Playing -> {
            MediaState.Paused(
                session = session,
                media = media
            )
        }
        is MediaState.Paused -> {
            MediaState.Playing(
                session = session,
                media = media
            )
        }
        is MediaState.None -> { MediaState.None }
    }
}

@pocmo
Copy link
Contributor Author

pocmo commented Sep 9, 2019

@sblatz 👀 I wonder what you need that for. I'd expect those to only come out of MediaStateMachine.

@jonalmeida
Copy link
Contributor

bors r=csadilek

bors bot pushed a commit that referenced this pull request Sep 9, 2019
4351: Closes #4161: Make MediaState extension methods visible for consuming apps. r=csadilek a=pocmo



4360: Closes #4350: Allow for a missing `action` param in FxA auth flows r=jonalmeida a=grigoryk

It's possible that `action` parameter may be missing. This patch adds handling for that case,
and tests for the interceptor (which were missing entirely) that cover all combinations.




Co-authored-by: Sebastian Kaspari <s.kaspari@gmail.com>
Co-authored-by: Grisha Kruglov <gkruglov@mozilla.com>
@bors
Copy link

bors bot commented Sep 10, 2019

Build succeeded

@bors bors bot merged commit 2b25394 into mozilla-mobile:master Sep 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🕵️‍♀️ needs review PRs that need to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants