fix(ui): improve tagging ux (#784)

* fix(ui): always reset tag search panel when opened * feat: return parent tags in tag search Known issue: this bypasses the tag_limit * refactor: use consistant `datetime` imports * refactor: sort by base tag name to improve performance * fix: escape `&` when displaying tag names * ui: show "create and add" tag with other results * fix: optimize and fix tag result sorting * feat(ui): allow tags in list to be selected and added by keyboard * ui: use `esc` to reset search focus and/or close modal * fix(ui): add pressed+focus styling to "create tag" button * ui: use `esc` key to close `PanelWidget` * ui: move disambiguation button to right side * ui: expand clickable area of "-" tag button, improve styling * ui: add "Ctrl+M" shortcut to open tag manager * fix(ui): show "add tags" window title when accessing from home
2026-01-28 22:01:24 +00:00 · 2025-02-03 16:15:40 -08:00
parent 480328b83b
commit dbf7353bdf
11 changed files with 189 additions and 121 deletions
--- a/tagstudio/src/core/library/alchemy/library.py
+++ b/tagstudio/src/core/library/alchemy/library.py
@@ -31,6 +31,7 @@ from sqlalchemy import (
    func,
    or_,
    select,
+    text,
    update,
 )
 from sqlalchemy.exc import IntegrityError
@@ -70,6 +71,18 @@ from .visitors import SQLBoolExpressionBuilder

 logger = structlog.get_logger(__name__)

+TAG_CHILDREN_QUERY = text("""
+-- Note for this entire query that tag_parents.child_id is the parent id and tag_parents.parent_id is the child id due to bad naming
+WITH RECURSIVE ChildTags AS (
+    SELECT :tag_id AS child_id
+    UNION ALL
+    SELECT tp.parent_id AS child_id
+	FROM tag_parents tp
+    INNER JOIN ChildTags c ON tp.child_id = c.child_id
+)
+SELECT * FROM ChildTags;
+""")  # noqa: E501
+

 def slugify(input_string: str) -> str:
    # Convert to lowercase and normalize unicode characters
@@ -752,10 +765,7 @@ class Library:

            return res

-    def search_tags(
-        self,
-        name: str | None,
-    ) -> list[Tag]:
+    def search_tags(self, name: str | None) -> list[set[Tag]]:
        """Return a list of Tag records matching the query."""
        tag_limit = 100

@@ -775,8 +785,23 @@ class Library:
                    )
                )

-            tags = session.scalars(query)
-            res = list(set(tags))
+            direct_tags = set(session.scalars(query))
+            ancestor_tag_ids: list[Tag] = []
+            for tag in direct_tags:
+                ancestor_tag_ids.extend(
+                    list(session.scalars(TAG_CHILDREN_QUERY, {"tag_id": tag.id}))
+                )
+
+            ancestor_tags = session.scalars(
+                select(Tag)
+                .where(Tag.id.in_(ancestor_tag_ids))
+                .options(selectinload(Tag.parent_tags), selectinload(Tag.aliases))
+            )
+
+            res = [
+                direct_tags,
+                {at for at in ancestor_tags if at not in direct_tags},
+            ]

            logger.info(
                "searching tags",
--- a/tagstudio/src/core/library/alchemy/models.py
+++ b/tagstudio/src/core/library/alchemy/models.py
@@ -2,7 +2,7 @@
 # Licensed under the GPL-3.0 License.
 # Created for TagStudio: https://github.com/CyanVoxel/TagStudio

-import datetime as dt
+from datetime import datetime as dt
 from pathlib import Path

 from sqlalchemy import JSON, ForeignKey, ForeignKeyConstraint, Integer, event
@@ -185,9 +185,9 @@ class Entry(Base):

    path: Mapped[Path] = mapped_column(PathType, unique=True)
    suffix: Mapped[str] = mapped_column()
-    date_created: Mapped[dt.datetime | None]
-    date_modified: Mapped[dt.datetime | None]
-    date_added: Mapped[dt.datetime | None]
+    date_created: Mapped[dt | None]
+    date_modified: Mapped[dt | None]
+    date_added: Mapped[dt | None]

    tags: Mapped[set[Tag]] = relationship(secondary="tag_entries")

@@ -222,9 +222,9 @@ class Entry(Base):
        folder: Folder,
        fields: list[BaseField],
        id: int | None = None,
-        date_created: dt.datetime | None = None,
-        date_modified: dt.datetime | None = None,
-        date_added: dt.datetime | None = None,
+        date_created: dt | None = None,
+        date_modified: dt | None = None,
+        date_added: dt | None = None,
    ) -> None:
        self.path = path
        self.folder = folder
