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 custom color support to project folders #80440

Merged
merged 1 commit into from
Aug 30, 2023

Conversation

the-sink
Copy link
Contributor

@the-sink the-sink commented Aug 9, 2023

This PR addresses godotengine/godot-proposals#768 and adds the ability to give folders in a project a custom color when viewed in the FileSystem dock. Child files & folders will inherit their parent folder's color if they don't have one set.

As of b98981e:

image

image

project.godot:

image

@Mickeon
Copy link
Contributor

Mickeon commented Aug 9, 2023

I think the background color is too exaggerated. In my opinion, the folder icon color helps by a long shot already, so it doesn't feel so necessary for the background to be this visible.

Also it's worth considering that the default colour of the folder icon depends on the editor's accent color, too.

@DennisGHUA
Copy link

DennisGHUA commented Aug 9, 2023

I think the introduction of background colors has notably improved the efficiency of locating assets and folders. This enhancement is exemplified by the darker background color now visible under the "another_child" folder when opened. Prior to this update, Godot lacked a distinct differentiation between opened and closed folders. It's worth noting that the absence of open and closed folder icons in Godot remains a separate topic for consideration. Nevertheless, it might be beneficial to fine-tune the brightness of the background colors slightly to achieve an even more polished visual effect.

Alternatively, the option to enable the background color could be integrated within the editor settings. While I find this to be a valuable addition, considering that it constitutes a substantial alteration to the file explorer, its implementation might encounter varying opinions within the community. To address this potential divide, it could prove beneficial to incorporate a toggle in the editor settings, allowing users to independently enable or disable the background colors according to their preference.

editor/filesystem_dock.cpp Outdated Show resolved Hide resolved
editor/filesystem_dock.cpp Outdated Show resolved Hide resolved
editor/filesystem_dock.cpp Outdated Show resolved Hide resolved
editor/filesystem_dock.cpp Outdated Show resolved Hide resolved
editor/filesystem_dock.cpp Outdated Show resolved Hide resolved
@Calinou
Copy link
Member

Calinou commented Aug 9, 2023

I think the introduction of background colors has notably improved the efficiency of locating assets and folders. This enhancement is exemplified by the darker background color now visible under the "another_child" folder when opened. Prior to this update, Godot lacked a distinct differentiation between opened and closed folders. It's worth noting that the absence of open and closed folder icons in Godot remains a separate topic for consideration. Nevertheless, it might be beneficial to fine-tune the brightness of the background colors slightly to achieve an even more polished visual effect.

