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

Keep focus on floating window when showing ProgressDialog #83290

Merged

Conversation

trollodel
Copy link
Contributor

@trollodel trollodel commented Oct 13, 2023

Fixes #78672

This uses the first approach described in #78672 (comment)

I tested this only on Linux (KDE neon 5.27 22.04 - X11 - Vulkan (Forward+) - dedicated AMD Radeon Graphics (RADV NAVI23) - AMD Ryzen 5 5600 6-Core Processor (12 Threads)).
Other OSes should be tested as well, just in case any of them doesn't like quick focus changes.


I have done this patch ~2 months ago, but I never get to publish it 🙈.
Initially, I want to base this PR to #79261, but it's not merged.

@trollodel trollodel force-pushed the move_progress_dialog_on_open branch 2 times, most recently from 841db2c to be88020 Compare October 13, 2023 20:20
@AThousandShips AThousandShips added this to the 4.2 milestone Oct 14, 2023
@trollodel trollodel force-pushed the move_progress_dialog_on_open branch from be88020 to 9ae48c7 Compare October 15, 2023 20:13
@akien-mga
Copy link
Member

Initially, I want to base this PR to #79261, but it's not merged.

How different would this PR be with #79261? That PR was moved to 4.3 due to feature freeze, but if it helps simplify a bugfix, it could still be merged now.

@KoBeWi
Copy link
Member

KoBeWi commented Oct 16, 2023

That PR would allow to get rid of the whole host_windows vector, so it would simplify the code a lot.

Though the current code is only limited to the wrapped windows, so we won't get popups or transient windows unexpectedly getting progress dialog. This is a safer approach, not sure if it matters.

@Sauermann
Copy link
Contributor

How different would this PR be with #79261? That PR was moved to 4.3 due to feature freeze, but if it helps simplify a bugfix, it could still be merged now.

I would recommend to not consider #79261 for 4.2, because it changes things, that would benefit from testing.

@KoBeWi
Copy link
Member

KoBeWi commented Oct 16, 2023

btw the linked issue is already "fixed" so this PR can be postponed too probably.

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.

The code looks fine to me, although I think this should be a part of the popup behavior in the editor, not a part of ProgressDialog specifically. But we can look into it later.

@YuriSizov YuriSizov modified the milestones: 4.3, 4.2 Nov 15, 2023
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

I tested it on Linux X11 (KDE) a few days ago, worked fine for me.

@akien-mga akien-mga merged commit d89c531 into godotengine:master Nov 15, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

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.

Saving script from floating script window moves window focus back to the main editor window
7 participants