Remove the Tabs node in favor of TabContainer

[Jummit]

Describe the project you are working on

Multiple projects where vertical tabs would be helpful.

Describe the problem or limitation you are having in your project

Currently TabContainer does not have an option to display vertical tabs. I started to implement it, the problem is that all of the tab functionality is duplicated for the Tabs control. The obvious move is to refactor the TabContainer to use it (see godotengine/godot#4512, but I propose to delete it. Here are the reasons:

1. It solves a bunch of problems by being a generic solution, instead of solving them with dedicated solutions (see: Why use Tabs below)

2. From my searches it is rarely used, which is against this:

3. In most cases, you can just use a TabContainer (kinda a repetition of point 2)

4. This is a weak point, but having both TabContainer and Tabs was confusing for me until I looked at the docs, because I didn't know what to use.

Why use Tabs instead of TabContainer?

I have only seen Tabs being used once, and it's for the scene tabs:

Tabs is used to add the tool panel between the tabs and the scene. This could easily be made a function of TabContainer with a method like set_middle_control. Again, point 1.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

By deleting Tabs I could finish implementing vertical tabs and the above mentioned set_middle_control, making it more powerful, without having to worry about refactoring it and moving the functionality to Tabs. It's my fault I started with TabContainer, but I think this is the right move nonetheless.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

• Remove or deprecate Tabs
• Add set_middle_control to TabContainer
• Make editor use set_middle_control here:
  https://github.com/godotengine/godot/blob/8a331070868cf4e4c5aa91d20ca3a22a8cc42a8e/editor/editor_node.cpp#L5971-L5987
• Add vertical tabs to TabContainer

If this enhancement will not be used often, can it be worked around with a few lines of script?

Vertical tabs could be implemented in GDScript, but this proposal is about the refactor to make vertical tabs and more possible in core.

Is there a reason why this should be core and not an add-on in the asset library?

It's a refactor in core.

[Arecher]

2. From my searches it is rarely used, which is against this:

We're using Tabs over TabContainer in our project at the moment :(
However, that was mostly due to the fact that we couldn't get the TabContainer working properly early on. I'm sure that I could (and probably should) easily configure the TabContainer without issue these days.

4. This is a weak point, but having both TabContainer and Tabs was confusing for me until I looked at the docs, because I didn't know what to use.

The Tabs vs TabContainer also confused me in the beginning, but that isn't exclusive to those two Nodes. ScrollContainer also exists alongside VScrollBar and HScrollBar. The one is simply a component of the other. I don't see why Tabs & TabContainer couldn't get the same treatment. Perhaps splitting Tabs into HTabs and VTabs, and having both easily configurable with a TabContainer would be a more consistent solution?

---

I realise this might be an odd point, but rather than the proposed set_middle_control, wouldn't it be more versatile to allow the user to indicate/link/set the node under which the scenes should be spawned? (Perhaps as long as it is a child of the TabContainer).

TabContainer
    VBoxContainer
        RandomScene: Bar with extra controls
        Control - **set as location for TabScenes to spawn**
        RandomScene: Bar with extra controls

I don't immediately have a lot of common use cases for this, but fixing any more complex setups of Tabs with only a set_middle_control seems like it would become a limitation quite quick.

[Jummit]

Perhaps splitting Tabs into HTabs and VTabs, and having both easily configurable with a TabContainer would be a more consistent solution?

The reason to remove Tabs is to avoid these "utility" classes.

wouldn't it be more versatile to allow the user to indicate/link/set the node under which the scenes should be spawned?

That is definitely be a better solution.

[YuriSizov]

The Tabs vs TabContainer also confused me in the beginning, but that isn't exclusive to those two Nodes. ScrollContainer also exists alongside VScrollBar and HScrollBar. The one is simply a component of the other. I don't see why Tabs & TabContainer couldn't get the same treatment.

And that is the point of the confusion. TabContainer is not related to Tabs whatsoever, they are two ways to do the same thing. And ScrollContainer is a complex control that utilizes dedicated scrollbar controls. TabContainer doesn't need any specific control as its child, any control would do.

I think it's fine this way and is the reason to remove Tabs. Many beginners think that they need to put Tabs inside of TabContainer, when this is actually going to break things instead. I don't think it makes much sense to have a reusable Tab component, when tabs don't make much sense outside of a container of tabs. Unlike scrollbars, which can be reused in a number of different cases.

I also don't think there is a need to mix vertical and horizontal tabs, so instead of making two controls it should just be a setting for the container.

[Arecher]

And that is the point of the confusion. TabContainer is not related to Tabs whatsoever,

Ah I see. I was under the presumption TabContainers used Tabs as one of it's core components. In that case I understand the need for removing one or the other.

[Jack2104]

Any updates?

[YuriSizov]

@Jack2104 10 days is usually not enough time to gather feedback on a big compability breaking change such as removing some existing functionality. We still need more people to agree on it to be considered, I think.

Everyone, yourself included, are free to make a PR, though.

[me2beats]

Tabs give higher flexibility when creating custom TabContainers.
For example now there is no way to add close button to tabs of TabContainer.
Also there is no way to add add tab button properly.

So I think at least these proposals should be implemented first
#2287
#2250

[YuriSizov]

So, after running into a lacking feature set of TabContainer while working on the Editor and thinking more about the usefulness of Tabs vs TabContainer, I think I don't support removing Tabs. I think this component needs to be repurposed, like it was proposed some 5 years ago in godotengine/godot#4512.

Currently in the Godot Editor we have only one use case for Tabs, and that's the scene tabs at the top of the editor. As far as I understand, tabs are used there due to the fact that you have manual control over when and how to draw tab's content. Namely, you can unload a node with tab's content from the tree completely, not just hide it, which I guess helps a lot with maintaining multiple scene tabs and their states.

Another reason to use Tabs there is related to additional controls, such as the add tab button and the distraction-free mode button. Tabs being a control responsible only for the tabbar can be reasoned with when you want to add extra controls there. However, to my surprise, those are basically hacked in with manual positioning and trick on what to do if there are too many tabs and the navigation buttons appear. I somehow believed that was an existing feature of Tabs, in some form. Incidentally, TabContainer has such a feature, although in a very limited way. Using set_popup you can make a button appear at the far right end of it, which would bring up a Popup-derived node. This was seemingly implemented specifically for docks, and thus lacks a lot of possible useful customization options, such as position and icon.

Tabs also have a built-in ability to be closed, though you don't control it on the tab by tab basis and instead select a "close button display policy" for the whole set. Alongside with the hacked in "add tab" button, this is the basic functionality that people can expect from tabs in modern UI, as people are extremely familiar with this in web browsers among other tools. And this is not present in TabContainer at all.

TabContainer on the other hand does a lot of manual drawing which basically means you have no control or information about its tabbar. This in turn means you have no means to actually impact the tabbar in the container, nor can you add controls there as you will.

I think that both nodes have a right to exist. They have useful perks to them and mixing all of them into a single node doesn't make much sense. However, they can synergize. So what I propose is the following:

1. TabContainer drops its own way to handle the tabbar and relies on Tabs instead. This means that Tabs becomes a hidden child node of TabContainer, similar to how many other complex nodes are. That Tabs child can be directly accessed with a new get_tabbar() method, which allows to keep tabbar specific API encapsulated in Tabs. Other child nodes added to TabContainer are handled as automatically generated tabs and fed to Tabs' add_tab() method. Signals are connected and TabContainer reacts on tabs changing like it does now.

TabContainer property	What happens to it
all_tabs_in_front	Moves to Tabs
current_tab	Already exists in Tabs
drag_to_rearrange_enabled	Already exists in Tabs, but see below
tab_align	Already exists in Tabs, should be named tabs_align probably
tabs_visible	Stays in TabContainer
use_hidden_tabs_for_min_size	Stays in TabContainer, but needs to be renamed to reflect that this is about content, not the tabbar

Most of the methods are either moved to Tabs or are already present there. get_tab_control() and get_current_tab_control() remain, though the latter is a very small convenience over the former. get/set_popup is removed in its current form as too limited.

Dealing with get/set_tabs_rearrange_group() is trickier, but I think we can safely remove them from TabContainer in favor the same logic in Tabs. We would need to create signals for tabs being rearranged, so that the content controls can be correctly moved. In fact, I'm not sure how you're supposed to keep track of content controls doing this in Tabs currently. So it should be an overall improvement.

2. Tabs is renamed to TabBar, because that's what it is and this way it has less chances to cause confusion. It retains most of its existing functionality, and gets things mentioned above. CloseButtonDisplayPolicy needs to be split into an actual display policy (only active tab, all tabs) and a per-tab setting that allows to close it. This way you can have some pinned tabs that cannot be closed with an X no matter what.

Closing should work more like a "close request" type of interaction, allowing users to define additional behavior before the tab is actually closed, or preventing it all together. Similar to how GraphNodes operate. Though I think it already works like that, so just making sure.

3. Add a more flexible way to add custom controls to TabBar. I would go off of the idea I've described in Allow TabContainers to position some child nodes in the tabs bar #2250 (comment). It makes sense to provide 4 locations for users to add controls to: far left, to the left of tabs, to the right of tabs, far right (the only current option). If we keep this limited to buttons, then users must be able to set the icon (and possible the title)? We can extend it to arbitrary controls, though, allowing groud to achieve what he wants with Allow TabContainers to position some child nodes in the tabs bar #2250.

As a built-in feature controls would be automatically repositioned when changes to tabbar happen, so this could be dropped.

The API can be as follows

enum ExtraControlPosition {
  CONTROL_FAR_LEFT,
  CONTROL_LEFT_OF_TABS,
  CONTROL_RIGHT_OF_TABS,
  CONTROL_FAR_RIGHT,
};

TabBar::set_extra_control(int p_position, Control *p_node);
TabBar::remove_extra_control(int p_position);

To restore the old set_popup functionality one would need to add a popup somewhere as well as set a button to CONTROL_FAR_RIGHT with an icon with 3 dots, and connect to its pressed signal to bring up the popup.

---

This proposal makes #2287, #2250 obsolete, and moves #2318, #2286 to Tabs/TabBar instead of TabContainer.

[Jummit]

Sorry, I read through your message multiple times but don't understand what you are getting at. What can Tabs do that can't be moved to TabsContainer?

[YuriSizov]

@Jummit
TabContainer operates on its children, while Tabs operate on an arbitrary set of "tabs" which you define. Tabs is just a tabbar and what happens when you interact with a tab of Tabs is up to you, the programmer. This is used in the editor to manage scene tabs without having all of them constantly on the tree. So when you select a scene tab, the content of the previous tab is removed from the tree and the content of the new tab is added to it. This is impossible to achieve with TabContainer, because for TabContainer tab's content defines the tab itself and so removing a child from TabContainer will remove the tab.

Basically, Tabs is a control for virtual tabs where you write custom logic to display some content corresponding to the tab selected. TabContainer is a control that turns its children into tabs. You can't change TabContainer so it supports virtual tabs without making it overly complicated. It makes more sense to keep tabbar logic in Tabs (and call it TabBar instead, as I propose), while TabContainer should be built on top of it by automatically creating tabs for its children and controlling their visibility.

Using TabContainer would be pretty much the same as before, just some APIs would be one more method call away (e.g. get_tabbar()->set_tab_title() instead of set_tab_title()).

PS. Alternative option would be to build them from the same base class, but that won't work well, with one being a container and the other not.

[Jummit]

You can't change TabContainer so it supports virtual tabs without making it overly complicated.

You can create dummy nodes to add tabs, similar to how you add dummy nodes to GraphNodes to add ports. You can then react to the tab switched signal and use it like Tabs. I stand by my point that Tabs is unnecessary generalization.

[YuriSizov]

We need to refactor TabContainer to support more of the asked for and required featured. Currently it draws tabs directly, which limits what we can do with them, and prevents us from sensibly implementing some proposals (I've linked a few).

So in the end, tabs of TabContainer would need to be its own set of controls, similar to how GraphEdit has a set of internal controls to make it work. At this point those controls wouldn't be much different from what Tabs is or can be with a little work. So we would still have Tabs for all intents and purposes, but unexposed to users, when we could expose it and allow people who need more control over the logic of tabs to use it. Like we already do need now and rely on Tabs for.

[me2beats]

Could be closed I think. Tabs is more flexible alternative to TabContainer. Currently TabContainer is limited, for example you can't customize it by adding buttons using add_child().
On the other hand I understand that Tabs can be confused with TabContainer (the user can think Tabs should be TabContainer children), but this is for another proposal imo.

[YuriSizov]

Could be closed I think.

It cannot be closed, because it's already in works, in a way.

Edit: Well, I guess that other way is #2514. But I'd still keep this open until we implement it.

[Jummit]

After thinking about it, I like #2514 's solution better. Closing as it has the same level of support.