--- a/tagstudio/src/core/library/alchemy/visitors.py
+++ b/tagstudio/src/core/library/alchemy/visitors.py
@@ -23,7 +23,7 @@ else:
 logger = structlog.get_logger(__name__)

 # TODO: Reevaluate after subtags -> parent tags name change
-CHILDREN_QUERY = text("""
+TAG_CHILDREN_ID_QUERY = text("""
 -- Note for this entire query that tag_parents.child_id is the parent id and tag_parents.parent_id is the child id due to bad naming
 WITH RECURSIVE ChildTags AS (
    SELECT :tag_id AS child_id
@@ -151,7 +151,7 @@ class SQLBoolExpressionBuilder(BaseVisitor[ColumnElement[bool]]):
                return tag_ids
            outp = []
            for tag_id in tag_ids:
-                outp.extend(list(session.scalars(CHILDREN_QUERY, {"tag_id": tag_id})))
+                outp.extend(list(session.scalars(TAG_CHILDREN_ID_QUERY, {"tag_id": tag_id})))
            return outp

    def __entry_has_all_tags(self, tag_ids: list[int]) -> ColumnElement[bool]:
--- a/tagstudio/src/core/utils/refresh_dir.py
+++ b/tagstudio/src/core/utils/refresh_dir.py
@@ -1,6 +1,6 @@
-import datetime as dt
 from collections.abc import Iterator
 from dataclasses import dataclass, field
+from datetime import datetime as dt
 from pathlib import Path
 from time import time

@@ -42,7 +42,7 @@ class RefreshDirTracker:
                    path=entry_path,
                    folder=self.library.folder,
                    fields=[],
-                    date_added=dt.datetime.now(),
+                    date_added=dt.now(),
                )
                for entry_path in self.files_not_in_library
            ]
--- a/tagstudio/src/qt/helpers/escape_text.py
+++ b/tagstudio/src/qt/helpers/escape_text.py
@@ -0,0 +1,8 @@
+# Copyright (C) 2025 Travis Abendshien (CyanVoxel).
+# Licensed under the GPL-3.0 License.
+# Created for TagStudio: https://github.com/CyanVoxel/TagStudio
+
+
+def escape_text(text: str):
+    """Escapes characters that are problematic in Qt widgets."""
+    return text.replace("&", "&&")
--- a/tagstudio/src/qt/modals/build_tag.py
+++ b/tagstudio/src/qt/modals/build_tag.py
@@ -386,6 +386,16 @@ class BuildTagPanel(PanelWidget):
        else:
            text_color = get_text_color(primary_color, highlight_color)

+        # Add Tag Widget
+        tag_widget = TagWidget(
+            tag,
+            library=self.lib,
+            has_edit=False,
+            has_remove=True,
+        )
+        tag_widget.on_remove.connect(lambda t=parent_id: self.remove_parent_tag_callback(t))
+        row.addWidget(tag_widget)
+
        # Add Disambiguation Tag Button
        disam_button = QRadioButton()
        disam_button.setObjectName(f"disambiguationButton.{parent_id}")
@@ -412,6 +422,15 @@ class BuildTagPanel(PanelWidget):
            f"QRadioButton::hover{{"
            f"border-color: rgba{highlight_color.toTuple()};"
            f"}}"
+            f"QRadioButton::pressed{{"
+            f"background: rgba{border_color.toTuple()};"
+            f"color: rgba{primary_color.toTuple()};"
+            f"border-color: rgba{primary_color.toTuple()};"
+            f"}}"
+            f"QRadioButton::focus{{"
+            f"border-color: rgba{highlight_color.toTuple()};"
+            f"outline:none;"
+            f"}}"
        )

        self.disam_button_group.addButton(disam_button)
