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.

Sign up for GitHub

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

Jump to bottom

Add Search feature to WebUI #9758

Merged
merged 5 commits into from
Dec 9, 2018
Merged

Add Search feature to WebUI #9758

merged 5 commits into from
Dec 9, 2018

Conversation

Piccirello
Copy link
Member

@Piccirello Piccirello commented Oct 25, 2018 •
edited
Loading

This PR is a WebUI implementation of the search API from #8584.

It currently supports a single search job, but supporting multiple concurrent searches here and in the GUI is on my eventual todo list. My current ugly hack for concurrent searches is to open a new browser tab for each search.

screen shot 2018-10-25 at 12 02 13 am

screen shot 2018-10-25 at 12 03 10 am

Closes #859, closes #8107, closes #9154

@Piccirello Piccirello added the WebUI WebUI-related issues/changes label Oct 25, 2018
@glassez glassez requested a review from Chocobo1 October 29, 2018 12:29
@rebekah65
Copy link

@Piccirello
This is looking really good, when is it lightly to be committed.

@Piccirello
Copy link
Member Author

@Chocobo1 Please review when you have a chance. It's a large PR but a very widely requested feature.

Chocobo1
Chocobo1 previously approved these changes Nov 28, 2018
View reviewed changes
Copy link
Member

@Chocobo1 Chocobo1 left a comment

Choose a reason for hiding this comment

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

LGTM, I haven't got time to test it yet...

glassez
glassez previously approved these changes Nov 29, 2018
View reviewed changes
Copy link
Member

@glassez glassez left a comment

Choose a reason for hiding this comment

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

Let's start with this!

@glassez
Copy link
Member

glassez commented Nov 29, 2018

@Chocobo1, since this feature doesn't affect the main qBittorrent code, I think there's no point in postponing it until some major update, right?

@Chocobo1
Copy link
Member

wait... I just noticed there are 10 new alerts on LGTM, @Piccirello would you mind fix it before we merge it? Seems easy to fix.

@Piccirello Piccirello dismissed stale reviews from glassez and Chocobo1 via 4efe4d5 November 30, 2018 05:11
@Piccirello
Copy link
Member Author

Fixed the LGTM alerts and also ensured the search api doesn't get hit (and therefore SearchPluginManager instantiated) unless the search tab is enabled, per @sledgehammer999's recommendations and #9889.

@Chocobo1 Chocobo1 mentioned this pull request Dec 3, 2018
Merged
@Piccirello Piccirello force-pushed the new-search-webui-2 branch 2 times, most recently from 0c90d3b to 6782410 Compare December 7, 2018 22:47
@Piccirello
Copy link
Member Author

Pushed a minor change to make another PR I'm working on easier to implement. This PR is ready for final review.

glassez
glassez reviewed Dec 8, 2018
View reviewed changes
Copy link
Member

@glassez glassez left a comment

Choose a reason for hiding this comment

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

@Chocobo1, please proceed without me here.

@AceLewis AceLewis mentioned this pull request Dec 8, 2018
Open
61 tasks
@Chocobo1 Chocobo1 merged commit 2eb213e into qbittorrent:master Dec 9, 2018
@Chocobo1
Copy link
Member

Chocobo1 commented Dec 9, 2018

👍

@Piccirello Piccirello deleted the new-search-webui-2 branch July 15, 2019 00:44
@Piccirello Piccirello mentioned this pull request Mar 23, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glassez glassez glassez left review comments

@Chocobo1 Chocobo1 Chocobo1 approved these changes

Assignees
No one assigned
Labels
WebUI WebUI-related issues/changes
Projects
None yet
Milestone
No milestone
4 participants
@Piccirello @rebekah65 @glassez @Chocobo1