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

Update top nav search input to open new window on shift click/enter #2427

Conversation

PikachuEXE
Copy link
Collaborator

Pull Request Type
Please select what type of pull request this is:

  • Feature Implementation

Related issue
N/A

Description
Opening a new window, loading channel and start searching seems wasting time
Browsers have search bar to open search in new tab/window already

Top nav search bar updated to open new window on shift click (on search/go button) or shift enter (on input bar)

Clicking search suggestion with shift still won't open a new window (Can be changed if needed)

Screenshots (if appropriate)

Screen.Recording.2022-07-31.at.11.00.54.mov

Testing (for code that is not small enough to be easily understandable)
Enter

  • Random search string
  • Channel/playlist/video URL

And perform

  • Shift click on search/go button
  • Shift + Enter

Desktop (please complete the following information):

  • OS: MacOS
  • OS Version: 12.1
  • FreeTube version: 9fa4a5a

Additional context
Add any other context about the problem here.

@PrestonN PrestonN enabled auto-merge (squash) July 31, 2022 03:02
@absidue absidue added the PR: waiting for review For PRs that are complete, tested, and ready for review label Jul 31, 2022
absidue
absidue previously approved these changes Aug 7, 2022
Copy link
Member

@absidue absidue left a comment

Choose a reason for hiding this comment

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

Should this be documented in the docs?

@PikachuEXE
Copy link
Collaborator Author

Sure!
Just not sure where to put it...
Let me study

@PikachuEXE
Copy link
Collaborator Author

There is no doc about the main nav search (coz not much to document right now)
Probably need a new doc?

@absidue absidue added PR: merge conflicts / rebase needed and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Aug 9, 2022
@absidue
Copy link
Member

absidue commented Aug 9, 2022

Please fix the merge conflicts

@PikachuEXE
Copy link
Collaborator Author

@absidue Done~

@PikachuEXE PikachuEXE added PR: waiting for review For PRs that are complete, tested, and ready for review and removed PR: merge conflicts / rebase needed labels Aug 9, 2022
Copy link
Member

@absidue absidue left a comment

Choose a reason for hiding this comment

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

Looks good.

Tested it with various video, channel and playlist URLs, as well as a bunch of search queries

@absidue
Copy link
Member

absidue commented Aug 9, 2022

Not sure about the docs, maybe on the keyboard shortcuts page we could have a small section with special ones?

@PikachuEXE
Copy link
Collaborator Author

Will update doc after this is all working

@PikachuEXE
Copy link
Collaborator Author

Hmmm what's holding this PR's review?

@absidue
Copy link
Member

absidue commented Aug 21, 2022

@ChunkyProgrammer @efb4f5ff-1298-471a-8973-3d47447115dc When you have time could you please review this PR? :)

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

Testing (for code that is not small enough to be easily understandable)
Enter

Random search string
Channel/playlist/video URL
And perform

Shift click on search/go button
Shift + Enter

Im not sure if im testing this right everything looks to do the same as before.

@PikachuEXE
Copy link
Collaborator Author

I am using this (in my custom build but tested in dev too) on windows and macos
Not sure what makes it not working...

@absidue
Copy link
Member

absidue commented Aug 28, 2022

It worked when I tested it, the biggest change since then was the Electron version upgrade, so that might have broken this PR.

@PikachuEXE
Copy link
Collaborator Author

@absidue I am using my custom build with electron 20 included (but when tested with dev it was electron 16)
You can merge dev branch after checking out this branch to see if that makes it work...

@PikachuEXE
Copy link
Collaborator Author

I just tested with yarn dev (with old electron 16 that means)
Both Shift + Enter + Shift + button click opens a new window for me

@absidue
Copy link
Member

absidue commented Aug 29, 2022

@efb4f5ff-1298-471a-8973-3d47447115dc It works fine for me too, even when I rebase on the development branch. The new window is opened right on top of the old one, so it might be difficult to tell when a new window has been opened.

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes it was difficult to spot that it opened a new window. User error on my end. LGTM

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.

5 participants