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

feat(ui): sort tags in add tag panel by color/alphabetical (close #327) #329

Merged

Conversation

samuellieberman
Copy link
Contributor

Sort tags in the Library Tags panel and the Add Parent Tags panel with Archived and Favorite at the top, then sort by color, and then sort alphabetically.

This is what the panels look like now when you have many tags of different colors:
image
image

Sort tags in the Library Tags panel and the Add Parent Tags panel with Archived and Favorite at the top, then sort by color, and then sort alphabetically.
@CyanVoxel CyanVoxel changed the title Implement #327 feat(ui): sort tags in add tag panel by color/alphabetical (close #327) Jul 22, 2024
@CyanVoxel CyanVoxel added Type: Enhancement New feature or request Type: UI/UX User interface and/or user experience TagStudio: Tags Relating to the TagStudio tag system Status: Review Needed A review of this is needed TagStudio: Search The TagStudio search engine labels Jul 22, 2024
@CyanVoxel
Copy link
Member

This default order for the tag boxes looks great! While I personally don't use color to categorize tags, this is still much more useful than simply the creation order. Just a couple thoughts on the implementation:

  1. Should Favorite and Archived tags be given special treatment at the top? I feel it would make more sense for them to follow the new sorting rules as well. This also introduces the question of precedent when it comes to future built-in tags - if 10 more built-in tags were added, would they all be sorted at the top? If this was done due to you not wanting them to appear in other color groups you're using for categorization, you could also remove the colors from the built-in tags to keep them separate. While these aren't designed to be deleted, they are designed to be modified.

  2. I'm not sure if this behavior is desired in search results. It feels a bit odd to have results broken up by color in this context, when it would make more sense to keep things strictly alphabetical here. That being said, I didn't realize how bad the default tag search result order was...

@samuellieberman
Copy link
Contributor Author

samuellieberman commented Jul 23, 2024

Should Favorite and Archived tags be given special treatment at the top?

I didn't realize that you could change their color, or that this was intended behavior. I also thought that Favorite and Archived appearing at the top of the tag list was intentional. You are right that the built-in tags violate my categorization, since "Favorite" isn't an ideology and "Archived" isn't related to how easy it is to examine the file. If you prefer Favorite and Archived not to be given special treatment then I'll implement it like that, though without your suggestion I would not have considered that I could recolor the built-in tags to "Olive" or something.

Is this behavior desired in search results?

One useful aspect of tag searches is that they return child tags when searching parent tags. If you want to decide whether it makes sense to sort searches by color, then maybe it will be useful to consider the case when a user searches for a parent tag with lots of children.
image image

If search results are going to be left exhibiting the left kind of behavior for now, then the tag_limit for the panels should probably be increased to reduce the number of tags that can only be found with a search. In that case, sorting searches by color would not matter to me either way. I basically never have to search for results anyway with my system and with the new default order. I tend to color child tags identically to the color of their parent tags, so I can see the results of a parent tag search even before performing it.

It seem a bit strange to me to imagine a user that associates consistent meaning within tag colors, but who colors tags independently to their parent tags. But I believe that the decision here should be based on whether we should prioritize a user who has lots of different tag colors that may not carry meaning to them, or such a user who cares about tag colors, but colors independently of parent colors. As I said, if we increase the tag_limits then I wouldn't mind either way. What do you think makes sense?

@CyanVoxel
Copy link
Member

The way I see tag colors is that they can be used to assign optional visual meaning to tags, either by categorizing or by color association - but they should not be the de facto method for creating categories. That should be handled by a more powerful tag category system, something that's planned for the future and intended to replace the current "tag field" system with user-defined categories and automatic grouping based on those. With that being said though, I'd rather not step on the toes of people who use color for categorization if there's no need to. Color itself is still a useful way to recall and sort information, so I'm fine with it being an option here as long as it isn't the primary system for categorization going forward.

As for the tag_limit, this is in place for performance reasons. The long-term solution is to rebuild the tag panels using a recyclable MVC approach to effectively allow for infinite scrolling, but a quick short-term solution could be to just let the user select the tag limit themselves if they wish to trade more results for a performance hit. Maybe this could be done with a dropdown around the search bar with some predefined options such as "50, 100, 300, Unlimited", etc.

@CyanVoxel CyanVoxel added Status: Changes Requested Changes are quested to this and removed Status: Review Needed A review of this is needed labels Jul 25, 2024
@samuellieberman
Copy link
Contributor Author

Thank you for your questions on my code and my intent CyanVoxel, and I appreciate hearing more about your considerations for TagStudio.

To try and respond to your points, yes, absolutely. Color is not a good permanent default. Even if we have to choose a default in the future, much better options will show up, including tag categories no doubt. The main goal of this was to provide a temporary improvement to tag ordering without me having to muck around with UI. I'm fine with leaving the PR on color-alphabetical in every case, and I'm also fine with switching it to full alphabetical for searches if that's what you want. I haven't noticed any performance issues loading tags, but if you want alphabetical searches without increasing the limit right now then I don't mind that either.

Right now I'm wondering, is there anything that would have you consider a tag sort related PR like this acceptable without any corresponding UI changes?

@CyanVoxel
Copy link
Member

I would say that color-alphabetical is fine for the default view, with relevancy-alphabetical-color being the ideal default order for search results. The relevancy aspect (matches between name, aliases, and parent tags) could be tricky to get working in a satisfying way if you were to try it though. If anything I wouldn't mind pulling this PR if it just included the color-alphabetical view for non-search results.

The current 100 tag limit was decided by me here, where I gave a similar proposal for how it could be overcome in the future. Although if you feel comfortable with adding to the configuration .ini file (see the use of QSettings inside ts_qt.py), perhaps you could add a tag_search_limit entry to load from with the same default value while allowing you to increase it for yourself without touching any of the UI. In the future I could either incorporate this value into the UI somewhere or remove the need for it with a more efficient modal.

Let me know how you feel about the options here. Thank you for working on this and getting back!

@samuellieberman
Copy link
Contributor Author

Alright, I set it up so searches are color-alphabetical by default, and alphabetical-color when searching. I realize that the tag limit is probably a minor issue right now, so I just left it as is.

I have had some other ideas about searching tags since opening this PR, but I think that I will leave them for a later.

Please let me know if this PR looks good to you. Thanks!

@CyanVoxel
Copy link
Member

Looks good for the most part! Although the reason I suggested a relevancy-alphabetical-color order for search results is because an alphabetical-first approach can create the following issue where the most relevant result is moved further down the list due to the alphabetizing:
image
This breaks the "search, hit enter to add, search again" workflow at best and can bury relevant results off-screen at worst.

There's a few solutions to this I can think of:

  1. Separate the first returned result given by search_tags() from the alphabetizing process and keep it at the top of the results list (might cause some other relevant results to get buried)
  2. Separate any results whose names begin with the search string (probably with punctuation removed using strip_punctuation()) from the rest of the alphabetizing group, instead alphabetizing these separately and positioning this group first in the results (This might be the best way to go)
  3. Don't worry about sorting the search results and leave them to the previous default order

I'd also be willing to pitch into this PR if you'd like. I think the 2nd solution could provide a well-rounded order but I understand if you'd rather have this PR be done sooner and just stick to keeping the non-search order changes. This process has also gotten me thinking about further changes and features that are definitely outside the scope here, and I'd like to hear the ideas you have sometime as well!

I should also mention for posterity's sake that if the sorting logic is going to be the same between tag_database.py and tag_search.py, then it would be best to DRY it and break it out somewhere else - but the more I've thought where to put it the more I've considered it being moved inside the search_tags() method itself, which would be too complicated to tinker with right now in my opinion. This can be saved for the future when both the backend and frontend are revisited in more depth, just thought I aught to make note of it.

@samuellieberman
Copy link
Contributor Author

You're right, I haven't considered that issue. I implemented the second fix for that issue, so now tags whose actual names match the search text show up before the rest of the results.

If you can't find any more immediate issues we haven't talked about then I'd prefer to try to get this PR finished before either of us do too much new work on the tag search, including trying to refactor or create some sort of unified backend functionality.

I also want to note the results of search_tags() in the Library class is not the only place lists of tags can come from. I believe that we should consider the ordering of tags in entries before we start unifying tag sorting.

@CyanVoxel
Copy link
Member

Sounds good! I also didn't consider sources of tag lists from other than search_tags(), good call. I've got some ideas but yes, I think this PR is well off with what we've already discussed 😁

@CyanVoxel CyanVoxel removed the Status: Changes Requested Changes are quested to this label Jul 29, 2024
@CyanVoxel CyanVoxel changed the base branch from main to Alpha-v9.4 July 29, 2024 23:55
@CyanVoxel CyanVoxel merged commit 30b60a0 into TagStudioDev:Alpha-v9.4 Jul 29, 2024
4 checks passed
@yedpodtrzitko yedpodtrzitko mentioned this pull request Aug 23, 2024
26 tasks
CarterPillow pushed a commit to CarterPillow/TagStudio that referenced this pull request Sep 7, 2024
…StudioDev#327) (TagStudioDev#329)

* Implement TagStudioDev#327

Sort tags in the Library Tags panel and the Add Parent Tags panel with Archived and Favorite at the top, then sort by color, and then sort alphabetically.

* Sort tags alphabetically when a search is performed

* Format with Ruff

* Prioritize tags whose names match the query over tags that match the query in other ways
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: Enhancement New feature or request Type: UI/UX User interface and/or user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants