Allow adding buttons to the TabContainer header

[raulsntos]

Implements methods to add/remove buttons to the TabContainer header, as suggested by @YeldhamDev in #80227 (comment).

Preview

		LTR		RTL
		Start-aligned tabs	End-aligned tabs	Start-aligned tabs	End-aligned tabs
No clipping tabs	Only buttons
	With popup menu
Clipping tabs	Only buttons
	With popup menu

Example project

TabContainerButtons.zip

[YeldhamDev]

buttons_container should have some space on its sides separating it from both the tabs and the popup button, either a new theme constant for TabContainer, or reusing HBoxContainer's separation.

Besides that, it seems fine. However, the way it currently works (adding already created full-fledged Button nodes) goes a little different than what the add_button() methods from other nodes do (custom, more modest implementations). So I would like to hear what the rest of the @godotengine/gui-nodes team thinks.

[YuriSizov]

However, the way it currently works (adding already created full-fledged Button nodes) goes a little different than what the add_button() methods from other nodes do (custom, more modest implementations). So I would like to hear what the rest of the @godotengine/gui-nodes team thinks.

I agree that the current implementation seems extra. It would be better to mimic existing examples and add a method that takes a string, a icon, and an id. Then the TabBar would emit a signal with this id if the button was interacted with, and the method itself would also return a reference to the created button so you can do more complex things with it if you want.

---

Other than that, feature looks great. If it's not too much work, would make sense to make use of it in scene tabs in the same PR.

[raulsntos]

the way it currently works (adding already created full-fledged Button nodes) goes a little different than what the add_button() methods from other nodes do

For my use-case¹ I need to add Button and MenuButton, so passing an already constructed Button seemed like the easiest way to achieve this.

If it's not too much work, would make sense to make use of it in scene tabs in the same PR.

The scene tabs are using TabBar directly, not TabContainer, and this PR adds the API only to TabContainer (the popup_menu is also a TabContainer-only feature). Are you suggesting they should be moved to TabBar?

Footnotes

1. https://github.com/godotengine/godot/pull/80260. ↩

[YuriSizov]

The scene tabs are using TabBar directly, not TabContainer, and this PR adds the API only to TabContainer (the popup_menu is also a TabContainer-only feature).

Ah, right. Well, I was hoping we could remove the hackish code that we have to add buttons there, but I'm not sure if it fits the TabBar design. It does exactly the same thing as you want to add, so it feels justified, but maybe we need more opinions about that.

[YeldhamDev]

For my use-case I need to add Button and MenuButton, so passing an already constructed Button seemed like the easiest way to achieve this.

If that's the case, then maybe this should be generalized to allow to add any Control node.

Also, the separation between the clip buttons and the first custom one is different from the separation between each custom button:

[raulsntos]

If that's the case, then maybe this should be generalized to allow to add any Control node.

I'd be fine with that too. I didn't need other types of Controls, but if that's what the GUI team prefers I'll change it.

Also, the separation between the clip buttons and the first custom one is different from the separation between each custom button

I have changed the buttons_container separation to 0, and this is the result:

If I draw a red outline for each icon rect, and a green outline for each button rect. This shows that there is some separation caused by the button margin that I don't think we can get rid of, so the custom buttons will always look more separated.

This margin seems to be determined by the button styleboxes, but since we don't create the button it's up to the user to remove this margin by setting a StyleBoxFlat.

---

Drawing code

diff --git a/scene/gui/button.cpp b/scene/gui/button.cpp
index 430569432ab..8438f702091 100644
--- a/scene/gui/button.cpp
+++ b/scene/gui/button.cpp
@@ -228,6 +228,8 @@ void Button::_notification(int p_what) {
 				style2->draw(ci, Rect2(Point2(), size));
 			}
 
