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

Add project tags #75047

Merged
merged 1 commit into from
May 30, 2023
Merged

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Mar 18, 2023

This PR adds ability to assign tags to projects. A project can have any number of tags and they are stored in project.godot. Tags are only usable and editable via Project Manager.

image

You can edit tags using the new Manage Tags button.

godot.windows.editor.dev.x86_64_9m3ovMnIUm.mp4

The dialog includes all tags used by projects on your project list. You can add or remove tags to the project simply by clicking them.

You can filter and sort projects by tag.

godot.windows.editor.dev.x86_64_9DflE8DQtX.mp4

Tag sort works based on the alphabetical order of tags.

Right now tag colors are assigned based on their name and they can't be edited. This is because a tag is a simple String.

Closes godotengine/godot-proposals#2329

To consider: Favorite projects can be possibly removed now. You can just slap a favorite tag.

EDIT:
I added a bit more restrictions to tags, to make them more consistent (when multiple people create the same tag etc.). They can be only lowercase now, but they appear capitalized on project list:

EDIT2:
Redesigned tags after feedback

@KoBeWi KoBeWi added this to the 4.x milestone Mar 18, 2023
@KoBeWi KoBeWi requested a review from a team as a code owner March 18, 2023 00:23
@KoBeWi KoBeWi force-pushed the #new_#feature_#tags_#much_#wow branch from a878daf to 5e9644c Compare March 18, 2023 00:26
editor/project_manager.cpp Outdated Show resolved Hide resolved
@KoBeWi KoBeWi force-pushed the #new_#feature_#tags_#much_#wow branch 2 times, most recently from 50983b7 to 7c01041 Compare March 18, 2023 00:41
@JohnMeadow1
Copy link

Oh this I will love. +5
Having hundreds of small projects, browsing gets tidious.

@Calinou
Copy link
Member

Calinou commented Mar 18, 2023

To consider: Favorite projects can be possibly removed now. You can just slap a favorite tag.

Favorite state shouldn't be stored in version control, as every user will have a different definition of a "favorite" project. You don't want a random downloaded project to automatically appear in favorites just because its author added a favorite tag to it 🙂

@seppoday
Copy link

Like idea, but visually it could get some more love 😅

@dalexeev
Copy link
Member

dalexeev commented Mar 18, 2023

godotengine/godot-proposals#1133 is about

tree-like


There is also a tabs idea, with many 👍.

@KoBeWi KoBeWi force-pushed the #new_#feature_#tags_#much_#wow branch from 7c01041 to 62b1e24 Compare March 18, 2023 13:49
@KoBeWi
Copy link
Member Author

KoBeWi commented Mar 18, 2023

Like idea, but visually it could get some more love 😅

Like what?

@MewPurPur
Copy link
Contributor

I think it would be less eye-popping if the saturation is forced to less than, say, 0.7?

@seppoday
Copy link

Like idea, but visually it could get some more love 😅

Like what?

I'll try to give some visual feedback later tonight 👍

@Jummit
Copy link
Contributor

Jummit commented Mar 18, 2023

Can these tags be searched for using the search input? I don't see anything referencing it in the code. I also think that the colors are a but overwhelming, for me a single color would enough. That's just personal preference of course.
I also like these designs: godotengine/godot-proposals#2329 (comment)

@KoBeWi
Copy link
Member Author

KoBeWi commented Mar 18, 2023

Can these tags be searched for using the search input?

Yes, the second video shows filtering.

also think that the colors are a but overwhelming, for me a single color would enough.

Colors make them easier to distinguish visually. Could be less saturated maybe.

I also like these designs

Named tags are more descriptive. Also that design allows only one tag per project.

@seppoday
Copy link

seppoday commented Mar 18, 2023

Like idea, but visually it could get some more love 😅

Like what?

Here are 4 quick ideas for different looks of tags. I didn't change behaviour of tags (placement, etc.) just visuals. First idea is closest to your PR @KoBeWi, just littlebit more pleasing to the eyes in my opinion.

Other 3 are just for discussion purposes... but personally I kinda like second idea?

  1. Littlebit less saturated colors. Also text is in background color but darker.
    image

  2. Godot theme background color. With subtle color stripe on left side (colors similar to 1st idea). It kinda reminds me the look of tabs in godot (blue stripe at top)
    image

  3. No colors, but added icons.
    image

  4. No colors. Just godot theme color.
    image

@AThousandShips
Copy link
Member

2 is a lot more readable to me at least, and I think more accessible

@seppoday
Copy link

seppoday commented Mar 18, 2023