@@ -421,18 +440,7 @@ class BuildTagPanel(PanelWidget):
        disam_button.clicked.connect(lambda checked=False: self.toggle_disam_id(parent_id))
        row.addWidget(disam_button)

-        # Add Tag Widget
-        tag_widget = TagWidget(
-            tag,
-            library=self.lib,
-            has_edit=False,
-            has_remove=True,
-        )
-
-        tag_widget.on_remove.connect(lambda t=parent_id: self.remove_parent_tag_callback(t))
-        row.addWidget(tag_widget)
-
-        return disam_button, tag_widget.bg_button, container
+        return tag_widget.bg_button, disam_button, container

    def toggle_disam_id(self, disambiguation_id: int | None):
        if self.disambiguation_id == disambiguation_id:
--- a/tagstudio/src/qt/modals/tag_search.py
+++ b/tagstudio/src/qt/modals/tag_search.py
@@ -7,8 +7,9 @@ import typing

 import src.qt.modals.build_tag as build_tag
 import structlog
+from PySide6 import QtCore, QtGui
 from PySide6.QtCore import QSize, Qt, Signal
-from PySide6.QtGui import QColor, QShowEvent
+from PySide6.QtGui import QShowEvent
 from PySide6.QtWidgets import (
    QFrame,
    QHBoxLayout,
@@ -26,10 +27,6 @@ from src.qt.translations import Translations
 from src.qt.widgets.panel import PanelModal, PanelWidget
 from src.qt.widgets.tag import (
    TagWidget,
-    get_border_color,
-    get_highlight_color,
-    get_primary_color,
-    get_text_color,
 )

 logger = structlog.get_logger(__name__)
@@ -85,12 +82,7 @@ class TagSearchPanel(PanelWidget):
        self.root_layout.addWidget(self.search_field)
        self.root_layout.addWidget(self.scroll_area)

-    def __build_row_item_widget(self, tag: Tag):
-        container = QWidget()
-        row = QHBoxLayout(container)
-        row.setContentsMargins(0, 0, 0, 0)
-        row.setSpacing(3)
-
+    def __build_tag_widget(self, tag: Tag):
        has_remove_button = False
        if not self.is_tag_chooser:
            has_remove_button = tag.id not in range(RESERVED_TAG_START, RESERVED_TAG_END)
@@ -115,53 +107,9 @@ class TagSearchPanel(PanelWidget):
        #     )
        # )

-        row.addWidget(tag_widget)
-
-        primary_color = get_primary_color(tag)
-        border_color = (
-            get_border_color(primary_color)
-            if not (tag.color and tag.color.secondary)
-            else (QColor(tag.color.secondary))
-        )
-        highlight_color = get_highlight_color(
-            primary_color
-            if not (tag.color and tag.color.secondary)
-            else QColor(tag.color.secondary)
-        )
-        text_color: QColor
-        if tag.color and tag.color.secondary:
-            text_color = QColor(tag.color.secondary)
-        else:
-            text_color = get_text_color(primary_color, highlight_color)
-
-        if self.is_tag_chooser:
-            add_button = QPushButton()
-            add_button.setMinimumSize(22, 22)
-            add_button.setMaximumSize(22, 22)
-            add_button.setText("+")
-            add_button.setStyleSheet(
-                f"QPushButton{{"
-                f"background: rgba{primary_color.toTuple()};"
-                f"color: rgba{text_color.toTuple()};"
-                f"font-weight: 600;"
-                f"border-color: rgba{border_color.toTuple()};"
-                f"border-radius: 6px;"
-                f"border-style:solid;"
-                f"border-width: 2px;"
-                f"padding-bottom: 4px;"
-                f"font-size: 20px;"
-                f"}}"
-                f"QPushButton::hover"
-                f"{{"
-                f"border-color: rgba{highlight_color.toTuple()};"
-                f"color: rgba{primary_color.toTuple()};"
-                f"background: rgba{highlight_color.toTuple()};"
-                f"}}"
-            )
-            tag_id = tag.id
-            add_button.clicked.connect(lambda: self.tag_chosen.emit(tag_id))
-            row.addWidget(add_button)
-        return container
+        tag_id = tag.id
+        tag_widget.bg_button.clicked.connect(lambda: self.tag_chosen.emit(tag_id))
+        return tag_widget

    def build_create_tag_button(self, query: str | None):
        """Constructs a Create Tag Button."""
@@ -187,7 +135,7 @@ class TagSearchPanel(PanelWidget):
            f"font-weight: 600;"
            f"border-color:{get_tag_color(ColorType.BORDER, TagColorEnum.DEFAULT)};"
            f"border-radius: 6px;"
-            f"border-style:solid;"
+            f"border-style:dashed;"
            f"border-width: 2px;"
            f"padding-right: 4px;"
            f"padding-bottom: 1px;"
@@ -197,6 +145,15 @@ class TagSearchPanel(PanelWidget):
            f"QPushButton::hover{{"
            f"border-color:{get_tag_color(ColorType.LIGHT_ACCENT, TagColorEnum.DEFAULT)};"
            f"}}"
+            f"QPushButton::pressed{{"
+            f"background: {get_tag_color(ColorType.LIGHT_ACCENT, TagColorEnum.DEFAULT)};"
+            f"color: {get_tag_color(ColorType.PRIMARY, TagColorEnum.DEFAULT)};"
+            f"border-color: {get_tag_color(ColorType.PRIMARY, TagColorEnum.DEFAULT)};"
+            f"}}"
+            f"QPushButton::focus{{"
+            f"border-color: {get_tag_color(ColorType.LIGHT_ACCENT, TagColorEnum.DEFAULT)};"
+            f"outline:none;"
+            f"}}"
        )

        create_button.clicked.connect(lambda: self.create_and_add_tag(query))
