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 "share at current time" button to the main video player #2271

Merged
merged 5 commits into from
Apr 11, 2019

Conversation

Stypox
Copy link
Member

@Stypox Stypox commented Apr 6, 2019

The button is placed in the upper right corner, under "more settings". No changes has been made to the popup video player (as for other things, there is no space for a share button).
main player share button
The share button generates this text (for the video above at the current time):

TEST VIDEO
https://www.youtube.com/watch?v=C0DPdy98e4c&t=3

This implements #2000, but it does not give a chance of choosing whether to share the url normally or to at the current time. But as discussed in #2000 this is not a problem because there are already many other ways to do that.

The share utilities (i.e. shareUrl() and openUrlInBrowser()) were placed under the BaseStateFragment class. But since they have nothing to do with fragments, I moved them to a separate utilities class: utils.ShareUtils. This way they can be accessed from anywhere. I could not find any way to access them from the MainVideoPlayer, using the previous setup.

I tested the feature both from a phone (Huawei P9 lite, where the used layout is normal) and from a tablet (Amazon Kindle, where the layout is large-land, I think). Also, since I moved the sharing functions, I tested the "share" buttons in the app and they work as they did before.

Please note that this is my first pull request ever, so if I did something wrong I'm sorry :-)

@TobiGr
Copy link
Contributor

TobiGr commented Apr 6, 2019

Thanks! Code looks good so far. Will test the changes later this week

@TobiGr
Copy link
Contributor

TobiGr commented Apr 6, 2019

Please note that this is my first pull request ever, so if I did something wrong I'm sorry :-)

Everything is fine :) It is perfect to have small commits. This allows us to review the changes faster 👍

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