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

Allow closing scene tab preview using Escape #90720

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

tbreese3
Copy link
Contributor

@tbreese3 tbreese3 commented Apr 15, 2024

Fixes #90706

This PR is a potential implementation for closing the tooltip window which appears after hovering over a tab in the editor.

To test this code, open multiple scenes in the editor, hover over an inactive scene and once the panel pops up, press escape to close it.

@tbreese3 tbreese3 requested review from a team as code owners April 15, 2024 21:31
@tbreese3 tbreese3 changed the title Command to close tooltip panel for tabs Command to close preview panel for inactive tabs Apr 15, 2024
@RedMser
Copy link
Contributor

RedMser commented Apr 15, 2024

Instead of a new keybind, you can check for ui_cancel since that's what tooltips also do.

@tbreese3
Copy link
Contributor Author

tbreese3 commented Apr 15, 2024

I modified it to check for ui_cancel. That being said, I feel like it should go in EditorSceneTabs::_scene_tab_input, but when I put it in there, I get very inconsistent behavior because events aren't sent there after adding a new tab for some reason. If it should be in there, I could spend some time debugging and trying to figure out why a call isn't always sent to EditorSceneTabs::_scene_tab_input, just let me know.

@tbreese3
Copy link
Contributor Author

I made some changes based on your comments. Sorry if my solution is kind of naive, this is my first time working with open source. Also, let me know if there is anything else that should be done differently. Thank you for the feedback.

@tbreese3 tbreese3 requested a review from KoBeWi April 16, 2024 19:48
@RedMser

This comment was marked as outdated.

@tbreese3
Copy link
Contributor Author

tbreese3 commented Apr 16, 2024

It should be squashed down to one commit now. Is there anything else I should do or need to change? (also sorry, I just saw you said after it's approved, I can squash it again after if I need to make another commit)

@tbreese3 tbreese3 requested a review from KoBeWi April 16, 2024 20:03
Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

Left some more style comments, but the implementation looks alright now.

@tbreese3
Copy link
Contributor Author

Okay, sounds good, thanks for taking the time to look at this and giving me feedback. I'm super new to this codebase so it was very helpful.

@RedMser

This comment was marked as outdated.

@KoBeWi
Copy link
Member

KoBeWi commented Apr 16, 2024

You need to squash again.

@KoBeWi KoBeWi changed the title Command to close preview panel for inactive tabs Allow closing scene tab preview using Escape Apr 16, 2024
@akien-mga akien-mga merged commit dc68025 into godotengine:master Apr 17, 2024
16 checks passed
@akien-mga
Copy link
Member

akien-mga commented Apr 17, 2024

Thanks! And congrats for your first merged Godot contribution 🎉

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.

Scene preview tooltips can't be closed with Escape
5 participants