Add interface/theme/{window_border_margin,top_bar_separation,default_margin_size} editor settings

[adamscott]

This PR adds interface/theme/window_border_margin, interface/theme/top_bar_separation and interface/theme/default_margin_size editor settings in order to easily edit the editor interface margins.

Note

This PR doesn't directly add a way to change the tree items margin size, only the default margin size used for all sorts of themes initiated during the startup for the editor.

Preview

TreeView (impacted by the change change)

default_margin_size at 4	default_margin_size at 2

Current issues with the PR

• Some UI break with the default margins lower than 4 (like the Tree arrow placement and lines)

Fixes

Fixes godotengine/godot-proposals#8505

[YuriSizov]

As mentioned in the proposal, I don't think we should be exposing so many properties. If you want to customize the editor theme so granularly, you can already do that using the theme editor.

The editor settings menu should only expose high level and frequently needed settings. So, for example, it can expose a setting that is responsible for overall padding in various elements. We'd use it for the default margin and use twice as much for window border and the top bar separator, if that's a reasonable need.

We also already have a setting that is supposed to add extra spacing. We can evaluate if it should be utilized better and cover the cases explained in the proposal. And we can discuss if it maybe should support negative values to reduce spacing in places. All in all, there are more options to consider and it's not the implementation that's the hard part here.

I also have plans for making custom editor/default themes based on the same parameters we use to do it in code, but exposed nicely to users. I feel like this will cover most of these edge case requests.

[Calinou]

I think it makes sense to reduce the default spacing between docks (and the outer editor margin) as it doesn't really serve much of a practical purpose¹, but we don't need to add a setting for this.

This was something I attempted a while ago but back then, you couldn't reduce dock margins without affecting their draggable area, which negatively impacted usability. This means my PR had an "unbalanced" look as the outer margins were decreased, but not the margins between docks. Now, we can finally have a "balanced" appearance.

Footnotes

1. SplitContainer drag margin can be increased regardless of visual margin since 4.0, so you don't lose any space for dragging docks with the mouse. ↩

[coppolaemilio]

@YuriSizov

As mentioned in the proposal, I don't think we should be exposing so many properties. If you want to customize the editor theme so granularly, you can already do that using the theme editor.

We are not exposing every property here, this is only a few that could be considered independent from the current theme. You could be using a theme to style it in some way and still prefer to tweak some values on top of it. Same as when users modify the text size or the rendering scale. So it is a bit of a strawman here. We are not adding every option the theme has to the panel, we are adding one that was requested and can benefit any theme you might already be using without having to modify it.

I also have plans for making custom editor/default themes based on the same parameters we use to do it in code, but exposed nicely to users. I feel like this will cover most of these edge case requests.

It is great that you have plans to do this, but right now we have this PR which would fix this. Do you have an estimate on when you'll be able to make a PR that implement this in a way that you think it's better?

I don't want this to be a "there can be a better way" hence we do nothing about it and we leave the PR, the proposal and the benefit of the user to rot.

[YuriSizov]

@coppolaemilio

We are not adding every option the theme has to the panel, we are adding one that was requested and can benefit any theme you might already be using without having to modify it.

At least two of these properties are very specific and don't really benefit most of the themes as independent properties. I explained above that we don't have to expose properties one to one to their theme definitions. There is no such requirement. We can be smart about it and use a generalized and simple setting that would adjust all the properties described here. Or we can use the existing one that already works in a similar way by modifying it. Or we can just reduce margins by default, as Calinou suggests.

There are more options offered in just these few comments above than "we expose all or nothing". But if you insist on exposing them exactly as they are in the theme, then we do go back to the question raised in the proposal: where do we draw the line? How do you assess what's reasonable and what's not? Two people asked for two different things, and this PR does as it was bid by those two people. They have different amount of public support and are questioned on the merits differently. So what's the criteria that is not a strawman to you?

It is great that you have plans to do this, but right now we have this PR which would fix this. ... I don't want this to be a "there can be a better way" hence we do nothing about it and we leave the PR, the proposal and the benefit of the user to rot.

And I don't want this to be a "Someone made a PR so we're already past the point of discussion, let's just merge whatever". I understand that you don't want to get stuck in a limbo discussing these changes, but that doesn't mean we shouldn't have a discussion at all. This is not how we work, and you are not in a position to decide for everyone if this is a good change or not. I feel slightly offended that key stakeholders like Calinou and myself are not taken seriously, like we're just hurdles in your path forward with this.

If you want to spend Adam's time on these improvements, why not spend it on a solution that we agree upon and see merit in? You don't have to get a consensus from the entire contributor base, but you could at least discuss it with the team before deciding that, yes, this is exactly what we need. Adam can just as well execute the plan that I imagine in my place, if we decide that it's a better solution, so you don't have to wait for me to be available. There is no need to commit to something hastily and then get defensive about it. We can just talk first, you know?

[coppolaemilio]

It's great that you feel passionate about this change!

What I see here is that there was:

problem -> solution a

And then, when presented with the solution, you say:

problem

• solution b
• solution c
• solution d

So without going into the realm of: let's take this personal and fight about process.

What would your ideal solution be? I still don't know what you think should be done here.

[MichaelWengren]

This is an incredibly useful feature, window_border_margin and top_bar_separation should be configurable in the settings, they greatly influence the visual style, and in most cases they will improve the editor experience, especially on small monitors.

[reduz]

This is a specific topic that is brought up time and time again by users for years, so I agree that it should be an editor setting instead of forcing to go through a theme. Seems a significant amount of people really prefer to customize their margins, so I am no one to judge or impede this.

[passivestar]

