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

Increase ItemList item height in editor theme #65230

Merged
merged 1 commit into from
Sep 5, 2022

Conversation

fire-forge
Copy link
Contributor

Makes ItemList use the same item height as PopupMenu. This makes it easier to select items.

With the default theme, items are only 2 pixels taller, so there is not a huge difference but it still goes a long way in making it easier to select ItemList items and making the UI look less squished together.

Before After
image image

ItemList now uses the same value for v_separation as PopupMenu. This makes it easier to select items.
@Mickeon
Copy link
Contributor

Mickeon commented Sep 2, 2022

I like the "extra squashing" in the File System but I do understand the decision.

Copy link
Contributor

@YuriSizov YuriSizov left a comment

Choose a reason for hiding this comment

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

I'm fine with this change, but it kind of sucks that this is the minimum and people who'd preferred a more squashed look cannot do anything about it.

Both before and after this change the property is controlled by the additional spacing editor setting, so technically if you want the UI to be more spacious, you can achieve that (but it will affect a whole lot of other things too). But personally, I'd prefer this to be what users see out of the box. And it matches popup menus.

@Calinou
Copy link
Member

Calinou commented Sep 5, 2022

Currently, ItemList in the editor theme uses a 26 pixel-tall highlight, while this PR uses 28 a pixel-tall highlight. (By the way, there's an off-by-one error in the Y offset of the selection highlight drawing.)

For reference, this is the line spacing I use in Dolphin (KDE's file manager) in details mode. This is the second lowest zoom level (highlight is 26 pixels tall, like before this PR):

image

I find Dolphin's spacing a tad too low in some cases, so I think this PR is a good change.

As for spacing on small displays, we can allow the Additional Spacing editor setting to have a negative value in a separate PR (up to -3 or so).

@YuriSizov YuriSizov merged commit 00fa4e2 into godotengine:master Sep 5, 2022
@YuriSizov
Copy link
Contributor

Thanks!

@fire-forge fire-forge deleted the itemlist-spacing branch September 5, 2022 19:49
@fire-forge
Copy link
Contributor Author

it kind of sucks that this is the minimum and people who'd preferred a more squashed look cannot do anything about it.

Both before and after this change the property is controlled by the additional spacing editor setting, so technically if you want the UI to be more spacious, you can achieve that (but it will affect a whole lot of other things too).

I've seen many people saying that the editor UI has too much padding on proposals and discussions. To fix that, I think the additional spacing setting of 0 should be made more compact than it is now, and the default value should be changed to something like 2. 2 would look the same as what we have now, and 0 would look something like Blender's UI, which has a lot less space than Godot.

Those who want a more compact editor could change additional spacing to 0 or 1.

It would take a lot of work to adjust the editor theme for all node types though, so I think the change done here is good because it makes ItemList and PopupMenu consistent.

@Mickeon
Copy link
Contributor

Mickeon commented Sep 5, 2022

I know it's no place for discussion but with the current padding all around even testing Godot 4 on a screen laptop is... quite the task. Certainly it can be mitigated by shrinking the view to 75%, but it's not exactly a great first impression.

@fire-forge
Copy link
Contributor Author

I know it's no place for discussion but with the current padding all around even testing Godot 4 on a screen laptop is... quite the task. Certainly it can be mitigated by shrinking the view to 75%, but it's not exactly a great first impression.

I agree that it should be possible to use the editor with less padding. I like the current amount of padding on my 1920x1280 screen, but I know many users have smaller screens or just prefer a more compact view.

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.

5 participants