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 theme type variations for secondary Trees and ItemLists #97884

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

passivestar
Copy link
Contributor

@passivestar passivestar commented Oct 6, 2024

This aims to address the issue raised in passivestar/godot-minimal-theme#8

This PR has no effect on the default theme.

One of the reasons I made a theme was to simplify the layout by removing some of the docks' background panels. But there's an issue with that.

In Godot some trees occupy the majority of their containers and show what you could call "primary content", like the trees in the Scene and the FileSystem docks, or the main area of the File Dialog. Meanwhile some other trees (and itemlists) show secondary "sidebar" content, like the ones on the left in the Shader Editor, Profiler, Create Dialog, etc. Those sidebar trees and lists need to have background panels to be visually separated from the rest of the content, but it's impossible to have different background panels for them that wouldn't affect "primary" lists

This PR adds type variations for secondary lists to make it possible to theme them separately

Old New
master pr

Theme for testing: godot-minimal-theme.zip

@passivestar
Copy link
Contributor Author

Moved them to the editor theme as per suggestion

Copy link
Member

@Geometror Geometror left a comment

Choose a reason for hiding this comment

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

This is great! Code looks fine, I'm only a bit sceptical about the "Side~" prefix. What do you think about "Sub~" (or "Secondary~"/"Sec~")?
I'd also consider using a suffix instead to make it a tiny bit clearer that it's a theme type variation and not another type, so e.g. "TreeSecondary" and "ItemListSecondary". IMO having a longer name for a theme type variation isn't a problem.

@passivestar
Copy link
Contributor Author

passivestar commented Nov 15, 2024

This is great! Code looks fine, I'm only a bit sceptical about the "Side~" prefix. What do you think about "Sub~" (or "Secondary~"/"Sec~")?

Decided to go with Secondary, 'Sub'Tree could imply that it belongs to a tree hierarchy of sorts. Secondary is ok though

Applied the suggestions and tested again with an updated theme, works as expected

Upd: Also added a previously missed itemlist of editor help overview

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.

3 participants