@@ -221,6 +178,7 @@ class TagSearchPanel(PanelWidget):

            self.tag_chosen.emit(tag.id)
            self.search_field.setText("")
+            self.search_field.setFocus()
            self.update_tags()

        self.build_tag_modal: BuildTagPanel = build_tag.BuildTagPanel(self.lib)
@@ -235,32 +193,44 @@ class TagSearchPanel(PanelWidget):

    def update_tags(self, query: str | None = None):
        logger.info("[Tag Search Super Class] Updating Tags")
+
        # TODO: Look at recycling rather than deleting and re-initializing
        while self.scroll_layout.count():
            self.scroll_layout.takeAt(0).widget().deleteLater()
-        tag_results = self.lib.search_tags(name=query)
-        if len(tag_results) > 0:
-            results_1 = []
-            results_2 = []
-            for tag in tag_results:
-                if tag.id in self.exclude:
-                    continue
-                elif query and tag.name.lower().startswith(query.lower()):
-                    results_1.append(tag)
-                else:
-                    results_2.append(tag)
-            results_1.sort(key=lambda tag: self.lib.tag_display_name(tag.id))
-            results_1.sort(key=lambda tag: len(self.lib.tag_display_name(tag.id)))
-            results_2.sort(key=lambda tag: self.lib.tag_display_name(tag.id))
-            self.first_tag_id = results_1[0].id if len(results_1) > 0 else tag_results[0].id
-            for tag in results_1 + results_2:
-                self.scroll_layout.addWidget(self.__build_row_item_widget(tag))
-        else:
-            # If query doesnt exist add create button
+
+        query_lower = "" if not query else query.lower()
+        tag_results: list[set[Tag]] = self.lib.search_tags(name=query)
+        tag_results[0] = {t for t in tag_results[0] if t.id not in self.exclude}
+        tag_results[1] = {t for t in tag_results[1] if t.id not in self.exclude}
+
+        results_0 = list(tag_results[0])
+        results_0.sort(key=lambda tag: tag.name.lower())
+        results_1 = list(tag_results[1])
+        results_1.sort(key=lambda tag: tag.name.lower())
+        raw_results = list(results_0 + results_1)[:100]
+        priority_results: set[Tag] = set()
+        all_results: list[Tag] = []
+
+        if query and query.strip():
+            for tag in raw_results:
+                if tag.name.lower().startswith(query_lower):
+                    priority_results.add(tag)
+
+        all_results = sorted(list(priority_results), key=lambda tag: len(tag.name)) + [
+            r for r in raw_results if r not in priority_results
+        ]
+
+        if all_results:
            self.first_tag_id = None