+			draw_rect(Rect2(Point2(), size), Color::named("GREEN"), false, 2.0);
+
 			Ref<Texture2D> _icon;
 			if (icon.is_null() && has_theme_icon(SNAME("icon"))) {
 				_icon = theme_cache.icon;
@@ -319,6 +321,8 @@ void Button::_notification(int p_what) {
 				if (icon_region.size.width > 0) {
 					Rect2 icon_region_rounded = Rect2(icon_region.position.round(), icon_region.size.round());
 					draw_texture_rect(_icon, icon_region_rounded, false, color_icon);
+
+					draw_rect(icon_region_rounded, Color::named("RED"), false, 2.0);
 				}
 			}
 
diff --git a/scene/gui/tab_bar.cpp b/scene/gui/tab_bar.cpp
index 959a51eff91..d0ec7153bc3 100644
--- a/scene/gui/tab_bar.cpp
+++ b/scene/gui/tab_bar.cpp
@@ -435,11 +435,15 @@ void TabBar::_notification(int p_what) {
 						draw_texture(theme_cache.decrement_icon, Point2(0, vofs), Color(1, 1, 1, 0.5));
 					}
 
+					draw_rect(Rect2(Point2(0, vofs), theme_cache.decrement_icon->get_size()), Color::named("RED"), false, 2.0);
+
 					if (offset > 0) {
 						draw_texture(highlight_arrow == 0 ? theme_cache.increment_hl_icon : theme_cache.increment_icon, Point2(theme_cache.increment_icon->get_size().width, vofs));
 					} else {
 						draw_texture(theme_cache.increment_icon, Point2(theme_cache.increment_icon->get_size().width, vofs), Color(1, 1, 1, 0.5));
 					}
+
+					draw_rect(Rect2(Point2(theme_cache.increment_icon->get_size().width, vofs), theme_cache.increment_icon->get_size()), Color::named("RED"), false, 2.0);
 				} else {
 					if (offset > 0) {
 						draw_texture(highlight_arrow == 0 ? theme_cache.decrement_hl_icon : theme_cache.decrement_icon, Point2(limit_minus_buttons, vofs));
@@ -447,11 +451,15 @@ void TabBar::_notification(int p_what) {
 						draw_texture(theme_cache.decrement_icon, Point2(limit_minus_buttons, vofs), Color(1, 1, 1, 0.5));
 					}
 
+					draw_rect(Rect2(Point2(limit_minus_buttons, vofs), theme_cache.decrement_icon->get_size()), Color::named("RED"), false, 2.0);
+
 					if (missing_right) {
 						draw_texture(highlight_arrow == 1 ? theme_cache.increment_hl_icon : theme_cache.increment_icon, Point2(limit_minus_buttons + theme_cache.decrement_icon->get_size().width, vofs));
 					} else {
 						draw_texture(theme_cache.increment_icon, Point2(limit_minus_buttons + theme_cache.decrement_icon->get_size().width, vofs), Color(1, 1, 1, 0.5));
 					}
+
+					draw_rect(Rect2(Point2(limit_minus_buttons + theme_cache.decrement_icon->get_size().width, vofs), theme_cache.increment_icon->get_size()), Color::named("RED"), false, 2.0);
 				}
 			}
 
diff --git a/scene/gui/tab_container.cpp b/scene/gui/tab_container.cpp
index e34e7d648c2..4a6aaf4e02c 100644
--- a/scene/gui/tab_container.cpp
+++ b/scene/gui/tab_container.cpp
@@ -208,8 +208,10 @@ void TabContainer::_notification(int p_what) {
 
 				if (menu_hovered) {
 					theme_cache.menu_hl_icon->draw(get_canvas_item(), Point2(x, (header_height - theme_cache.menu_hl_icon->get_height()) / 2));
+					draw_rect(Rect2(Point2(x, (header_height - theme_cache.menu_hl_icon->get_height()) / 2), theme_cache.menu_hl_icon->get_size()), Color::named("RED"), false, 2.0);
 				} else {
 					theme_cache.menu_icon->draw(get_canvas_item(), Point2(x, (header_height - theme_cache.menu_icon->get_height()) / 2));
+					draw_rect(Rect2(Point2(x, (header_height - theme_cache.menu_icon->get_height()) / 2), theme_cache.menu_icon->get_size()), Color::named("RED"), false, 2.0);
 				}
 			}
 

[YeldhamDev]

My idea was more having the separation distance be the same as the container's separation.

[raulsntos]

I'm not sure I understand, the container's separation is 0 now. We can increase the container's separation and use the same value to add separation between the container and the clip buttons and popup button, but the custom buttons will still have more separation because of the button stylebox.

[YeldhamDev]

We can increase the container's separation and use the same value to add separation between the container and the clip buttons and popup button

I think in the end that should be case. Also, the separation constant should be it's own theme item (that defaults to the HBox's value), since having to rely on a constant of another unrelated node (from the user's view at least) isn't great for discoverability.

[raulsntos]

I have added a new theme item button_separation (feel free to suggest a better name). Here's how it looks when the Button StyleBox is flat and the TabContainer has separation=8:

[YuriSizov]

For the sake of unblocking #80260 let's go for #80227 for 4.2 and give this one a bit more time to cook.

[YeldhamDev]

I have added a new theme item button_separation (feel free to suggest a better name).

I feel the name is as clear as it can get. 👍

Here's how it looks when the Button StyleBox is flat and the TabContainer has separation=8

Hum, I see that the default size is too much, I think 4 is the ideal.

[raulsntos]

Hum, I see that the default size is too much, I think 4 is the ideal.

Just to be clear, the default is 0 (I used 8 in the screenshot because I thought it would be easier to see). I can change the default to 4 if you want.

[YeldhamDev]

Yeah, let's try 4. I also hope I'm not being too much of a burden by frequently asking for those little modifications. 😛

[raulsntos]

Not at all. I misspoke earlier, the screenshot I shared was actually using a separation of 16 and yes that's excessive. Here's a few screenshots for comparison, the Buttons all have a flat StyleBox so they have no additional margin:

Separation	Preview
0
4
8

I have set the default value to 4, and renamed it to buttons_separation (notice the s in buttons) because I noticed there is another theme item in AcceptDialog with the same name and I thought we'd want to be consistent.

[YeldhamDev]

4 seems to be the sweet spot, so that nitpick is done.

However, I still think that it should be generalized to allow adding any Control node, so I would like for others in the @godotengine/gui-nodes team to chime in with their opinions.

[KoBeWi]

IMO unless you have a signal like custom_button_pressed (see Tree), there is no reason to limit the type.

[YeldhamDev]

The method names and docs should be updated to reflect the changes.

[kitbdev]

related: godotengine/godot-proposals#2250