Improve SpinBox interaction, split arrows, add theme attributes

[davthedev]

Discussion here: godotengine/godot-proposals#9260

This improves the usability & styleability of the SpinBox control.

• Separate up and down buttons, each one having its own icon
• Interaction feedback when hovered, pressed and disabled arrow buttons
• When the value is at the extremity of the allowed range, one of the arrows is disabled
• Stylable arrow buttons & icons, separators, configurable minimum buttons width & spacing with the edit field. Each button support different styles, different backgrounds, icons & tinting (modulate) colors can be set for each interaction state
• The embedded LineEdit receives a theme variation name so that it is easy to style it a different way from the regular LineEdit using a global theme

Care is taken not to break the default theme & existing projects that depend on it.

• Slightly larger default width of the arrow buttons, enough to have a better usability with the mouse without risking breaking themes too much
• The single icon for both arrows is still supported; an empty icon is supplied for it in the proposed default theme change
• The behaviour to size the buttons width according to the button icons always works, although it is now possible to disable it in the theme properties.

With some styling, what is possible. This resembles what I want to achieve in my project:

Big buttons for touch screens

Another possible minimal look with the new separator theme option

Live in Godot in the color selector

Actually, there are two different widgets that render this UI element. The second one is a specialized cell of Tree, and seems to be used inside the inspector and project / editor options to set the various values. This PR only addresses the SpinBox standalone control. To see the result on Godot itself, use the color selector popup anywhere.

[KoBeWi]

Perhaps updown could be deprecated.

[MewPurPur]

Suggested change

	<svg height="8" viewBox="0 0 16 8" width="16" xmlns="http://www.w3.org/2000/svg"><path d="m12 2-4 3.5L4 2" fill="none" stroke="#fff" stroke-opacity="1" stroke-width="2"/></svg>
	<svg height="8" viewBox="0 0 16 8" width="16" xmlns="http://www.w3.org/2000/svg"><path d="m12 2-4 3.5L4 2" fill="none" stroke="#fff" stroke-width="2"/></svg>

Likewise for the other graphic.

[MewPurPur]

If I'm to believe that the old code is correct, then the new code should also be correct.

[Mickeon]

All of these are straight upgrades but I'm just ever so slightly concerned by how many theme items have been added by this single PR

[MewPurPur]

IMO This amount of configurability should be the rule for Godot's interactive GUI, not the exception.

[Mickeon]

It's moreso the fact that some are not even utilized by the editor itself, which I feel like should be the main point of reference to base all of this customization from. For example, the field_and_buttons_separator and up_down_buttons_separator styleboxes. If needed these can also be achieved with a draw_style_box method call.

[MewPurPur]

Okay, that's a fair point... I disagree about basing GUI customization off of the Godot editor, but I agree that these two theme items may be a bit overkill.

[davthedev]

The configurability seems a little verbose, I agree. IMHO this is not a problem to have multiple theme attributes. They remain hidden in their own submenu when you do not actively edit those and drawing performance is not affected.

Let's not forget that Godot came from being a Nintendo DS game authoring tool AFAIK. There has been a long way since. I am proposing update to core widgets to add proper support for interaction feedback; the thing that gives that snappy feeling and improves usability.

The pattern to set styles for normal/hover/pressed/disabled/etc... states of a given customizable resource is pretty common to ensure customization of interaction feedback. Those multiple combinations of states and resources make up the large number of theme resources added.

I thought about something to address this verbosity when having those combinations of resources and states, that would be proposed separately. Would be state-aware theme resources for example, like ColorStateLists of the early days of Android, in which one single slot receives the resources that defines the customization for the desired states instead of having as many slots as there are different states. Would allow combining states together, defining only a few of those and inherit the rest automatically... For example, you would be able to define only a normal and pressed style and automatically inherit the normal style on other states instead of having to fill them all.
Such a system would be a major change and would need to wait for a major Godot version upgrade where breaking changes can be added.

Regarding the separators, I think each graphic used to render parts of a control should be exposed to theming. For example, there are separators in ItemList but you can only set a color; not a graphic - something to fix in a different PR.

The solution I propose here is usable right now and does not break current projects.

[KoBeWi]

When the value is at the extremity of the allowed range, one of the arrows is disabled

When both are at extremity, both arrows should be disabled:

