abbasfreestyle / react-native-af-video-player

MIT License
381 stars 288 forks source link

logo prop required in `Controls` and `TopBar`? #63

Open pandigita opened 6 years ago

pandigita commented 6 years ago

Hi there, first of all, thank you for this wonderful, well functioning, well fullscreening video player! I'm strangely getting a prop type error, claiming logo is required, when the docs and common logic clearly suggest it's not required. Both the Controls and the TopBar component seem to require it.

Both in the console and the YellowBox: "Warning: Failed prop type: The prop logo is marked as required in Controls, but its value is undefined." and "Warning: Failed prop type: The prop logo is marked as required in TopBar, but its value is undefined."

The video player works as expected, however, and I'm simply ignoring the warning, like so: console.ignoredYellowBox = ['Warning: Failed prop type: The prop logo is marked as required'];

jorgeluisrezende commented 6 years ago

I already had fixed this issue, if you want, you can use the project om my profile, i still will up that on npm and give suport of my fork, please give me a start to keep me motivated on this project

kodayashi commented 6 years ago

@jorgeluisrezende - I ran into this as well. In your comment above are you asking for someone to submit a PR with a suggested fix?

jorgeluisrezende commented 6 years ago

Well, i had fixed this, i'm was asking for someone to help to continue working in this project, i had made a fork and already mad some changes. theres a lot of work to do...

kodayashi commented 6 years ago

All good @jorgeluisrezende, I just suppressed the issue for now. I will try to reach back out next month, maybe I can help :)

kmausolf commented 5 years ago

I just want to say that my partner and I are finding your library to be quite nice. Regarding this issue, there is a discrepancy between the PropTypes directives of Controls.js and Video.js; specifically the required fields. The only required field in Video is url, and I would imagine these to match. If you would like, I can submit a pull request with this change.