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

fix(QtDriver): redundant search override #247

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

career-yashaswee
Copy link

fix(QtDriver): redundant search override

Description:

This pull request addresses an issue where the filter_items function was being called unnecessarily in the QtDriver class. This function filters assets based on a search term.

Expected Behavior:

When the user enters a search term and clicks the search button or presses Enter:
If the search term hasn't changed (i.e., it's the same as the previous search), filter_items shouldn't be called again.
This check should be bypassed during window initialisation to ensure all assets are initially visible.

Previous Behavior:
filter_items was called regardless of whether the search term had changed, leading to redundant filtering.

Changes Introduced:
A new variable, previous_search_string, is introduced.
When the user performs a search:previous_search_string is set to the current search term.

Before calling filter_items:
The code checks if previous_search_string is the same as the current search term.
If they are the same and the window is not initializing, filter_itemsis skipped.
An override flag is added for functions that call filter_items during window initialisation to bypass the check and ensure all assets are displayed.

Benefits:

  • Improves performance by avoiding unnecessary filtering.
  • Enhances user experience by preventing redundant filtering actions.

Testing:
Manual testing has been conducted to ensure the fix functions as intended.

  • Additional unit tests can be added to further solidify the fix.

Please review the code changes and provide feedback.

filter items on same search string override
tagstudio/src/qt/ts_qt.py Outdated Show resolved Hide resolved

def filter_items(self, query: str = ""):

def filter_items(self, query: str = "", override: bool = True):
if self.lib:
Copy link
Collaborator

Choose a reason for hiding this comment

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

While you're touching the code already, it could be worth to flip this condition as well. There's no else: block, so instead of this:

if self.lib:
    # all the code

it could be done as this:

if not self.lib:
    return

# the same code except it's less indented now and you dont have to scroll 50 lines to see what will happen (spoiler: nothing) if the condition `if self.lib:` is not fulfilled 

@YoyloNerd
Copy link
Contributor

You also fully broke some stuff from the main branch when merging that affects search

@CyanVoxel CyanVoxel added the Type: Bug Something isn't working as intended label Jun 4, 2024
@CyanVoxel CyanVoxel added the Status: Changes Requested Changes are quested to this label Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Changes Requested Changes are quested to this Type: Bug Something isn't working as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants