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

Added youtube window in the app to view trailers without intent to youtube #107

Closed
wants to merge 2 commits into from

Conversation

Hemant27031999
Copy link

INITIAL STATE
To see the trailers of a movie, the intent was made to the youtube app and the user was watching the trailer there.

WHAT I ADDED
I have added a youtube player window in the app itself. Now upon clicking on a trailer, the user can see the trailer in the app itself without being sent to the youtube app. He can see the trailer in the full window also if he wants.


MovieGuideYoutubePlayer

EXTRA CHANGES MADE
I have changed the FAB button's position in movie_details fragment because the youtube player cannot play any video if some view is floating/placed above it. So I have put it in the top right end between toolbar and fragment screen.

Also, right now I have my own youtube API in the Config file which I am sharing, please make sure to replace it with your own.

@esoxjem
Copy link
Owner

esoxjem commented Dec 4, 2019

Hi @Hemant27031999, this looks good. Could you please rebase from master and resolve the conflicts, thanks.

@Hemant27031999
Copy link
Author

Hello @esoxjem , I have resolved the conflicts. Is it fine now? Thanks.

@esoxjem
Copy link
Owner

esoxjem commented Dec 4, 2019

Though your changes lead to a good user experience, there are a couple of issues with this approach:

  1. All users will use your API key since it is hard coded in Config.
  2. One way to avoid hardcoding would be add this to local.properties as done for TMDB's API key. Unfortunately, doing so makes the project harder to use as everyone would have to setup a Google project and generate an API key before they can use this repo. I would like to avoid this additional overhead at this point.

For changes in the behaviour of the app, I would advise to create an issue on GitHub first where it can be discussed prior to implementation. This would help in avoiding any effort that is not in sync with the roadmap of this project.

@esoxjem esoxjem closed this Dec 4, 2019
@Hemant27031999
Copy link
Author

Ya @esoxjem, you have a point. Thanks.

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

2 participants