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

Added the smart word wrap property to preview label in batch rename dialog #52690

Merged
merged 1 commit into from
Jan 20, 2022

Conversation

nsrCodes
Copy link
Contributor

closes #52074

batch-rename.mp4

@nsrCodes
Copy link
Contributor Author

The mentioned bug is fixed but this raises another problem that the height of the label does not shrink after the label is resized (when each new line is added).
I was expecting this to be covered in another issue because it has to do with the minimum_size_changed function in control.cpp. AFAIU, it does not resize the node if its children are still inside, which leaves the empty gaps

Demo of the new issue:

batch-rename-new-glitch.mp4

@YuriSizov YuriSizov requested a review from a team September 15, 2021 14:47
@YuriSizov YuriSizov added this to the 4.0 milestone Sep 15, 2021
@nsrCodes
Copy link
Contributor Author

nsrCodes commented Sep 17, 2021

The mentioned bug is fixed but this raises another problem that the height of the label does not shrink after the label is resized (when each new line is added).

This bug already exists in master. You can check the same by just toggling the advanced options, it does not resize correctly.

@LoipesMas
Copy link
Contributor

Here's how it looks on current master:

batch_rename2.mp4

The original issue (text getting out of the window) isn't there. It's only on 3.x branch.
I'm not sure if cherry-picking this PR for 3.x would fix that.

Maybe make a separate PR for 3.x to fix size not updating and another one for master to set the autowrap (or just use this PR for that, just changing the title).

@nsrCodes
Copy link
Contributor Author

I have found the reason of the bug in master but currently can't find a way to fix it.
Before #37317 the dialog was implemented using control and there if we set the value of size.y to 0 (which is done while toggling the advanced option button), the size computed for the re-render was equal to the minimum size required to fit the children of that dialog (follow the get_combined_minimum_size function in _update_minimum_size).

But after that PR, dialogs are now implemented using window where the _update_window_size does not let the window shrink back. This is because along with the minimum size of the window, it also tracks the dynamically updated window size and always chooses the max of the two.

@akien-mga akien-mga requested review from a team and bruvzg January 20, 2022 16:27
@akien-mga akien-mga merged commit 8bb98ad into godotengine:master Jan 20, 2022
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@akien-mga akien-mga added cherrypick:3.4 cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Jan 20, 2022
@akien-mga
Copy link
Member

Cherry-picked for 3.5.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Jan 25, 2022
@akien-mga
Copy link
Member

Cherry-picked for 3.4.3.

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.

[3.x] Batch rename window size not updating correctly
5 participants