+            self.first_tag_id = all_results[0].id if len(all_results) > 0 else all_results[0].id
+            for tag in all_results:
+                self.scroll_layout.addWidget(self.__build_tag_widget(tag))
+        else:
+            self.first_tag_id = None
+
+        if query and query.strip():
            c = self.build_create_tag_button(query)
            self.scroll_layout.addWidget(c)
-        self.search_field.setFocus()

    def on_return(self, text: str):
        if text:
@@ -276,11 +246,22 @@ class TagSearchPanel(PanelWidget):
            self.parentWidget().hide()

    def showEvent(self, event: QShowEvent) -> None:  # noqa N802
-        if not self.is_initialized:
-            self.update_tags()
-            self.is_initialized = True
+        self.update_tags()
+        self.search_field.setText("")
+        self.search_field.setFocus()
        return super().showEvent(event)

+    def keyPressEvent(self, event: QtGui.QKeyEvent) -> None:  # noqa N802
+        # When Escape is pressed, focus back on the search box.
+        # If focus is already on the search box, close the modal.
+        if event.key() == QtCore.Qt.Key.Key_Escape:
+            if self.search_field.hasFocus():
+                self.parentWidget().hide()
+            else:
+                self.search_field.setFocus()
+                self.search_field.selectAll()
+        return super().keyPressEvent(event)
+
    def remove_tag(self, tag: Tag):
        pass

