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

Add FoldableContainer Node #87559

Closed

Conversation

WhalesState
Copy link
Contributor

@WhalesState WhalesState commented Jan 25, 2024

Bugsquad edit: closes godotengine/godot-proposals#8974

A Container similar to EditorInspector Sections!

FoldableContainer.mp4

WIP!
1- Generate class docs.
2- Allow RTL text orientation.
3- Allow text translation.
4- Add font outlines.
5- Add more advanced text styling [clipping text and fill alignment] to optionally respect the title width when text changes, similar to Button text behavior.

@WhalesState WhalesState requested review from a team as code owners January 25, 2024 01:20
@WhalesState WhalesState marked this pull request as draft January 25, 2024 01:20
@WhalesState WhalesState force-pushed the foldable-container branch 6 times, most recently from 202ceca to 128a71e Compare January 25, 2024 03:13
@WhalesState
Copy link
Contributor Author

I have tried to override the theme stylebox with a null value for TabBar, and it throws this error only without pausing the game!
Condition "!p_style.is_valid()" is true. <C++ Source> scene\gui\control.cpp:2833 @ Control::add_theme_style_override()
and in TabBar it's only checking if the Tab icon is valid!
is it safe to use theme_cache.styleboxes directly without checking if they are valid?

@YuriSizov
Copy link
Contributor

This needs a proposal.

@WhalesState
Copy link
Contributor Author

WhalesState commented Jan 25, 2024

This needs a proposal.

Yes, but it's still a work in progress since I'm learning more about TextLine class now, after finishing it we can discuss it's usability in a feature proposal, since it can be helpful for game menus, programs UI and the editor because it can replace the editor inspector sections and can be used in project and editor settings to simplify the UI.

@YuriSizov
Copy link
Contributor

YuriSizov commented Jan 25, 2024

Well, we might reject the proposal because a foldable container can be easily done with a script, for example. So I would rather you not wasted your time unless you know this is desired.

Remember, proposal comes before a PR.

@WhalesState
Copy link
Contributor Author

WhalesState commented Jan 25, 2024

Well, we might reject the proposal because a foldable container can be easily done with a script, for example. So I would rather you not wasted your time unless you know this is desired.

Remember, proposal comes before a PR.

it will have a limitation with theming, this is why i have started it, i do have the same Container written in gdscript, and to customize it you need to create a custom_type, and if the project doesn't have a project_theme in project setting it will be hard to find the theme resource to add the custom_type to it, also you will have to keep the theme styleboxes, constants, colors, fonts as variables, and without a theme changing a stylebox property such as color it will not load since they don't have signals to emit to call queue_redraw() to redraw the styleboxes.

@KoBeWi
Copy link
Member

KoBeWi commented Jan 25, 2024

without a theme changing a stylebox property such as color it will not load since they don't have signals to emit to call queue_redraw() to redraw the styleboxes.

Pretty sure you can use NOTIFICATION_THEME_CHANGED and connect to the stylebox changed signal if the notification alone doesn't suffice.

@WhalesState WhalesState marked this pull request as ready for review February 12, 2024 15:50
@WhalesState WhalesState requested a review from a team as a code owner February 12, 2024 15:50
scene/gui/foldable_container.h Outdated Show resolved Hide resolved
scene/gui/foldable_container.cpp Outdated Show resolved Hide resolved
scene/gui/foldable_container.cpp Outdated Show resolved Hide resolved
scene/gui/foldable_container.h Outdated Show resolved Hide resolved
scene/gui/foldable_container.cpp Outdated Show resolved Hide resolved
scene/gui/foldable_container.cpp Outdated Show resolved Hide resolved
doc/classes/FoldableContainer.xml Outdated Show resolved Hide resolved
@Mickeon
Copy link
Contributor

Mickeon commented Feb 12, 2024

If you could roughly fill in the documentation it would be heavily appreciated. Since you are the one that developed the Node in the first place, it should be easier for you to document and note down any additional quirk, as well.
I can take a look at it for additional polish at a later time.

ClassDB::bind_method(D_METHOD("set_text_direction", "text_direction"), &FoldableContainer::set_text_direction);
ClassDB::bind_method(D_METHOD("get_text_direction"), &FoldableContainer::get_text_direction);

ADD_SIGNAL(MethodInfo("expanded", PropertyInfo(Variant::BOOL, "is_expanded")));
Copy link
Member

@AThousandShips AThousandShips Feb 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't name the signal and the property the same

Edit: and also expanded is IMO a non-correct name as it happens when both collapsed and expanded, so something else like expansion_changed might be better, or folding_changed

I'd prefer flipping the condition though and call the property folded and have it be false by default

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I also was wondering about that error. Thank you all.

@WhalesState WhalesState force-pushed the foldable-container branch 3 times, most recently from b5de14e to c9d2463 Compare February 12, 2024 17:20
@WhalesState WhalesState reopened this Feb 12, 2024
@WhalesState
Copy link
Contributor Author

WhalesState commented Feb 12, 2024

Sorry about the noise again, sometimes I forget to commit the changes before doing a force push.

@WhalesState WhalesState marked this pull request as draft September 19, 2024 02:33
@WhalesState WhalesState deleted the foldable-container branch October 25, 2024 12:04
@AThousandShips AThousandShips removed this from the 4.x milestone Oct 25, 2024
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.

Add FoldableContainer Node
6 participants