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

Change default visual indication for the Tree cursor #88537

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

Conversation

passivestar
Copy link
Contributor

@passivestar passivestar requested review from a team as code owners February 19, 2024 07:46
@Chaosus Chaosus added this to the 4.3 milestone Feb 19, 2024
@AThousandShips AThousandShips changed the title Remove visual indication for the Tree cursor Remove default visual indication for the Tree cursor Feb 19, 2024
@KoBeWi
Copy link
Member

KoBeWi commented Feb 19, 2024

The cursor is still relevant, e.g. when renaming with multiple nodes selected. Like here you can't tell which element will be edited when you press F2:

pFc6inKuT4.mp4

@passivestar
Copy link
Contributor Author

@KoBeWi maybe F2 should call batch rename when multiple nodes are selected? 🤔

If there are more cases where the cursor is relevant, I suppose I can try using a flat stylebox with low alpha for the cursor to keep the consistent no-outline look in editor. The cursor will be subtle in that case, but it might be appropriate for it to be subtle considering how rare you need to know the active element with multiple selected

@KoBeWi
Copy link
Member

KoBeWi commented Feb 19, 2024

maybe F2 should call batch rename when multiple nodes are selected? 🤔

I have a PR for that, but still only one item is being edited. You wouldn't show an editing text field for every selected node.

@passivestar
Copy link
Contributor Author

I have a PR for that, but still only one item is being edited. You wouldn't show an editing text field for every selected node.

Not sure what you mean, what does the PR do? Godot already has batch rename, I meant just call that instead of regular rename if you press F2 with more than 1 selected node. It opens a popup

Screenshot 2024-02-19 at 19 28 53

@KoBeWi
Copy link
Member

KoBeWi commented Feb 19, 2024

PR = Pull Request, specifically this one: #69087
Batch rename dialog sucks.

@passivestar
Copy link
Contributor Author

Yeah I know that pr is a pull request, I meant yours. Well I see now, I'm not a fan of popup dialogs either

Plan B is something like this:

image

(except I'll make the item with the cursor match the color of a regular selected item in other parts of the editor ui for consistency)

@passivestar passivestar changed the title Remove default visual indication for the Tree cursor Change default visual indication for the Tree cursor Feb 19, 2024
@davthedev
Copy link
Contributor

I am not shocked by an outline IMHO. If we abandon it, it breaks the consistency of keyboard focus indicators that are elsewhere, such as on buttons and tabs. What about just making it of a more harmonious color rather that grey-over-blue?

If it becomes a filled indicator, there will be no way to see if the item below is selected or unselected.

It is possible to replace the indicator in custom themes anyway.

Just do not forget to change the ItemList for the sake of consistency. The same outline is used in the case of a multi-selectable ItemList, and, as opposite to the TreeView, the arrows move the cursor and not the selection in this case.

@passivestar
Copy link
Contributor Author

passivestar commented Feb 19, 2024

it breaks the consistency of keyboard focus indicators that are elsewhere, such as on buttons and tabs

I was thinking recently about opening a discussion on focus indication in Godot, because it's a little bit over the top now tbh

For most items such as buttons, tabs, trees, etc focus indication is an accessibility feature and you should only see it when you're navigating with the keyboard (just like it works on the web). There's no reason for the high-contrast outline to appear when you click on a button with your mouse. If it was handled in the same way in Godot as it is on the web, and if the outline around tree item elements only appeared when you're using arrow keys, then yes, I'd agree that it should use the same style as all of the other focused elements for consistency (a thick outline)

But until then, while it's always visible regardless of what input method you're using, it probably makes sense to get rid of the outline for consistency with non-multi-selectable trees and lists

Because in the end of the day most users click on tree items with their mouse, and from a regular user perspective the tree cursor doesn't exist, it's just a selected tree item that inexplicably has a gray outline unlike other active elements in godot. That's the perceived inconsistency of it 🙂

@davthedev
Copy link
Contributor

Focus indication should be thought globally, I agree.

In case of multiple items selected, the indicator is a hint of where your starting point is in case you continue with the keyboard arrows from a multiple selection. This sparkles an idea: what about showing the focus indicator only if there is more than one item selected, or if the focus is on an unselected item, or if the latest interaction was through the keyboard? Out of the scope of this PR because needs some dev.

@passivestar
Copy link
Contributor Author

if the latest interaction was through the keyboard

yeah this is how it should probably work everywhere, it's roughly how it works on web, including this website

what about showing the focus indicator only if there is more than one item selected, or if the focus is on an unselected item

sounds interesting, we should discuss it more when I open a discussion on focus. I just need to find time for it

@Calinou
Copy link
Member

Calinou commented Feb 20, 2024

For most items such as buttons, tabs, trees, etc focus indication is an accessibility feature and you should only see it when you're navigating with the keyboard (just like it works on the web). There's no reason for the high-contrast outline to appear when you click on a button with your mouse. If it was handled in the same way in Godot as it is on the web, and if the outline around tree item elements only appeared when you're using arrow keys, then yes, I'd agree that it should use the same style as all of the other focused elements for consistency (a thick outline)

There's a proposal for this: godotengine/godot-proposals#6577

@passivestar passivestar marked this pull request as draft June 13, 2024 07:45
@AThousandShips AThousandShips modified the milestones: 4.3, 4.4 Jul 25, 2024
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.

6 participants