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 setting for opening URLs passed to FreeTube in a new window #6242

Merged
merged 7 commits into from
Dec 3, 2024

Conversation

OothecaPickle
Copy link
Contributor

@OothecaPickle OothecaPickle commented Nov 29, 2024

Add setting for opening URLs passed to FreeTube in a new window

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Related issue

closes #1501
also relates to #6237

Description

Adds a toggle under General Settings to open freetube:// links in a new window rather than replacing the contents of an existing window.

Screenshots

openlinksinnewwindow

Testing

Test that freetube:// links opened from other applications are opened in a new window when the added setting is toggled on.

Desktop

  • OS: macOS
  • OS Version: 11
  • FreeTube version: e646a0f
    &
  • OS: Windows
  • OS Version: Windows 10 21H2
  • FreeTube version: e646a0f

Additional context

I used some code from @pakoito's #5592.

@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Nov 29, 2024
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 29, 2024 10:38
@PikachuEXE
Copy link
Collaborator

Are you talking about https://www.electronjs.org/docs/latest/tutorial/launch-app-from-url-in-another-app#overview ?
Maybe the preference name should be updated first

Also I am not sure how/if the doc mentions deep links and if users understand what is freetube:// links

@OothecaPickle
Copy link
Contributor Author

OothecaPickle commented Nov 29, 2024

@PikachuEXE

Are you talking about https://www.electronjs.org/docs/latest/tutorial/launch-app-from-url-in-another-app#overview ?

Yes.

Maybe the preference name should be updated first
Also I am not sure how/if the doc mentions deep links and if users understand what is freetube:// links

I agree, but I wasn't sure what else to name it. Do you have any suggestions?

src/renderer/App.js Outdated Show resolved Hide resolved
@PikachuEXE
Copy link
Collaborator

I found one place mentioning extensions but not sure how to call it
https://docs.freetubeapp.io/usage/browser-extension/

It shouldn't be called "YouTube links" on the preference coz there are many "YT" links in video description/comment etc.

Inviting more members to comment on this

@pakoito
Copy link

pakoito commented Nov 30, 2024

In Android land these are called "Intents" rather than "Links". But tbh I'd be sad if this feature is blocked on a label :D

@OothecaPickle OothecaPickle marked this pull request as draft December 1, 2024 09:54
auto-merge was automatically disabled December 1, 2024 09:54

Pull request was converted to draft

@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Dec 1, 2024
@OothecaPickle OothecaPickle marked this pull request as ready for review December 1, 2024 11:33
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) December 1, 2024 11:33
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Dec 1, 2024
src/main/index.js Outdated Show resolved Hide resolved
auto-merge was automatically disabled December 1, 2024 12:43

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) December 1, 2024 12:43
@OothecaPickle
Copy link
Contributor Author

Sorry to bother @pakoito, didn't mean to request another review from you.

@@ -283,6 +283,7 @@ Settings:
Fallback to Non-Preferred Backend on Failure: Fallback to Non-Preferred Backend
on Failure
Enable Search Suggestions: Enable Search Suggestions
Open Links In New Window: Open freetube:// Links in a New Window
Copy link
Contributor Author

@OothecaPickle OothecaPickle Dec 1, 2024

Choose a reason for hiding this comment

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

Suggested change
Open Links In New Window: Open freetube:// Links in a New Window
Open Links In New Window: Open URLs Passed to FreeTube in a New Window

Copy link
Contributor Author

@OothecaPickle OothecaPickle Dec 1, 2024

Choose a reason for hiding this comment

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

or maybe "URLs Passed to FreeTube" as this setting also applies to links passed via the command line

Copy link
Member

Choose a reason for hiding this comment

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

I have no objection to changing it to URLs Passed to FreeTube because that does sound clearer. If others think the messaging doesnt sound clear enough then maybe we should opt for implementing an tool tip message that explains what this does?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or just add tooltip giving examples of redirect extensions (mostly from those right?
Setting External Link Handling has one

@absidue absidue removed the request for review from pakoito December 1, 2024 15:09
@efb4f5ff-1298-471a-8973-3d47447115dc

could someone provide a build for me to test this one 😬

@OothecaPickle
Copy link
Contributor Author

could someone provide a build for me to test this one 😬

@efb4f5ff-1298-471a-8973-3d47447115dc https://github.com/OothecaPickle/FreeTube/actions/runs/12128622135

@efb4f5ff-1298-471a-8973-3d47447115dc

PR LGTM, only thing that should be looked at is the wording of the setting

src/renderer/store/modules/settings.js Outdated Show resolved Hide resolved
@@ -283,6 +283,7 @@ Settings:
Fallback to Non-Preferred Backend on Failure: Fallback to Non-Preferred Backend
on Failure
Enable Search Suggestions: Enable Search Suggestions
Open Links In New Window: Open freetube:// Links in a New Window
Copy link
Collaborator

Choose a reason for hiding this comment

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

Or just add tooltip giving examples of redirect extensions (mostly from those right?
Setting External Link Handling has one

auto-merge was automatically disabled December 3, 2024 02:32

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) December 3, 2024 02:32
auto-merge was automatically disabled December 3, 2024 03:09

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) December 3, 2024 03:10
@PikachuEXE
Copy link
Collaborator

You committed when I am about to comment on it lol

@OothecaPickle OothecaPickle changed the title Add setting for opening freetube:// links in new window Add setting for opening URLs passed to FreeTube in a new window Dec 3, 2024
PikachuEXE
PikachuEXE previously approved these changes Dec 3, 2024
Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

Did not test actual link open on mac but someone else tested it already

Copy link
Member

Choose a reason for hiding this comment

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

Tooltip wording change

static/locales/en-US.yaml Outdated Show resolved Hide resolved
auto-merge was automatically disabled December 3, 2024 05:26

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) December 3, 2024 05:26
@FreeTubeBot FreeTubeBot merged commit eecb261 into FreeTubeApp:development Dec 3, 2024
10 checks passed
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Dec 3, 2024
Soham456 pushed a commit to Soham456/FreeTube that referenced this pull request Dec 5, 2024
…TubeApp#6242)

* open freetube:// links in new window

* add setting for opening freetube:// links in new window

* fix two windows being opened when link launches freetube

* use multiple parameters instead of destructuring assignment

* change setting name & add tooltip

* minor correction

* change tooltip wording
jlvivero pushed a commit to jlvivero/FreeTube that referenced this pull request Dec 7, 2024
…TubeApp#6242)

* open freetube:// links in new window

* add setting for opening freetube:// links in new window

* fix two windows being opened when link launches freetube

* use multiple parameters instead of destructuring assignment

* change setting name & add tooltip

* minor correction

* change tooltip wording
SuperAKWA pushed a commit to SuperAKWA/FreeTube that referenced this pull request Jan 24, 2025
…TubeApp#6242)

* open freetube:// links in new window

* add setting for opening freetube:// links in new window

* fix two windows being opened when link launches freetube

* use multiple parameters instead of destructuring assignment

* change setting name & add tooltip

* minor correction

* change tooltip wording
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.

Add option to always open a redirected video in a new FreeTube window / instance
6 participants