-
-
Notifications
You must be signed in to change notification settings - Fork 307
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
[tvOS] Fix play/pause button of siri remote #933
Conversation
6f9b685
to
1fe5b6c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This actually requires a lot more state handling.
1 - Don't make a new state handler, the player state is held in the environment VideoPlayerManager
object. The behavior should be the same as the play buttons based on the player state for iOS.
2 - We need to set the state of the overlays. When the play/pause button is pressed, the main overlay should be presented if it isn't and poke the overlay timer.
@State | ||
private var isPlaying = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@State | |
private var isPlaying = true | |
@EnvironmentObject | |
private var videoPlayerManager: VideoPlayerManager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably best to place this @EnvironmentObject
above with the other environment objects, its just that I couldn't find a way to use GitHub's suggestion feature on lines that have not been edited in the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it should go with the other EnvironmentObject
s.
if isPlaying { | ||
proxy.pause() | ||
isPlaying = false | ||
} else { | ||
proxy.play() | ||
isPlaying = true | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if isPlaying { | |
proxy.pause() | |
isPlaying = false | |
} else { | |
proxy.play() | |
isPlaying = true | |
} | |
if !isPresentingOverlay { | |
isPresentingOverlay = true | |
overlayTimer.start(5) | |
} | |
switch videoPlayerManager.state { | |
case .playing: | |
proxy.pause() | |
default: | |
proxy.play() | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LePips does this solve your change request?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not just seeing if the overlay is being presented but also set it to the main overlay.
This will be overridden by #1066 with the change to not use the SwiftUI press handlers and use See why necessary: |
Restore the Siri Remote play/pause button handling in main.