I think we can keep the background color, just decrease its opacity a bit compared to what we have now. The current background color opacity makes it a bit distracting, on top of possibly being confusable with a selection highlight (if your editor theme's accent color is close to the folder color).

@the-sink
Copy link
Contributor Author

the-sink commented Aug 9, 2023

@Calinou Yep, agreed - Reduced opacity of the background color instead of darkening it looks better, with the added benefit of fixing the highlight issue:
image


I also added a few additional color options and changed them all to more saturated versions that exist in the editor_themes.cpp color conversion list.
image

Many areas of the editor (i.e. icons, accent lines, etc) use lighter, more pastel-like colors so I initially tried to keep them closer to that appearance, but personally I like the more saturated colors in this context. Interested in what others think though.

@DennisGHUA
Copy link

@the-sink Currently, I believe that opting for lighter, more pastel-like colors for the background would create a slightly less distracting environment.

@Mickeon
Copy link
Contributor

Mickeon commented Aug 9, 2023

I like the way more saturated colors for the folders. If they are coloured, it means the user finds them noteworthy, somewhat.

Still not convinced on the background color. It's quite distracting, I feel. Although, it does grant the benefit of being able to discern under what folder everything is much more easily.

I do have a suggestion: what if only the nested elements inside the folder had the background color (still toned down a notch)?

A mock-up:
image

@DennisGHUA
Copy link

@Mickeon If there's a divide within the community concerning the use of background colors, a better approach might be to offer it as an option within the editor settings, as I mentioned earlier. If you still want to distinguish between open and closed folders without relying on background colors, consider updating the default file explorer's background colors to match the image in the proposal on the right. Essentially, this involves making the default background slightly lighter when no custom background image color is chosen. While this addition might slightly exceed the current scope of the pull request, its value makes it worth considering.
image

@Mickeon
Copy link
Contributor

Mickeon commented Aug 9, 2023

Editor settings should always be a last resort, I feel. It's worth choosing the most favourable default before considering options. Especially because not all editor settings are easy to find.

@the-sink
Copy link
Contributor Author

the-sink commented Aug 9, 2023

This would require a separate PR, but another idea; what if the Tree relationship lines matched folder colors instead? An option could be added to the Tree object for path lines to inherit the icon modulate color of their parent items.

image
This still makes the folder hierarchy more visually clear to users, I think.

Alternatively, the background color could be made even more subtle:
image
I personally don't mind the background color, but I can see how it could be distracting.

We'll also need to think about colorblindness accessibility & potentially having different patterns or shapes for these color options, however I'm not sure if that should be a part of this PR or a future one.

@Norrox
Copy link
Contributor

Norrox commented Aug 11, 2023

Great work on this!
The more saturated folders look fine for me.
I actually prefer a more saturated background color for the primary folder. The stronger background helps in quickly identifying and differentiating the folders. While I recognize the concern about potential distractions, I feel that a bolder color, especially for the main folder, adds a layer of emphasis that can be beneficial. However, I’m on board with the idea of giving nested elements a more subtle shade. This can provide the right contrast and hierarchy between primary and nested folders. And for those who might find colors distracting, there’s always the option not to use them at all.

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.

Tested locally (rebased on top of master 4714e95), it works as expected.

Testing project: test_pr_80440.zip

The highlight colors look good to me now:

Default Light Black (OLED)
Screenshot_20230812_120437 Screenshot_20230812_120448 Screenshot_20230812_120427

We could consider adjusting the highlight colors later, but I think this is better left for a future PR after people start using this feature in their projects.

editor/filesystem_dock.cpp Outdated Show resolved Hide resolved
@the-sink the-sink force-pushed the folder-colors branch 2 times, most recently from 5347641 to ae31a29 Compare August 12, 2023 18:00
@the-sink
Copy link
Contributor Author

the-sink commented Aug 12, 2023

Fixed the light theme folder icons:
image

You can see in the screenshot I also used the CanvasItem icon (felt it was the most suitable) for the option in the context menu. We could also leave it blank/without an icon, although I feel that'd be a little out of place considering all of the other items do have icons.

Marking the PR as ready - unless I have anything to fix that comes up in CI, it seems to be ready!
Edit: yes I do

@the-sink the-sink marked this pull request as ready for review August 12, 2023 18:03
@the-sink the-sink requested review from a team as code owners August 12, 2023 18:03
@the-sink the-sink force-pushed the folder-colors branch 3 times, most recently from a7c4bb7 to dac3683 Compare August 12, 2023 18:21
@DennisGHUA
Copy link

What's the reason behind certain .json files inheriting the color of their parent folder, while others do not? Isn't the expected behavior for all child folders and files to use the parent's color? Or did you set the color of those files to "default"?

@the-sink
Copy link
Contributor Author

the-sink commented Aug 12, 2023

@DennisGHUA Oops, didn't notice that! Files will only inherit their parent folder color if the parent itself has a custom color set (not if it was inherited). I'll need to fix that.

Also having a bit of trouble understanding where the memory leak is coming from in the failed Linux CI test, if anyone could take a look at that.

@the-sink
Copy link
Contributor Author

the-sink commented Aug 25, 2023

451c7d3 passed CI but c682745 didn't, despite the only changes being related to comments...?
Passed after a re-run.

Otherwise, I believe pretty all of the issues that were brought up have been addressed.

editor/filesystem_dock.cpp Outdated Show resolved Hide resolved
@the-sink the-sink force-pushed the folder-colors branch 2 times, most recently from 55a208f to dc53f43 Compare August 26, 2023 19:13
@akien-mga
Copy link
Member

Needs rebase after #61818, you should now use the new ProjectSettings::add_hidden_prefix to hide the prefixed properties.

@akien-mga akien-mga merged commit 0a349d5 into godotengine:master Aug 30, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@JekSun97
Copy link
Contributor

JekSun97 commented Sep 5, 2023

This is needed in 3.6+

@Calinou
Copy link
Member

Calinou commented Sep 9, 2023

This is needed in 3.6+

To my knowledge, this PR relies on Tree changes only found in 4.0 (double-check this though). As a result, if this turns out to be the case, it can't be backported easily for 3.6.

The FileSystem dock code also has changed a fair bit between 3.x and 4.x.

@sanyi12
Copy link

sanyi12 commented Nov 30, 2023

Whoever made this please do the same for code lines. Having hundreds of loc's colored would be great. Grouping them into a comment is nice but by having them colored you can also assign similar colors to similar functions without having to actually read the line of description. Thanks.

@GuyUnger
Copy link

GuyUnger commented Dec 3, 2023

It would be very useful if the colors would show up in more places, made a proposal for it godotengine/godot-proposals#8581

@Splendid-Failuers
Copy link

Splendid-Failuers commented May 13, 2024

Would be really nice to have this extend to all icons, and not just folders. I've used colours to track state of files across a project before in other engines, and really miss that feature in Godot. Setting a red icon for a file if it's a block-up asset, yellow if it's in review, and green if it's a final asset proposal. Really nice to not have to track progress on all files in an external program. But then again, might just be better to make a proposal for a tagging system for the file-manager.

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.