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: correct behavior for tag widget search options #398

Merged
merged 6 commits into from
Aug 31, 2024
Merged

Conversation

CyanVoxel
Copy link
Member

@CyanVoxel CyanVoxel commented Aug 25, 2024

This PR addresses three related issues:

  1. Subtags not working as expected? #211: "Search for Tag" context menu option + left-click behavior for tag widgets did not include tag clusters in results; only instances of that exact tag ID. Expected behavior is to include the entire inheritance cluster, and this has now been implemented. Additional commented-out code has been removed inside the search_library() method.
  2. "Add to Search" context menu option was non-functional. This option has been removed from the context menu.
  3. Undocumented issue found during testing: The "Search for Tag" context menu and left-click behavior was non-functional for tag widgets inside the "Tag Manager" panel. This functionality has been implemented.

I have not updated the display of tag IDs in the search bar as discussed in 211, as I feel this is much better suited for a future UI revamp of how tags are displayed in the search bar.

Closes #211.

Remove unused "Add to Search" context menu item for tag widgets.
Add missing functionality for the "Search for Tag" context menu option + left click functionality inside `tag_database.py` aka the "Tag Manager" panel.
@CyanVoxel CyanVoxel added Type: Bug Something isn't working as intended Type: UI/UX User interface and/or user experience Priority: Medium An issue that shouldn't be be saved for last TagStudio: Tags Relating to the TagStudio tag system Status: Review Needed A review of this is needed TagStudio: Search The TagStudio search engine labels Aug 25, 2024
@CyanVoxel CyanVoxel added this to the Alpha v9.4 (Pre-SQL) milestone Aug 25, 2024
Copy link
Collaborator

@eivl eivl left a comment

Choose a reason for hiding this comment

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

Approved when comments are resolved.

Comment on lines 1360 to 1361
tag_only_ids.append(int(query_words[0]))
tag_only_ids = tag_only_ids + self.get_tag_cluster(int(query_words[0]))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will query_words[0] always be a digit?
Use extend on tag_only_ids instead of adding two lists together.

Copy link
Member Author

Choose a reason for hiding this comment

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

Was about to confidently say "Yes, the tag_id: prefix is only triggered automatically by the program" and then realized that if the user typed in something like "tag_id: gotcha" it would fail...


class TagDatabasePanel(PanelWidget):
tag_chosen = Signal(int)

def __init__(self, library):
def __init__(self, library: "Library", driver: "QtDriver"):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Literal string "Library" and "QtDriver" as the type looks like 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.

The string literals are there to prevent NameErrors, since QtDriver is not actually imported at runtime (nor should Library be, just caught that) and is only under if typing.TYPE_CHECKING: for type checking. It's quirky, but it's what PEP 484 suggests

Comment on lines 77 to 78
# add_to_search_action = QAction("Add to Search", self)
# self.bg_button.addAction(add_to_search_action)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can these two lines be deleted instead?

@CyanVoxel CyanVoxel removed the Status: Review Needed A review of this is needed label Aug 31, 2024
@CyanVoxel CyanVoxel merged commit 569390e into Alpha-v9.4 Aug 31, 2024
8 checks passed
@CyanVoxel CyanVoxel removed the Priority: Medium An issue that shouldn't be be saved for last label Aug 31, 2024
@CyanVoxel CyanVoxel deleted the fix-211 branch September 1, 2024 20:39
CarterPillow pushed a commit to CarterPillow/TagStudio that referenced this pull request Sep 7, 2024
* fix(tags): include cluster in tag_id search

* fix(ui): remove unused tag context menu item

Remove unused "Add to Search" context menu item for tag widgets.

* fix(ui): add tag search from tag_database

Add missing functionality for the "Search for Tag" context menu option + left click functionality inside `tag_database.py` aka the "Tag Manager" panel.

* fix: verify `tag_id:` input in search

* style: remove commented code

* fix: change `Library` import to type check only
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TagStudio: Search The TagStudio search engine TagStudio: Tags Relating to the TagStudio tag system Type: Bug Something isn't working as intended Type: UI/UX User interface and/or user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants