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

Google cast support with docs #293

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gelotte
Copy link

@gelotte gelotte commented Jun 29, 2019

A very simple implementation of a cast sender application, for Chromecast and other receivers. The only working button in the control bar is the PlayToggle. This can serve as a base for further implementation of the components required by projects.

I made a CastPlayToggle component that wraps the regular PlayToggle so the easiest implementation is to disable the default controls and use this one instead. Another way is to let the CastPlayToggle handle only the "remote player" and switching between controls in the ControlBar component instead, i.e.

<PlayToggle disabled={this.state.isCasting} />
<CastPlayToggle disabled={!this.state.isCasting} />

Let me know if anyone has thoughts about other approaches to the local/remote player controls and state display.

@codecov-io
Copy link

Codecov Report

Merging #293 into master will increase coverage by 0.44%.
The diff coverage is 92.3%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #293      +/-   ##
==========================================
+ Coverage   48.04%   48.48%   +0.44%     
==========================================
  Files          47       48       +1     
  Lines        1278     1291      +13     
==========================================
+ Hits          614      626      +12     
- Misses        664      665       +1
Impacted Files Coverage Δ
src/components/control-bar/CastPlayToggle.js 92.3% <92.3%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4c31635...49ce194. Read the comment docs.

@mondaychen
Copy link
Member

Thank you for submitting such a high-quality PR! However, at this moment I don't think it's a good idea to add a component in video-react just for Google Cast since it's not widely used. Video-react is highly customizable at many levels, so users can add the feature to support Google Cast by creating their own components.

It will be nice if you can keep it a simple and clear doc page (like https://video-react.js.org/customize/customize-source/) demonstrating this use case.

@stolinski
Copy link

I respectfully disagree that Google Cast is not widely used. It's extremely popular and expected functionality.

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.

None yet

4 participants