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

Expose the TabBar of a TabContainer #80227

Merged
merged 1 commit into from
Sep 24, 2023

Conversation

raulsntos
Copy link
Member

Add TabContainer::get_tab_bar method to retrieve the TabBar of a TabContainer.

@raulsntos raulsntos added this to the 4.x milestone Aug 3, 2023
@raulsntos raulsntos requested a review from a team as a code owner August 3, 2023 18:59
@YeldhamDev
Copy link
Member

YeldhamDev commented Aug 3, 2023

In my original PR, where I made TabContainer use TabBar internally, I decided to not have this function because of how the former depended on the latter to be in sync with what was happening. If it's decided to expose it anyways, it should be stated in its doc description (which appears that there isn't even one yet) how fragile it is.

@raulsntos
Copy link
Member Author

I'm not totally sure I need this to be exposed, my goal is just to add buttons to the right of the TabBar of a TabContainer. Do you think this functionality should be exposed in a different way?

@raulsntos raulsntos requested a review from a team as a code owner August 3, 2023 19:20
@YeldhamDev
Copy link
Member

Funnily enough, there's a theme constant that does something similar, side_margin. However, the margin affected depends of the tab orientation.

@raulsntos
Copy link
Member Author

I'm not sure how I can use side_margin to achieve what I want.

For context, I'm using TabBar in #80260 to make the MSBuild panel more consistent with the rest of the editor. I'm trying to follow the same design as the scene tabs which also has buttons on the right side:

image
image

To achieve this I'm using this code1:

var tabs = TabContainer.new()

var tab_actions = HBoxContainer.new()
tab_actions.size_flags_vertical = Control.SIZE_EXPAND_FILL
tab_actions.size_flags_horizontal = Control.SIZE_EXPAND_FILL
tab_actions.alignment = BoxContainer.AlignmentMode.ALIGNMENT_END
tab_actions.set_anchors_and_offsets_preset(Control.LayoutPreset.PRESET_FULL_RECT)

tabs.get_tab_bar().add_child(tab_actions)

Footnotes

  1. The PR does this in C# but I translated it to GDScript here because it'd be easier to understand for other contributors.

@YeldhamDev
Copy link
Member

I'm not sure how I can use side_margin to achieve what I want.

I was just pointing out one of the node's features that could be modified to achieve that.

Considering that currently TabContainer also has the set/get_popup() methods, which adds a button to open a given popup, maybe this should be expanded further, adding an add_button() method as well?

@raulsntos
Copy link
Member Author

I've opened #80301 to implement your suggestion. Either way works for me, let me know which you prefer or if you want me to change it further.

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.

Generally speaking, I don't see any harm in this. We expose methods to get inner nodes all the time, and this one is pretty important to the affected container. That said, if for the particular problem we don't need this solution specifically, then perhaps we should wait for some demand.

Add `TabContainer::get_tab_bar` method to retrieve the `TabBar` of a `TabContainer`.
@YuriSizov YuriSizov modified the milestones: 4.x, 4.2 Sep 23, 2023
@akien-mga akien-mga merged commit 42fb795 into godotengine:master Sep 24, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

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.

4 participants