My 2 cents:

it can expose a setting that is responsible for overall padding in various elements. We'd use it for the default margin and use twice as much for window border and the top bar separator

I'm fine with one setting that drives all of the paddings. I don't expect editor settings to provide too much granularity. As long as it affects the tree views enough to make the editor more practical, and if it's not hard to implement, I think it should be totally fine

we can discuss if it maybe should support negative values to reduce spacing in places

tbh to me as a user it doesn't really matter what range the values on a slider are as long as it does the thing. but I want to say that if it's going to be "extra spacing" it probably needs another name because to me as a user extra spacing that goes into negative feels like some kind of a hack

I think it makes sense to reduce the default spacing between docks

I agree with Calinou. None of the other apps that I use have dock gaps this big. I'd just cut them in half

I don't want this to be a "there can be a better way" hence we do nothing about it and we leave the PR

I agree with this too. This seems like a pretty simple and straightforward change, would be sad if it wasn't implemented due to some small disagreements on details, considering how many people expressed their interest in this

[YuriSizov]

This seems like a pretty simple and straightforward change, would be sad if it wasn't implemented due to some small disagreements on details

So far nobody said that this shouldn't be implemented.

[ghost]

So far nobody said that this shouldn't be implemented.

But why are you blocking then?
This is a very simple PR, it won’t lead to any problems and could already be merged.

[YuriSizov]

But why are you blocking then?

Because I want it to be implemented better. Above you can find a few suggestions about doing this change properly, including reducing default margins for everyone (which should already help most people who agree with @passivestar's proposal) and adding one general setting to control these margins instead of multiple settings.

Just because we defined independent properties in the editor theme doesn't mean we need to make it more finnicky to the end user to configure in the editor settings. The end user probably doesn't need to fine tune each individual property. They just want a more compact style for the editor in general. Please, don't give too much credit to the decision to use 3 different theme constants in the editor theme in the first place. It wasn't a conscious decision with a lot of weight on it, it was just a quick and convenient way to do it internally. So we don't have to abide by it. We can simplify and make it more sensible, more easily controlled, more high level.

and could already be merged

Just because someone made an effort it doesn't mean we are going to merge the PR. That's not how this project is governed. This is why we ask everyone to open proposals and discuss changes before committing to them. Making a PR is not a guarantee that your work is going to be accepted. And the proposal that this closes has an unresolved discussion, which now spills over here.

[ghost]

Because I want it to be implemented better.

This is great, and when you do it we will be happy to discuss it and if it turns out to be better it will be merged on top of this one.

But at the moment there is a simple solution that solves the problem, why are you preventing it from being merged?
If you want to do something better, you shouldn't stand in the way of others, this is ridiculous.
What right do you have to block other people’s proposals just because you want to do it differently?
How exactly accepting this proposal prevent you from implementing your changes in the future? How does it interfere with one another?

This is a very simple solution, it suits everyone.

[AThousandShips]

Making a temporary or partial solution in anticipation for a future, more comprehensive, solution just makes more work for everyone, the old feature has to be integrated, the old setting has to somehow be integrated with the new system, and users have to suddenly switch how they do things

Do don't want partial or temporary solutions for anything but critical issues that needs to be solved now

[YuriSizov]

What right do you have to block other people’s proposals just because you want to do it differently?

As a stakeholder and a maintainer of the area it's my literal job to make sure contributions satisfy user needs, quality standards, and the overarching design. And the review process includes offering feedback that may require changes from the PR's author. Just like in this case.

[MichaelWengren]

I also don’t quite understand why it is blocked.
I understand that there is always a way to do something better but c'mon look how simple this pr is, literally a few lines of code, simplicity is genius!

[Calinou]

@AsimDerth I have to remind you that we have a Code of Conduct. Please stay constructive.

[TiagomCoutinho]

This is an important feature for an usability standpoint that makes sense be available outside theming, someone working on a touch device would like to have more spacing just like a desktop user would like to have a more compact. But i understand what @YuriSizov is saying, having too many properties exposed could be detrimental to the user experience.
Having predetermined values could be a better experience, while allowing users to set a custom value using themes. One good example of that is Firefox

(ignore the compact version being deprecated)
They have a field called Density, with different options optimized for different use cases like Touch
But with custom css themes, you can set any value you want

We could do something like that, have a field called "Density" with multiple values like Compact, Normal, Tablet, etc with predetermined values optimized for that use case, but if the user would like to have a custom value, they could go with the custom theme route

(this is my first time posting and working with Godot, sorry for any mistakes i made)

[chutneyio]

I agree that this should be better implemented. Exposing 3 properties for only one purpose of reducing the gap between panels is so confusing and unnecessary complicated the UX, where the Settings already has the Reduce border size option which I thought it as an option to decrease the panel margin when the first time I’m using Godot. All other apps (like Blender, Unity…) have a very compact UI with small margin so why don’t we do the same? Just make it default by changing the hardcoded 8 to 1 or something, most user just go with it anyway because other popular apps do.

[tokengamedev]

Personally, I don't want to see all the three settings in settings page for to achieve this requirement(which I very much want it)
But to achieve the requirements these three values are required to be configured.
I doubt and have a feeling that these are not the only ones that needs to be configured. There may be more values required to be configured.

The option of using themes to configure is bad, as already theme is too complicated for a basic user, and touching the theme used by the Editor 😨

My suggestion will be using a user-friendly setting like display_mode, which will have values like Default/Compact/Touch.

Internally Godot may use config files for each option containing all the settings required to achieve it. For advanced users they can edit the config files to further customise.

[akien-mga]

Superseded by #87085.