It works correctly at runtime though.

[KoBeWi]

Unless there is use-case for negative width, buttons_width being -1 could be interpreted as using width from icons and this property could be removed.

[davthedev]

These are the possible cases:

• Do not enforce min width, just use the icon for that
• Enforce min width and let the icon enlarge it
• Enforce width and do not let the icon enlarge the buttons - even if there is some overflow. Useful for dense admin panels for example.

In the updated default & editor themes, I set the min width to have a little more of clickable area for the buttons. At the same time, existing themes may rely on the icon autogrow feature and I do not want to break it. So I configured those to use the second case.

[KoBeWi]

What I mean is that it's redundant to have both buttons_width and set_min_buttons_width_from_icons. If you use the latter, the width is useless. Unless it's bigger than button width, in which case width from icons is useless.

We can assume buttons_width being negative to have the same effect as set_min_buttons_width_from_icons being enabled. Because either you set the width, or let it be determined automatically. This allows to remove set_min_buttons_width_from_icons. Unless there is a case I'm missing?

[Mickeon]

I'd like elaboration on this. What KoBeWi is saying makes sense to me, as well.

[KoBeWi]

So I looked at the list of added properties and I agree that there is too many of them. Configurability is nice, but do we even need that much?
Every up and down icon has its stylebox equivalent, so you can basically use styleboxes instead of icons. That's overkill. I'd just add a single stylebox for up/down background. set_min_buttons_width_from_icons can be replaced with buttons_width as I mentioned. Bool constants are awkward and negative width makes the buttons go outside the SpinBox, which might be useful hack, but it can break layout.
I'm not sure about modulates (although in Control context they should be called color), they can be kind of useful if you want to use a single icon, but aside from fast prototyping, there is little advantage over simply using colored icons.

That's what I think at least, the properties are still up to discussion. Keep in mind that it's easier to add a property than to delete it later.

[davthedev]

So I looked at the list of added properties and I agree that there is too many of them. Configurability is nice, but do we even need that much? Every up and down icon has its stylebox equivalent, so you can basically use styleboxes instead of icons. That's overkill. I'd just add a single stylebox for up/down background.

With a single background resource used for both the up and down buttons, I would not be able to achieve the styles in my examples. The up button has a corner on the top, while the down button has a corner on the bottom, for example.

So far there is no "stacked" stylebox available. Therefore, in order to draw the arrow icon and the bg rectangle, with the arrow alignment performed by the control, it would require a stylebox and a texture.
Being able to draw both the arrow and the entire rectangle of the button is important for some styles and usability.

There is consistency with other buttons in toolbars where you can set the icon and the bg stylebox separately.

It is also much easier to tweak colors if you have the arrow icon and the bg stylebox set as separate elements.
Making the same in a one-stylebox operation would only work with StyleBoxTexture, defeating easy tweaking of colors of a StyleBoxRect without providing a new graphic. Customizing the editor theme is a scenario where easy tweaking matters.

set_min_buttons_width_from_icons can be replaced with buttons_width as I mentioned. Bool constants are awkward and negative width makes the buttons go outside the SpinBox, which might be useful hack, but it can break layout.

Yup, there are no native bool types for theme items, I have seen the constant hack used elsewhere and it is not straightforward to understand it at first sight. Just replicated the technique.

This is for a case where you want, at the same time, to have a minimum width AND let the icon grow it larger if necessary. Because, in the proposed change to the default and editor theme, I gave a minimum width to the buttons, so that they are convenient to target with the mouse, and not break the current themes in projects that may rely on the icon alone to widen up the buttons.

Alternatively, it is possible to disable this icon-based-width behavior at all and be able to have smaller buttons than the icon would dictate. Cases where the icon has some white space on the edges or you need to have densely-packed controls in some admin window can benefit of this.

I'm not sure about modulates (although in Control context they should be called color), they can be kind of useful if you want to use a single icon, but aside from fast prototyping, there is little advantage over simply using colored icons.

If you only want to change the color and not the graphic, can be more straightforward. Tinting icons is used extensively in controls, sometimes with hardcoded values that are not exposed to the theme, for example for the disabled state.

That's what I think at least, the properties are still up to discussion. Keep in mind that it's easier to add a property than to delete it later.

