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

[Issue #2487] Add YouTube share link option below the video #2610

Merged
merged 3 commits into from
Feb 2, 2024

Conversation

chonsser
Copy link
Contributor

Hi there! I'm happy to contribute to this project. I'd love (if time allows) to work more on this project.

In this first PR, I've added a new YouTube share button (solves issue #2487).
Also, based on my own experience (running ViewTube via non-HTTPS environment), I added a code to handle the copy operation on that environment (notifies the user that the copy operation has failed).

* Add YouTube share button
* Handle non-HTTPS environment in copy operation
@moisout
Copy link
Member

moisout commented Feb 1, 2024

Hi, awesome to have you here!

Your code looks great, the only thing I would suggest is to use the ViewTube message system instead of alert(). This will show a dismissible toast on the top right.

You can check out an example of how to use the messagesStore in the About.vue component.
https://github.com/ViewTube/viewtube/blob/development/client/components/About.vue#L16
Instead of type: 'info' you can use type: 'error' to show an error.

@chonsser
Copy link
Contributor Author

chonsser commented Feb 1, 2024

Thanks for the hints! I was searching for a toast component but I couldn't find one so I thought that there's no such component yet 😅. Applied your suggestions, thank you!

@moisout
Copy link
Member

moisout commented Feb 2, 2024

Awesome!

@all-contributors add @chonsser for code

Copy link
Contributor

@moisout

I've put up a pull request to add @chonsser! 🎉

@moisout moisout merged commit daab248 into ViewTube:development Feb 2, 2024
5 checks passed
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