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

Add hidden prop to ReactPlayer #102

Merged
merged 5 commits into from
Sep 2, 2016
Merged

Conversation

kevinzwhuang
Copy link
Contributor

@kevinzwhuang kevinzwhuang commented Sep 1, 2016

Description:
hidden is a standard HTML prop of react. I was building a toggle feature to hide ReactPlayer -- as opposed to re-rendering the player each toggle, which would lag due to having to retrieve and re-embed the media each time -- and found that ReactPlayer does not accept a hidden prop to hide its top level div. In order to hide it using hidden, I had to write another parent div above ReactPlayer just to hide the component.

// Not the best way to hide a component
<div hidden={ myBoolean }>
  <ReactPlayer url={ this.props.randomURL }>
</div>

Changes Proposed:
I'd like to propose that hidden be added to one of the standard props ReactPlayer accepts (alongside the standard style & className props). Like so:

// Feels better this way!
<ReactPlayer
  url={ this.props.randomURL }
  hidden={ myBoolean }
/>

I've included working code that allows this very use case in this PR. I've also included a line to the Props documentation in README.md about hidden as a prop that can hide ReactPlayer.

@kevinzwhuang
Copy link
Contributor Author

To verify full functionality that hidden works, you can try modifying the demo as such:

//src/demo/App.js
//...
//...
export default class App extends Component {
  componentDidMount() {
    var interval = setInterval(this.toggleHide, 1000); // Hide the player every 1000ms
  }
  toggleHide = () => this.setState({ hidden: !this.state.hidden }) // Toggle hidden state
//...
//...
  render () {
//...
//...
    return (
      <div className='app'>
        <section className='section'>
          <h1>ReactPlayer Demo</h1>
          <ReactPlayer
            ref='player'
            className='react-player'
            width={480}
            height={270}
            hidden={this.state.hidden} // Provide hidden state as a prop to ReactPlayer
//...
//...

@kevinzwhuang
Copy link
Contributor Author

Thoughts on this, @cookpete?

@cookpete
Copy link
Owner

cookpete commented Sep 2, 2016

Yeah this looks good. I would normally have just used className or style to hide the player but I wasn't aware of the hidden attribute. Nice work!

As this is adding new functionality, I'll publish as a new minor version. Should probably look at moving to 1.0.0 soon.

@cookpete cookpete merged commit 19a8f9c into cookpete:master Sep 2, 2016
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.

2 participants