Skip to content

Editor: Fix Tag Overrun Behavior#119088

Open
unit-tick wants to merge 1 commit intogodotengine:masterfrom
unit-tick:tag_overrrun_fix
Open

Editor: Fix Tag Overrun Behavior#119088
unit-tick wants to merge 1 commit intogodotengine:masterfrom
unit-tick:tag_overrrun_fix

Conversation

@unit-tick
Copy link
Copy Markdown
Contributor

Supplemental Fix to #118876. In that PR I was unable to set the overrun behaviour as recommended by @KoBeWi in this comment. This fix gets the overrun to behave properly by utilising the set_custom_minimum_size by plugging in the size it currently is, this is done when NOTIFICATION_READY is emitted.

proper_overrun

@unit-tick unit-tick requested a review from a team as a code owner April 29, 2026 15:30
@AThousandShips AThousandShips changed the title Editor: Fix Tag Overrun Behaviour Editor: Fix Tag Overrun Behavior Apr 29, 2026
@AThousandShips AThousandShips added this to the 4.x milestone Apr 29, 2026
@KoBeWi
Copy link
Copy Markdown
Member

KoBeWi commented Apr 29, 2026

Why is this in NOTIFICATION_READY, not the constructor?

@unit-tick
Copy link
Copy Markdown
Contributor Author

get_size does not return a value in the constructor.

@KoBeWi
Copy link
Copy Markdown
Member

KoBeWi commented Apr 29, 2026

That's very hacky, but if it works... 🤔

@unit-tick
Copy link
Copy Markdown
Contributor Author

Yeah, I spent the better part of a day trying to figure this out. At first I used get_minimum_size but the sizes it returned was all over the place. Using only x values, it returned 33px for the "Test" tag, which required 45px and 112 for "Development" which required 104px.

Copy link
Copy Markdown
Contributor

@StarryWorm StarryWorm left a comment

Choose a reason for hiding this comment

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

Ideally, buttons would properly support the flag, but that would require changes out of scope for 4.7 (and yes, I will get it done for 4.8).
The solution you put forward is rather hacky, as KoBeWi pointed out, but it's contained to the editor tags, so I support it as a temporary solution. If it works, it works.

@unit-tick
Copy link
Copy Markdown
Contributor Author

Yeah, I don't mind cleaning this up when buttons use internal labels as it'll no longer be a necessary bandaid.

@KoBeWi
Copy link
Copy Markdown
Member

KoBeWi commented Apr 29, 2026

You could add a FIXME or HACK comment pointing at the better solution.

@KoBeWi KoBeWi modified the milestones: 4.x, 4.7 Apr 29, 2026
@unit-tick unit-tick force-pushed the tag_overrrun_fix branch 2 times, most recently from e3d0283 to be8d7f4 Compare April 29, 2026 20:06
Copy link
Copy Markdown
Contributor

@Repiteo Repiteo left a comment

Choose a reason for hiding this comment

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

No need for a HACK and FIXME comment; just the one will suffice

Comment thread editor/project_manager/project_tag.cpp Outdated
Comment on lines +41 to +44
//HACK: can't be set in constructor because get_size() returns empty, will be made unnecessary when Button utilizes internal labels.
if (p_what == NOTIFICATION_READY) {
button->set_custom_minimum_size(get_size());
button->set_text_overrun_behavior(TextServer::OverrunBehavior::OVERRUN_TRIM_ELLIPSIS); //FIXME: Move to constructor when Button utilizes internal labels.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Adjusted to fit our comment style guidelines:

Suggested change
//HACK: can't be set in constructor because get_size() returns empty, will be made unnecessary when Button utilizes internal labels.
if (p_what == NOTIFICATION_READY) {
button->set_custom_minimum_size(get_size());
button->set_text_overrun_behavior(TextServer::OverrunBehavior::OVERRUN_TRIM_ELLIPSIS); //FIXME: Move to constructor when Button utilizes internal labels.
// HACK: Can't set overrun behavior in constructor because `get_size()` would return empty.
// This logic should be migrated once `Button` utilizes internal labels.
if (p_what == NOTIFICATION_READY) {
button->set_custom_minimum_size(get_size());
button->set_text_overrun_behavior(TextServer::OverrunBehavior::OVERRUN_TRIM_ELLIPSIS);

@unit-tick unit-tick force-pushed the tag_overrrun_fix branch 2 times, most recently from 985e26e to 6bfce50 Compare April 30, 2026 16:04
Fixes an issue with not being able to set overrun behaviour on tags
without the button size zeroing out.
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.

5 participants