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.

Sign up for GitHub

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

Jump to bottom

Hide animation toolbar above the viewport correctly when switching scenes #81606

Merged
merged 1 commit into from
Sep 15, 2023
Merged

Hide animation toolbar above the viewport correctly when switching scenes #81606

merged 1 commit into from
Sep 15, 2023

Conversation

jsjtxietian
Copy link
Contributor

Fixes #81558

@AThousandShips AThousandShips added this to the 4.2 milestone Sep 13, 2023
@YuriSizov YuriSizov requested review from YuriSizov and a team September 14, 2023 16:55
YuriSizov
YuriSizov reviewed Sep 14, 2023
View reviewed changes
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.

So I got curious how come the method with that name was never connected to a signal with that name. Apparently, it was! And then it got changed 8 years ago in f51c26c.

I'm not exactly sure why. The signal was moved to NOTIFICATION_VISIBILITY_CHANGED. But then the bind was also changed to react on visibility changes for some reason. This is still the case, even though the code is slightly different. I think you can replace the the line above with the one that you've added without any loss of functionality.

I tested it and didn't see any immediate issues.

@jsjtxietian jsjtxietian force-pushed the hide-animation-tool--correctly-when-switching-scenes branch from bd07fa4 to 0113e69 Compare September 15, 2023 02:49
@jsjtxietian
Copy link
Contributor Author

I think you can replace the the line above with the one that you've added without any loss of functionality.

I did some test and I did not find any immediate issues too.

@YuriSizov YuriSizov merged commit 2febb24 into godotengine:master Sep 15, 2023
15 checks passed
@YuriSizov
Copy link
Contributor

Thanks!

@jsjtxietian jsjtxietian deleted the hide-animation-tool--correctly-when-switching-scenes branch October 18, 2023 03:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YuriSizov YuriSizov YuriSizov approved these changes

Assignees
No one assigned
Labels
bug topic:animation topic:editor usability
Projects
None yet
Milestone
4.2
Development

Successfully merging this pull request may close these issues.

Animation tools do not get hidden correctly when switching scenes
3 participants
@jsjtxietian @YuriSizov @AThousandShips