Sure, and I need to make sure not to break existing stuff. More thorough refactorings have to wait a major version upgrade.
As I mentioned earlier in the discussion, this significant number of theme properties can be reduced by my idea of state-aware theme resources. For example, you may be able to set a single resource for the icon color, and it would manage on its own the normal/hover/pressed/etc... states without bloating up the properties list.

[Calinou]

Tested locally (rebased on top of master f6a78f8), it works as expected. This is a nice UX improvement 🙂

simplescreenrecorder-2024-04-04_17.23.35.mp4

In the editor with a light theme (theme switching works in the editor without requiring a restart):

[davthedev]

Rebased and code style changes are in - I believe this should be good to merge
@KoBeWi @akien-mga

[KoBeWi]

You didn't address all my comments though.

#89265 (comment) is still relevant.
Also #89265 (comment)

[davthedev]

Oops, thanks for pointing out the problem in EditorSpinSliders. Need fix before merge.

For the other comments, I already checked and:

Both arrows are greyed out when min and max are the same when running the project; works correctly. But I did not notice that the issue was when the control is rendered in edit mode. I have not worked with edit-time-specifics yet.

The presence of the "enlarge with icon size" option is explained above as well. To sum it up, here we have 3 cases:

• Fixed size when size is a fixed number, independent of the image width
• Size adjusted to image width when size is -1
• Whichever is greater between the two, similar as if you put a min-width in CSS (when the said option is enabled and size is a fixed number)

I have put a greater minimum button width than the icon itself in the editor & default themes to improve usability. For this purpose, I could have gone with the fixed size option. But this is going to break all projects in which the icon is set to a bigger one. This is why I added the third case.

We can remove the "enlarge with icon size" option, but we have to agree with having this possible project-breaking situation in this case.

[KoBeWi]

I have put a greater minimum button width than the icon itself in the editor & default themes to improve usability.

Ah, I guess that justifies it.
Though it means this property exists only for the sake of compatibility. It should be somehow marked as such (e.g. with a comment, or #ifndef DISABLE_DEPRECATED). I think you can implement the alternate negative behavior anyway, so if the property is removed in the future, the functionality is preserved.

But I did not notice that the issue was when the control is rendered in edit mode. I have not worked with edit-time-specifics yet.

tbh I'm not sure why it works at runtime, but not in the editor, but the issue is still there. It's very minor though, so it does not block merging I think.

[davthedev]

What do you mean by implementing the alternate negative behavior?

[davthedev]

For the missing arrows in the EditorSpinSliders, found the culprit. The EditorSpinSlider directly references theme elements from the SpinBox instead of having its own theme variables. I need to make adjustments on that side before we can merge.

[davthedev]

The missing arrows in EditorSpinSliders problem is now fixed; OK for merging on my side.

[Mickeon]

Suggested change

	Width of the up and down buttons. If smaller than any icon set on the buttons, the said icon may overlap neighboring elements, unless [theme_item set_min_buttons_width_from_icons] is enabled.
	Width of the up and down buttons. If smaller than any icon set on the buttons, the respective icon may overlap neighboring elements, unless [theme_item set_min_buttons_width_from_icons] is different than [code]0[/code].

[Mickeon]

I have to say that that a property name prefixed with set_ is rather unorthodox. I have no suggestions at the moment but it doesn't feel right.

[davthedev]

Would something like "use_icon_width_as_min_button_width" work out better?

Sure, "set" and "get" may seem odd here with a programmer mindset as this is not a proper setter/getter in OOP sense. More a contraction of "Set the minimum button width according to the icon width". I thought about it more from a user standpoint that may be not biased with programming terms.

[Mickeon]

Yes, I personally find that name to be much more suitable. Suggestions are still welcome if there's a briefer name.

[Mickeon]

I'd like elaboration on this. What KoBeWi is saying makes sense to me, as well.

[Mickeon]

All of these "graphic" below could be replaced by "style" (I've never seen these being referred as "graphic")

Suggested change

	Background graphic of the down button.
	Background style of the down button.

[davthedev]

Regarding set_min_buttons_width_from_icons, see my comment above on Jul 21 for explanation. Seems tricky to make a direct link to it.

[Mickeon]

Documentation is just fine now. My opinion on set_min_buttons_width_from_icons still remains but it shouldn't be an absolute blocker.

[akien-mga]

Thanks!