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

[WIP] Android support #51

Merged
merged 8 commits into from
Sep 28, 2016
Merged

[WIP] Android support #51

merged 8 commits into from
Sep 28, 2016

Conversation

jusierra
Copy link
Collaborator

@jusierra jusierra commented May 19, 2016

Developement of android support for YouTube UI component :

Issue concerned : #35
How is it going :

  • Display native YouTube player
  • Get Props & native events
  • Some props still to do
  • Improve native events
  • Clean the seekTo code
  • Test
  • Test a lot
  • RNPM support

@jtuchsen
Copy link

jtuchsen commented May 19, 2016

Hey, I've tried out your pull request and it looks like the nested YouTubeView inside the YouTubePlayerFragment isn't being rendered, are you running into this issue?

I think the issue may be that React Native handles the layout of Android views in a non traditional way. I've hacked around this in the past by using a ViewTreeObserver to register a global layout listener and then manually calling measure and layout on the inner view. Doing it that way is a bit of a hack though because the view is going to redraw on every render call the app makes, even if its props haven't changed. I have a hunch a better way to do it probably exists, but documentation for React Native is pretty sparse.

Thanks for working on Android support, a side project of mine is sitting unfinished because of this.

@jusierra
Copy link
Collaborator Author

Hi @jtuchsen, I do have this problem of 0,0 size view and PLAYER_TOO_SMALL error as well time to time (reload and it should work ...). I already tried the ViewTreeObserver method without success for the moment. I'm still working on it. ;)

@jusierra
Copy link
Collaborator Author

I'm also facing another problem : apparently there is problems of compatibility between YouTube app and the android YouTube API.

For me the issue is that I am able to play the video only once but after that YouTubePlayer doesn't play any video and I hope there are many other people who are also facing similar issues with the YouTubeAndroidPlayerAPI. I think the latest youtube app (version 10.37.58) and YouTubeAndroidPlayerAPI 1.2.1 are not compatible.

They speak about here or here.

So I'm still looking for some solution but if anybody has a solution, let me know !

@jtuchsen
Copy link

jtuchsen commented May 20, 2016

You can trigger the rendering bug I was referring to by returning only the YouTube component from the render method in Javascript. I'm pretty sure that React Native is ignoring the inner Java YouTubeView's requestLayout calls, or more likely batching them in a way that the YouTubeView doesn't expect. If you use the Example/index.ios.js file the bug wont be triggered because it renders itself twice, once initially and then again after onReady is called, which somehow renders the player correctly. So forcing a render from Javascript after onReady is called seems to be enough to fix it. I still don't know the cause of the issue though.

I think the PLAYER_TOO_SMALL issue may be a separate one entirely, I'm not getting that error at all.

Loading different videos into the player works for me as well! I loaded a 20 second video and waited until it finished, then loaded in another one and it worked. I also can restart a video after it's finished without any issues at all. My version of YouTube is 11.16.62, so maybe it was a temporary bug that got resolved?

@jusierra
Copy link
Collaborator Author

Using the Example/index.ios.js works indeed. I had a last problem of ProgressBar always displayed but I found a way to remove it.

@Mokto Mokto mentioned this pull request May 27, 2016
@adeebbasheer
Copy link

Hi @jujumoz, have you solved the PLAYER_TOO_SMALL issue ? Is there any solution for that?

@aitorMarrero
Copy link

no commits since 26 May :(

@jusierra
Copy link
Collaborator Author

Hi, I did find a solution which works on my side. Can you try the last commit ?
I added an index.android.js as exemple and a temporary android support README.md.
Please tell me how is it going !

@InfamousVague
Copy link

InfamousVague commented Jul 7, 2016

@jujumoz Super excited for this functionality, Looks great so far! 👍

Although for whatever reason when the player hit the onReady state its size grows to not fit the screen, and fixes itself after the component is updated. This was fixed by calling forceUpdate onReady.

@osoriano
Copy link

Has anyone got this working on react-native 0.29.0 or higher?
Looks like this issue needs to be resolved first: facebook/react-native#8661

@adeebbasheer
Copy link

@jujumoz last commit is working perfectly...awesome.. 👍

Option for cueVideo will be great in next commit

@jusierra
Copy link
Collaborator Author

@wski : sorry I didn't understand your problem, can you explain again ?
@osoriano : I didn't try with version 0.29, which version did you use @adeebbasheer to make it work ?
@adeebbasheer : thanks for testing and cuevideo will arrive ;)

@adeebbasheer
Copy link

i tried with 0.26, to work in 0.29 some slight changes is needed

@InfamousVague
Copy link

@jujumoz I think my initial bug was simply from trying to create a new component using the player before the old component had unmounted which was causing a crash in the player.

The only issue I'm having now (which very well could be my own fault) is sometimes the scrub bar extends the entire width of the player overlapping the numbers, In addition the three dots menu doesn't work.

However forceUpdating the component seems to fix the scrubbing issue, and full screening then minimizing the player makes the three dots clickable again.

@jusierra
Copy link
Collaborator Author

Apparently no solution was found yet nor commited to get the activity in 0.29. Wait and see ...

@mikelambert
Copy link
Contributor

I'm curious how to estimate the status of this. The first post says 'some more props and native events' which look like nice-to-have features, but the rest of this thread discusses bugs? What bugs are still open at this point? Is it just the 0.29 issue at this point? Or is there some initialization workaround still necessary?

As far as 0.29, looks like it should be fully fixed by facebook/react-native#9310 , which will be released in 0.33. Which if my math is correct, should be released the week of Sept 5th. So I guess this is blocked until then? :/

@mikelambert
Copy link
Contributor

FYI @jujumoz , 0.33 is released and live now, and I believe should have unblocked this problem introduced with 0.29.

(Eventually I'll get around to patching and fixing this up for my purposes with 0.33 and Android. But I'm swamped with other things at the moment...)

@jusierra
Copy link
Collaborator Author

Thanks @mikelambert , I will check 0.33 asap but I have no time to go ahead lately.

@cjmling
Copy link
Contributor

cjmling commented Sep 19, 2016

waiting waiting waiting , keep calm and wait :D

@mikelambert
Copy link
Contributor

So I've gotten it working again with RN 0.33:
https://github.com/mikelambert/react-native-youtube/tree/android

Outstanding issues:

  • I probably broke seekTo when simplifying a bunch of code, need to test and make that work again
  • I still sometimes see @wski's bug: "the scrub bar extends the entire width of the player overlapping the numbers"

@sibelius
Copy link

@inProgress-team any changes this will get into master of this repo?

after the first version we can send some PRs to fix some bugs

@jusierra
Copy link
Collaborator Author

7fc24d3
For 0.33 support guys !

@jusierra jusierra merged commit 8d74207 into master Sep 28, 2016
@jusierra jusierra deleted the android-support branch February 24, 2017 17:17
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.

10 participants