--- a/tagstudio/src/qt/ts_qt.py
+++ b/tagstudio/src/qt/ts_qt.py
@@ -294,7 +294,9 @@ class QtDriver(DriverMixin, QObject):
        # Initialize the main window's tag search panel
        self.tag_search_panel = TagSearchPanel(self.lib, is_tag_chooser=True)
        self.add_tag_modal = PanelModal(
-            self.tag_search_panel, Translations.translate_formatted("tag.add.plural")
+            widget=self.tag_search_panel,
+            title=Translations.translate_formatted("tag.add.plural"),
+            window_title=Translations.translate_formatted("tag.add.plural"),
        )
        self.tag_search_panel.tag_chosen.connect(
            lambda t: (
@@ -485,6 +487,13 @@ class QtDriver(DriverMixin, QObject):
        tag_database_action = QAction(menu_bar)
        Translations.translate_qobject(tag_database_action, "menu.edit.manage_tags")
        tag_database_action.triggered.connect(lambda: self.show_tag_database())
+        tag_database_action.setShortcut(
+            QtCore.QKeyCombination(
+                QtCore.Qt.KeyboardModifier(QtCore.Qt.KeyboardModifier.ControlModifier),
+                QtCore.Qt.Key.Key_M,
+            )
+        )
+        save_library_backup_action.setStatusTip("Ctrl+M")
        edit_menu.addAction(tag_database_action)

        # View Menu ============================================================
--- a/tagstudio/src/qt/widgets/panel.py
+++ b/tagstudio/src/qt/widgets/panel.py
@@ -4,6 +4,7 @@
 import logging
 from typing import Callable

+from PySide6 import QtCore, QtGui
 from PySide6.QtCore import Qt, Signal
 from PySide6.QtWidgets import QHBoxLayout, QLabel, QPushButton, QVBoxLayout, QWidget
 from src.qt.translations import Translations
@@ -125,3 +126,11 @@ class PanelWidget(QWidget):

    def add_callback(self, callback: Callable, event: str = "returnPressed"):
        logging.warning(f"add_callback not implemented for {self.__class__.__name__}")
+
+    def keyPressEvent(self, event: QtGui.QKeyEvent) -> None:  # noqa N802
+        if event.key() == QtCore.Qt.Key.Key_Escape:
+            if self.panel_cancel_button:
+                self.panel_cancel_button.click()
+        else:  # Other key presses
+            pass
+        return super().keyPressEvent(event)
--- a/tagstudio/src/qt/widgets/tag.py
+++ b/tagstudio/src/qt/widgets/tag.py
@@ -19,6 +19,7 @@ from PySide6.QtWidgets import (
 from src.core.library import Tag
 from src.core.library.alchemy.enums import TagColorEnum
 from src.core.palette import ColorType, get_tag_color
+from src.qt.helpers.escape_text import escape_text
 from src.qt.translations import Translations

 logger = structlog.get_logger(__name__)
@@ -127,9 +128,9 @@ class TagWidget(QWidget):
        self.bg_button = QPushButton(self)
        self.bg_button.setFlat(True)
        if self.lib:
-            self.bg_button.setText(self.lib.tag_display_name(tag.id))
+            self.bg_button.setText(escape_text(self.lib.tag_display_name(tag.id)))
        else:
-            self.bg_button.setText(tag.name)
+            self.bg_button.setText(escape_text(tag.name))
        if has_edit:
            edit_action = QAction(self)
            edit_action.setText(Translations.translate_formatted("generic.edit"))
@@ -150,10 +151,10 @@ class TagWidget(QWidget):

        self.inner_layout = QHBoxLayout()
        self.inner_layout.setObjectName("innerLayout")
-        self.inner_layout.setContentsMargins(2, 2, 2, 2)
+        self.inner_layout.setContentsMargins(0, 0, 0, 0)

        self.bg_button.setLayout(self.inner_layout)
-        self.bg_button.setMinimumSize(22, 22)
+        self.bg_button.setMinimumSize(44, 22)

        primary_color = get_primary_color(tag)
        border_color = (
@@ -189,6 +190,15 @@ class TagWidget(QWidget):
            f"QPushButton::hover{{"
            f"border-color: rgba{highlight_color.toTuple()};"
            f"}}"
+            f"QPushButton::pressed{{"
+            f"background: rgba{highlight_color.toTuple()};"
+            f"color: rgba{primary_color.toTuple()};"
+            f"border-color: rgba{primary_color.toTuple()};"
+            f"}}"
+            f"QPushButton::focus{{"
+            f"border-color: rgba{highlight_color.toTuple()};"
+            f"outline:none;"
+            f"}}"
        )
        self.bg_button.setMinimumHeight(22)
        self.bg_button.setMaximumHeight(22)
@@ -201,16 +211,34 @@ class TagWidget(QWidget):
            self.remove_button.setText("–")
            self.remove_button.setHidden(True)
            self.remove_button.setStyleSheet(
+                f"QPushButton{{"
                f"color: rgba{primary_color.toTuple()};"
                f"background: rgba{text_color.toTuple()};"
                f"font-weight: 800;"
-                f"border-radius: 3px;"
-                f"border-width:0;"
+                f"border-radius: 5px;"
+                f"border-width: 4;"
+                f"border-color: rgba(0,0,0,0);"
                f"padding-bottom: 4px;"
                f"font-size: 14px"
+                f"}}"
+                f"QPushButton::hover{{"
+                f"background: rgba{primary_color.toTuple()};"
+                f"color: rgba{text_color.toTuple()};"
+                f"border-color: rgba{highlight_color.toTuple()};"
+                f"border-width: 2;"
+                f"border-radius: 6px;"
+                f"}}"
+                f"QPushButton::pressed{{"
+                f"background: rgba{border_color.toTuple()};"
+                f"color: rgba{highlight_color.toTuple()};"
+                f"}}"
+                f"QPushButton::focus{{"
+                f"background: rgba{border_color.toTuple()};"
+                f"outline:none;"
+                f"}}"
            )
-            self.remove_button.setMinimumSize(18, 18)
-            self.remove_button.setMaximumSize(18, 18)
+            self.remove_button.setMinimumSize(22, 22)
+            self.remove_button.setMaximumSize(22, 22)
            self.remove_button.clicked.connect(self.on_remove.emit)

        if has_remove:
--- a/tagstudio/tests/test_library.py
+++ b/tagstudio/tests/test_library.py
@@ -119,7 +119,7 @@ def test_tag_search(library):
    assert library.search_tags(tag.name.lower())
    assert library.search_tags(tag.name.upper())
    assert library.search_tags(tag.name[2:-2])
-    assert not library.search_tags(tag.name * 2)
+    assert library.search_tags(tag.name * 2) == [set(), set()]


 def test_get_entry(library: Library, entry_min):