Add "Delete Tag" menu option to tags in the Tag Database and tag box #105
Conversation
There was a problem hiding this comment.
Looks good overall but there are some guardrails for system tags that I think should be implemented to prevent errors.
Also Highlights some other refactoring that should probably happen in the tag_database.py file and further supports the use use of a constants file recommended in #88
| @@ -92,8 +92,9 @@ def update_tags(self, query:str): | |||
| l = QHBoxLayout(c) | |||
| l.setContentsMargins(0,0,0,0) | |||
| l.setSpacing(3) | |||
| tw = TagWidget(self.lib, self.lib.get_tag(tag_id), True, False) | |||
| tw = TagWidget(self.lib, self.lib.get_tag(tag_id), True, False, True) | |||
There was a problem hiding this comment.
Should probably remove the ability to delete "System Tags" (archived & favorite) since TagStudio recreates those tags on system shutdown if they are missing.
| tw = TagWidget(self.lib, self.lib.get_tag(tag_id), True, False, True) | |
| if tag_id in [0, 1]: # [0, 1] should probably be extracted as a constant during refactor | |
| tag_deletable = False | |
| else: | |
| tag_deletable = True | |
| tw = TagWidget(self.lib, self.lib.get_tag(tag_id), True, False, tag_deletable ) |
| @@ -106,8 +107,9 @@ def update_tags(self, query:str): | |||
| l = QHBoxLayout(c) | |||
| l.setContentsMargins(0,0,0,0) | |||
| l.setSpacing(3) | |||
| tw = TagWidget(self.lib, tag, True, False) | |||
| tw = TagWidget(self.lib, tag, True, False, True) | |||
There was a problem hiding this comment.
Should probably remove the ability to delete "System Tags" (archived & favorite) since TagStudio recreates those tags on system shutdown if they are missing.
| tw = TagWidget(self.lib, tag, True, False, True) | |
| if tag.id in [0, 1]: # [0, 1] should probably be extracted as a constant during refactor | |
| tag_deletable = False | |
| else: | |
| tag_deletable = True | |
| tw = TagWidget(self.lib, self.lib.get_tag(tag_id), True, False, tag_deletable ) |
| def delete_tag(self, tag_id:int): | ||
| def show_delete_prompt() -> bool: | ||
| result = QMessageBox.question(self, "Delete Tag", f"Are you sure you want to delete Tag {tag.name}?", | ||
| QMessageBox.Yes | QMessageBox.No) | ||
|
|
||
| return result == QMessageBox.Yes | ||
|
|
||
| tag = self.lib.get_tag(tag_id) | ||
| shift_held = Qt.KeyboardModifier.ShiftModifier in QApplication.keyboardModifiers() | ||
|
|
||
| if shift_held or show_delete_prompt(): | ||
| self.lib.remove_tag(tag_id) | ||
| self.update_tags() | ||
|
|
||
| if not shift_held: | ||
| QMessageBox.information(self, "Delete Tag", "Tag deleted.") |
There was a problem hiding this comment.
Having to replicate this here and in tag_database makes me think there should be a more reusable way to implement these dialogs but I think that will be a whole codebase sweep that needs to happen so I don't think that's an issue for this PR.
There was a problem hiding this comment.
I considered putting this function in a place where I could reuse it without repeating myself. the other event listeners don't do this either though so I chose to be consistent with them
tagstudio/src/qt/widgets/tag_box.py
Outdated
| tw.on_click.connect(lambda checked=False, q=f'tag_id: {tag}': (self.driver.main_window.searchField.setText(q), self.driver.filter_items(q))) | ||
| tw.on_remove.connect(lambda checked=False, t=tag: (self.remove_tag(t))) | ||
| tw.on_edit.connect(lambda checked=False, t=tag: (self.edit_tag(t))) | ||
| tw = TagWidget(self.lib, self.lib.get_tag(tag_id), True, True, True) |
There was a problem hiding this comment.
Should probably remove the ability to delete "System Tags" (archived & favorite) since TagStudio recreates those tags on system shutdown if they are missing.
| tw = TagWidget(self.lib, self.lib.get_tag(tag_id), True, True, True) | |
| if tag_id in [0, 1]: # [0, 1] should probably be extracted as a constant during refactor | |
| tag_deletable = False | |
| else: | |
| tag_deletable = True | |
| tw = TagWidget(self.lib, self.lib.get_tag(tag_id), True, False, tag_deletable ) |
|
Would it be possible to add something like a reactive store to handle updates of the tags and similar structures instead of pushing signals back and forth. This would relax the coupling between the TagDatabase and the TagWidget. It would also make it easier to listen to such changes in other places, like under the image preview in the main tag browser. However, I am more of a Frontend Dev than a python dev. |
|
I implemented your suggestions that system tags can't be deleted now |
CyanVoxel
added
TagStudio: Library
CyanVoxel
removed
the
Priority: Medium
|
Thank you for the work put into this PR, but unfortunately due to the significant library backend changes in #332 I'll have to close this as it's no longer compatible. I apologize for the long turnaround time and and lack of input on my side about this. #569 is currently taking up the task of implementing this feature. |
I added this menu option to the tag database and tag boxes. It will prompt the user and if they say yes, delete the tags and remove all references to it
One thing I added is that when you hold shift while clicking Delete Tag, the prompt and popup after it was deleted will be skipped. I figured this would be useful when deleting a lot of tags at the same time.