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

onLoad #14

Closed
YeYinhai opened this issue Jan 3, 2017 · 7 comments
Closed

onLoad #14

YeYinhai opened this issue Jan 3, 2017 · 7 comments
Assignees

Comments

@YeYinhai
Copy link

YeYinhai commented Jan 3, 2017

OnLoad with BUG
Causes the Load icon to appear

@kylemilloy
Copy link
Contributor

Can you send me a bit of details? What version of React Native, what version of react-native-video-controls, etc.

@kylemilloy kylemilloy self-assigned this Jan 3, 2017
@YeYinhai
Copy link
Author

YeYinhai commented Jan 4, 2017

onLoadStart={ () => {} } // Fired when loading of the source starts
onProgress={ () => {} } // Fired every ~250ms when the video progresses
onError={ () => {} } // Fired when an error is encountered on load
onBack={ () => {} } // Function fired when back button is pressed.
onLoad={ () => {} } // Fired when loading is complete
onEnd={ () => {} } // Fired when the video is complete.

    this.events = {
        onLoadStart: this.props.onLoadStart || this._onLoadStart.bind( this ),
        onProgress: this.props.onProgress || this._onProgress.bind( this ),
        onError: this.props.onError || this._onError.bind( this ),
        onLoad: this.props.onLoad || this._onLoad.bind( this ),
        onEnd: this.props.onEnd || this._onEnd.bind( this ),
        onScreenPress: this._onScreenPress.bind( this ),
    };

RN0.39.2 android6.0.1 react-native-video-controls 1.1.1

I saw your code.
All custom events will replace the events you wrote
Appear BUG

@kylemilloy
Copy link
Contributor

So you're overwriting the onLoad function with a blank function?

@YeYinhai
Copy link
Author

YeYinhai commented Jan 6, 2017

Yes。
Custom events should not replace the original event。

@kylemilloy
Copy link
Contributor

Removed onLoad events from being editable in 1.2.x

@FuadBalashov
Copy link

@kylemilloy I realize I am late to this party but I think that removing the ability to pass an onLoad is not a good idea. Could I get your thoughts on this?

I just started looking at this library after working with react-native-video and need to use the onLoad to be able to get the the width and height of the video that is loaded so I can resize the component to fill the screen width and scale the height based on the aspect ratio. That being said I think it makes sense to not remove the original onLoad event since your component relies on the logic in there.

Based on this I would suggest that your onLoad calls the onLoad passed in with the props with the arguments it receives. (you may want to do this with all of the callback props that were removed) Thoughts?

Id be happy to contribute with a PR based on what we decide.

@kylemilloy
Copy link
Contributor

kylemilloy commented Apr 11, 2017

Pull away, my son. Thanks for the help. Yeah that'd work I think. Easy enough to just do something like

// original onLoad
_onLoad() {
 // do original stuff
 this.props.onLoad(); 
}

Would probably be good to do some checking to ensure the prop onLoad is a fx

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

No branches or pull requests

3 participants