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

Alternating background colours for tree items. #89890

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

Conversation

AyyZerrAsa
Copy link

@AyyZerrAsa AyyZerrAsa commented Mar 25, 2024

Alternating colours to improve item visual separation in trees. A feature in Blender and Unreal's outliners that helps readability.

For long lists of items (especially the shortcut settings), it can be difficult to read due to there being no clear boundaries between items. A subtle alternating background colour on every other item clearly shows the boundary box of items.

Before After
shortcuts_before shortcuts_after
tree_before tree_after

Bugsquad edit: Closes godotengine/godot-proposals#9376

@passivestar
Copy link
Contributor

A problem when using a custom theme with different line heights:

image

scene/gui/tree.h Outdated Show resolved Hide resolved
scene/gui/tree.cpp Outdated Show resolved Hide resolved
scene/gui/tree.cpp Outdated Show resolved Hide resolved
scene/gui/tree.cpp Outdated Show resolved Hide resolved
@Mickeon
Copy link
Contributor

Mickeon commented Mar 26, 2024

I feel like a grand majority of Trees do not benefit from this. There's way too much noise that does not make the additional distinction between lines worth it. Exceptions like the Shortcut tree do exist

scene/gui/tree.cpp Outdated Show resolved Hide resolved
scene/gui/tree.cpp Outdated Show resolved Hide resolved
scene/gui/tree.cpp Outdated Show resolved Hide resolved
scene/gui/tree.cpp Outdated Show resolved Hide resolved
scene/gui/tree.h Outdated Show resolved Hide resolved
scene/theme/default_theme.cpp Outdated Show resolved Hide resolved
scene/gui/tree.cpp Outdated Show resolved Hide resolved
// stripe_count - 1 to avoid an otherwise ugly gap at the top of the tree.
if ((stripe_count - 1) % 2 == 0) {
Rect2 j = Rect2(draw_ofs.x, total_size + theme_cache.v_separation, _get_content_rect().size.x, height);
j.position = j.position - get_scroll().floor();
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean to just floor the scroll? This code used to be ceil before, and for both parts

Copy link
Author

Choose a reason for hiding this comment

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

Yeah due to changes to fix the different vertical separation value issues I needed to floor here to avoid truncating to the wrong int value. Realise I also don't need the scroll logic for the empty stripes as if you can scroll, there is no empty space.

Added alternating background colours to trees. A visual feature in other digital content creation suite outliners which helps separate elements in a list. Particularly useful for the shortcuts editor which is otherwise a long list of text with no visual boundaries. Adjustable via theming.
@mhilbrunner
Copy link
Member

This is nice for tables with multiple columns (say, >2). For the others shown, simply highlighting the hovered rough is enough, IMO, and alternating lines don't serve a purpose there.

@KoBeWi
Copy link
Member

KoBeWi commented Apr 8, 2024

Since the stripes make sense for some Trees and for some doesn't, maybe it would be better as regular property instead of theme one? 🤔

Other than that the implementation looks fine. It would be nice to enable stripes in some editor Trees (input map at least, not sure about others).

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.

Implement alternating background colours for tree items
8 participants