Skip to content

Conversation

@poseidon-rises
Copy link
Contributor

Made tag name empty by default and added placeholder "New Tag" text.

@CyanVoxel CyanVoxel added Type: QoL A quality of life (QoL) enhancement or suggestion Type: UI/UX User interface and/or user experience Priority: Low Doesn't require immediate attention TagStudio: Tags Relating to the TagStudio tag system Status: Review Needed A review of this is needed labels Nov 18, 2024
@CyanVoxel CyanVoxel added this to the Alpha v9.5 (Post-SQL) milestone Nov 18, 2024
@poseidon-rises poseidon-rises changed the title [Feat]: Make the create tag panel have empty tag name field feat: Make the create tag panel have empty tag name field Nov 18, 2024
@python357-1
Copy link
Collaborator

Seems like it works as intended

Copy link
Collaborator

@Computerdores Computerdores left a comment

Choose a reason for hiding this comment

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

Looks good!

@poseidon-rises poseidon-rises marked this pull request as draft November 18, 2024 20:22
@poseidon-rises
Copy link
Contributor Author

I can't get the signals to activate, can anyone take a look?

@CyanVoxel CyanVoxel added the Status: Help Wanted Extra attention is needed label Nov 18, 2024
@poseidon-rises
Copy link
Contributor Author

If anyone could spare debugging time that would be greatly appreciated.

@python357-1
Copy link
Collaborator

I'm not exactly sure if this means anything (@CyanVoxel might know better), but I keep getting the message "add_callback not implemented for BuildTagPanel". However, it is defined on the object it inherits from, but defined to always return that message:
(qt/widgets/panel.py, lines 110-111)

def add_callback(self, callback: Callable, event: str = "returnPressed"):
        logging.warning(f"add_callback not implemented for {self.__class__.__name__}")

@VasigaranAndAngel
Copy link
Collaborator

VasigaranAndAngel commented Nov 19, 2024

If anyone could spare debugging time that would be greatly appreciated.

using signals for this isn't the best approach. because we'd need to connect the signal every time BuildTagPanel is created. it's being created from tag_box (which show tags in the preview panel and has the signals are connected right now. when you right click a tag in the preview panel to edit it, the save buttons works as expected.), ts_qt (for creating new tags via the menu bar or shortcut) and tag_database (used for managing tags).

BuildTagPanel inherits from PanelWidget and then it's passed to PanelModal as the widget parameter. this is how the widgets are constructed right now.

so, instead of using signals, you can access the buttons of PanelModal by adding new class variables in PanelWidget:

class PanelWidget(QWidget):
    """Used for widgets that go in a modal panel, ex. for editing or searching."""

    done = Signal()
    panel_save_button: QPushButton | None = None
    panel_cancel_button: QPushButton | None = None
    panel_done_button: QPushButton | None = None

then assign the buttons to those variables in PanelModal
image
now it's easy to access the buttons
image

here is a better on_name_changed:

    def on_name_changed(self):
        is_empty = not self.name_field.text().strip()

        self.name_field.setStyleSheet(
            "border: 1px solid red; border-radius: 2px" if is_empty else ""
        )

        if self.panel_save_button is not None:
            self.panel_save_button.setDisabled(is_empty)

i've added variables to cancel and done buttons also. that's not necessary.

Additionally,

  • i'm not sure why you have remove line 212 tag.name = self.name_field.text() in src\qt\modals\build_tag.py? it will break if that line is not there. pytest also failing because of that.
  • set the fixed height of the self.name_field to 24 to fix the flickering when adding and removing the border.

let me know if i'm not explained correctly.

@python357-1
Copy link
Collaborator

Yeah @Cool-Game-Dev I did this locally and it works perfectly. Just copy what he has and commit it, and this issue will be done.
@VasigaranAndAngel Thanks so much for the fix!

@poseidon-rises
Copy link
Contributor Author

Ok, should be done! Thanks for the help!

@poseidon-rises poseidon-rises marked this pull request as ready for review November 19, 2024 16:35
Copy link
Member

@CyanVoxel CyanVoxel left a comment

Choose a reason for hiding this comment

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

Looks fantastic! This perfectly balances the benefits of having a default tag name for quick creation and quick lookup with the benefit of being able to immediately start typing a desired tag name. I've just got a couple additional suggestions here, but once those are addressed then this is good to be merged!
Thank you @Cool-Game-Dev and everyone else who's helped!

@CyanVoxel CyanVoxel removed the Status: Help Wanted Extra attention is needed label Nov 19, 2024
poseidon-rises and others added 2 commits November 19, 2024 15:48
Co-authored-by: Travis Abendshien <46939827+CyanVoxel@users.noreply.github.com>
Co-authored-by: Travis Abendshien <46939827+CyanVoxel@users.noreply.github.com>
@poseidon-rises
Copy link
Contributor Author

Both of those seem great! Thanks everyone for the help!

@CyanVoxel CyanVoxel merged commit 7ae2bc2 into TagStudioDev:main Nov 19, 2024
5 checks passed
@CyanVoxel
Copy link
Member

Thank you all again for your work on this PR!

yedpodtrzitko pushed a commit to yedpodtrzitko/TagStudio that referenced this pull request Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority: Low Doesn't require immediate attention TagStudio: Tags Relating to the TagStudio tag system Type: QoL A quality of life (QoL) enhancement or suggestion Type: UI/UX User interface and/or user experience

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

[Feature Request]: Make the create tag panel have empty tag name field

6 participants