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

[tvOS] Fix play/pause button of siri remote #933

Closed
wants to merge 1 commit into from

Conversation

sy6sy2
Copy link
Contributor

@sy6sy2 sy6sy2 commented Dec 12, 2023

Restore the Siri Remote play/pause button handling in main.

Copy link
Member

@LePips LePips left a 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.

Comment on lines +29 to +30
@State
private var isPlaying = true
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@State
private var isPlaying = true
@EnvironmentObject
private var videoPlayerManager: VideoPlayerManager

Copy link
Contributor

@jhays jhays Dec 22, 2023

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.

Copy link
Member

@LePips LePips Jan 2, 2024

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 EnvironmentObjects.

Comment on lines +75 to +81
if isPlaying {
proxy.pause()
isPlaying = false
} else {
proxy.play()
isPlaying = true
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
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()
}

Copy link
Contributor

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?

Copy link
Member

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.

@LePips
Copy link
Member

LePips commented May 26, 2024

This will be overridden by #1066 with the change to not use the SwiftUI press handlers and use PreferencesView instead.

See why necessary:

@LePips LePips closed this May 26, 2024
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