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

Copy in-app links as YouTube links #2951

Merged
merged 2 commits into from
Dec 13, 2022

Conversation

absidue
Copy link
Member

@absidue absidue commented Dec 12, 2022

Copy in-app links as YouTube links

Pull Request Type

  • Feature Implementation

Related issue

closes #2479

Description

Currently all links in FreeTube, in-app and external ones, have the "Copy Link" context menu entry. This PR changes it to only show up on external links and certain in-app ones, to make it useful on the in-app ones it also transforms them into their YouTube equivalents. This means that you can copy the link to a video just by right clicking on it in the search results.

Testing

  1. Right click on menu items in the side bar, "Copy Link" should only show up for the list of subscribed channels
  2. Right click on videos, channels and playlists in channels, search results, playlists, up next and trending
  3. Right click on channel name and image on the watch and playlist pages

Desktop

  • OS: Windows
  • OS Version: 10
  • FreeTube version: 0.18.0

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) December 12, 2022 17:54
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Dec 12, 2022
}

return [{
label: 'Copy Lin&k',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing the & is a typo?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it makes that context menu entry activatable by clicking on it or pressing the k key while the context menu is open. As electron-context-menu does the same (https://github.com/sindresorhus/electron-context-menu/blob/main/index.js#L167) I thought it would be a good idea to copy it, so that the behaviour is the same (I should probably add it for the Select All option as well).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh cool, didnt know that

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add code comment for it :S

src/main/index.js Outdated Show resolved Hide resolved
@PikachuEXE
Copy link
Collaborator

Copy Link or Copy Youtube Link (and later might add Copy Invidious Link)?

@absidue
Copy link
Member Author

absidue commented Dec 13, 2022

I can definitely do Copy Link for external links, Copy YouTube Link for in-app links that can be converted to YouTube links (will hide the Copy Link one for those).

However doing an Invidious one is not going to be clean. The current Invidious instance is stored in the individual renderer processes, each renderer process (browser window) can have a diffferent invidious instance, unlike renderer to main ipc, you can't send replies when doing main to renderer, the only option is to send a separate ipc message to a different handler, so not what we want for the context menu.

@PikachuEXE
Copy link
Collaborator

In that case Copy Link is good enough

@FreeTubeBot FreeTubeBot merged commit c6299e6 into FreeTubeApp:development Dec 13, 2022
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Dec 13, 2022
@absidue absidue deleted the better-link-copying branch December 13, 2022 09:40
@ChunkyProgrammer
Copy link
Member

I can definitely do Copy Link for external links, Copy YouTube Link for in-app links that can be converted to YouTube links (will hide the Copy Link one for those).

However doing an Invidious one is not going to be clean. The current Invidious instance is stored in the individual renderer processes, each renderer process (browser window) can have a diffferent invidious instance, unlike renderer to main ipc, you can't send replies when doing main to renderer, the only option is to send a separate ipc message to a different handler, so not what we want for the context menu.

For invidious, you could just need to use the link "https://redirect.invidious.io/", that way anyone that goes to that link can choose which instance they want 🙂

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.

[Bug]: copied channel link is a local url that can't be shared
5 participants