Also, just an idea... but adding and removing tags directly from project manager view might be more intuitive and it might look better than tag manager popup?

But... it might be not consistent with how other stuff work in project manager (renaming, deleting etc...). Sugested fix for that is on bottom of this answer.

image

And same idea. but second design:
image

Adding tag to project (this one is more like a quick example, maybe flow container would be better? dunno...)
image

Also I was suggesting some changes to project manager overall in this PR (#74561)
I've added tags to my design... so I am just gonna leave this here. Just some random idea from that PR.

(You sort by clicking on header name, edit project by clicking on it and all other stuffs are under 3 dots menu. Buttons not connected to any project are on top bar - new project, scan etc.)
image

(sorry for shameless plug in this PR...)

@fire
Copy link
Member

fire commented Mar 18, 2023

Using the okhsl colors functions will give a more even gradient if you were trying for a constant lightness (slightly darker)

@KoBeWi
Copy link
Member Author

KoBeWi commented Mar 18, 2023

https://user-images.githubusercontent.com/62170071/226129562-a78ebe7d-37fa-4f65-b897-02b05fe8723a.png

Quick-editing tags like this won't work well for 2 reasons:

  • it's easy to missclick on the X button
  • every tag modification modifies the project.godot file, so it's better to batch it and make cancellable

In any case, I think my design matches the current Project Manager design, your suggestion is better for a re-design, which this PR is not about.

I tweaked the appearance of tags however:
image

@Riteo
Copy link
Contributor

Riteo commented Mar 18, 2023

@KoBeWi that's great! There is a small graphical glitch though, the band doesn't really cover the rounded corners fully:
scaled up image

I wonder if this also changes according to the rounded rectangles amount in the theme (if that even matters here)

@KoBeWi KoBeWi force-pushed the #new_#feature_#tags_#much_#wow branch from db1a355 to 9bb68cd Compare March 18, 2023 21:21
@KoBeWi
Copy link
Member Author

KoBeWi commented Mar 18, 2023

I haven't noticed they are already rounded. Fixed.
image

@seppoday
Copy link

https://user-images.githubusercontent.com/62170071/226129562-a78ebe7d-37fa-4f65-b897-02b05fe8723a.png

Quick-editing tags like this won't work well for 2 reasons:

  • it's easy to missclick on the X button
  • every tag modification modifies the project.godot file, so it's better to batch it and make cancellable

In any case, I think my design matches the current Project Manager design, your suggestion is better for a re-design, which this PR is not about.

I tweaked the appearance of tags however:
image

Thats ok. Great work nonetheless. Thanks for taking my idea into consideration

@KoBeWi
Copy link
Member Author

KoBeWi commented Mar 19, 2023

It's not feasible to disallow conflicts in this case. You might add e.g. demo/ff0000 tag and then import an official demo project which has demo/00ff00 tag. You didn't add it, it just came with a project. Now when you assign a new tag to some project, the dialog lists all tags it found in all projects. If we were to realistically disallow duplicate names, you'd be only allowed to assign either demo/ff0000 or demo/00ff00 depending on which project appeared first on the list.

@Malkverbena
Copy link

Malkverbena commented Mar 19, 2023

you'd be only allowed to assign either demo/ff0000 or demo/00ff00 depending on which project appeared first on the list.

Once you can edit the color of the tags, it would be ok?

@KoBeWi
Copy link
Member Author

KoBeWi commented Mar 19, 2023

But editing a tag color should edit only this specific tag or all tags? And if all tags, it should update all assigned tags to be consistent, which is not really possible...
Custom tag colors should be global if anything.

@Malkverbena
Copy link

I think we should edit only the pecific tag. as you mentioned, edit all tags will not allow update all assigned tags.

@Malkverbena
Copy link

What if when detected tags with the same name and different color, the user was asked what color he wants to keep?

@DukeVengeance
Copy link

To consider: Favorite projects can be possibly removed now. You can just slap a favorite tag.

Favorite state shouldn't be stored in version control, as every user will have a different definition of a "favorite" project. You don't want a random downloaded project to automatically appear in favorites just because its author added a favorite tag to it 🙂

I think Tags should also not be stored in project.godot, but only in project managers.

You don't want a random downloaded project to mess with your nicely organized tags in project manager as well.

@Calinou
Copy link
Member

Calinou commented Mar 28, 2023

I think Tags should also not be stored in project.godot, but only in project managers.

If we don't store tags in project.godot, this would remove the ability for collections of projects such as https://github.com/godotengine/godot-demo-projects to be automatically organized using tags.

@KoBeWi KoBeWi force-pushed the #new_#feature_#tags_#much_#wow branch from 9bb68cd to f458b77 Compare April 5, 2023 10:18
@KoBeWi
Copy link
Member Author

KoBeWi commented Apr 5, 2023

Rebased after the recent refactor. Unfortunately I made a mistake and the old tag look got squashed. I preserved it in a patch: 75047.patch.txt

@Calinou
Copy link
Member

Calinou commented May 20, 2023

Tested this PR locally (rebased against latest master), it works 🙂

It's pretty good so far, but I've found ways to further improve its usability:

  • Clicking a tab again for filtering should remove its tag: text from the filter, instead of appending it one more time. This can be done by checking whether tag:tag_name is present in the filter, then removing it if that's the case.

  • Tags only appear capitalized in the main project manager view, not the Manage Tags view. This feels inconsistent; I think we should capitalize them in both. The Create New Tag dialog could have a note to say "Tags are capitalized automatically.".

  • Pressing Enter while having the LineEdit focused in the Create New Tag should accept the text if it's valid.

  • Creating a new tag should add it to the currently managed project automatically, as this is generally the behavior I'd expect. It requires one less click to add a tag to a project 🙂

  • Managing tags when having multiple projects selected doesn't work (it only affects the first selected project). If you can't find a way to make it work, the Manage Tags button should be grayed out when selecting more than one project.

  • In the Manage Tags dialog, tags displayed at the top (that can be removed by clicking) should have a "cross" icon on the right similar to scene tabs. This is purely a visual aid; the existing logic should be kept identical, allowing you to click the entire tag to remove it (it's more touchscreen-friendly).

@KoBeWi KoBeWi force-pushed the #new_#feature_#tags_#much_#wow branch from f458b77 to 2b93dd2 Compare May 20, 2023 22:18
@KoBeWi
Copy link
Member Author

KoBeWi commented May 20, 2023

@Calinou All points addressed.

@Calinou
Copy link
Member

Calinou commented May 20, 2023

I get a warning when building locally:

editor/project_manager.cpp: In member function 'void ProjectManager::_manage_project_tags()':
editor/project_manager.cpp:2450:34: warning: possibly dangling reference to a temporary [-Wdangling-reference]
 2450 |         const ProjectList::Item &item = _project_list->get_selected_projects()[0];
      |                                  ^~~~
editor/project_manager.cpp:2450:81: note: the temporary was destroyed at the end of the full expression 'ProjectList::get_selected_projects() const().Vector<ProjectList::Item>::operator[](0)'
 2450 |         const ProjectList::Item &item = _project_list->get_selected_projects()[0];
      |                                                                                 ^

Also, the Close icon next to tags doesn't appear when the tag is freshly added in the Manage Tags dialog. It only appears if you close then reopen the dialog.

@KoBeWi KoBeWi force-pushed the #new_#feature_#tags_#much_#wow branch from 2b93dd2 to dc41261 Compare May 20, 2023 23:06
@KoBeWi
Copy link
Member Author

KoBeWi commented May 20, 2023

Fixed.

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Works great now!

@YuriSizov YuriSizov self-requested a review May 22, 2023 11:57
@YuriSizov
Copy link
Contributor

image

I think it looks weird with the unsupported features list. That list looks weird to begin with, but with the tags added to the mix, it just looks inconsistent. (Almost like it's two different people designed these two features.) I think it's worth addressing this awkward design before merging.

But otherwise I can confirm that it works as expected. We would probably want to do more with this feature, but as a starter kit for managing tags it's perfectly capable.

@KoBeWi KoBeWi force-pushed the #new_#feature_#tags_#much_#wow branch from dc41261 to e767ff5 Compare May 29, 2023 22:53
@KoBeWi
Copy link
Member Author

KoBeWi commented May 29, 2023

@YuriSizov YuriSizov modified the milestones: 4.x, 4.1 May 30, 2023
@YuriSizov YuriSizov merged commit f6db010 into godotengine:master May 30, 2023
@YuriSizov
Copy link
Contributor

Another long-awaited feature implemented. Great job!

@KoBeWi KoBeWi deleted the #new_#feature_#tags_#much_#wow branch May 30, 2023 13:21
@Abab-bk
Copy link

Abab-bk commented Jun 5, 2023

I think this looks good, but I don't know anything about program development :)
Group 207

@sosasees
Copy link

sosasees commented Jun 8, 2023

@Abab-bk i can't read the tags in your design, because the text color is too similar to the background color.

i like the current design much better because the text has much better contrast.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add color tags to the project manager