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

Update nativefiledialog and keep dialogs on top #1771

Merged
merged 6 commits into from
Jul 3, 2024

Conversation

btzy
Copy link
Contributor

@btzy btzy commented Jun 24, 2024

This PR updates the nativefiledialog submodule and uses its new feature to set the ImHex main window as the parent of the dialog window. This ensures that the dialog stays on top of the main window. This is currently supported by NFDe on Windows, macOS, and Linux/X11. Linux/Wayland behaves as it did previously due to limitations in NFDe.

Note that macOS file dialogs have already been parented properly as NFDe previously used the key window (the window currently receiving keyboard events) on macOS. However, it's probably better to do the correct thing and pass the main window to NFDe even on macOS.

Problem description

The file dialog go behind the main window if the main window is clicked while the file dialog is open.

Implementation description

Update nativefiledialog and pass the GLFWwindow* of the main window to the library function.

Screenshots

Before:

output.mp4

After:

output2.mp4

Additional things

I have tested this on Windows and Linux/X11, but did not test this on macOS. It would be ideal if someone can help with this. (But as far as NFDe is concerned, macOS NSWindow* handles have been tested (with SDL2) and works.)

@WerWolv
Copy link
Owner

WerWolv commented Jun 24, 2024

Hey! That's awesome thank you very much!
I can test it on macOS later, no problem

@btzy
Copy link
Contributor Author

btzy commented Jun 24, 2024

It seems that the CI build failures on macOS 12 and WebAssembly are unrelated, but the failures on Fedora are because they are using the system NFD library, which isn't updated to the latest version yet.

@WerWolv
Copy link
Owner

WerWolv commented Jun 24, 2024

Hey @jonathanspw, could you by any chance bump the Fedora package of nativefiledialog-extended? 🥺🙏

@jonathanspw
Copy link
Contributor

Hey @jonathanspw, could you by any chance bump the Fedora package of nativefiledialog-extended? 🥺🙏

Sure can! Will work on it now.

@jonathanspw
Copy link
Contributor

@WerWolv
Copy link
Owner

WerWolv commented Jun 24, 2024

Thanks a lot! I guess it takes a while for the repo to update. I'll leave this PR open for now until the fedora runners succeed as well, the rest looks good to me 👍

@WerWolv WerWolv enabled auto-merge (squash) July 2, 2024 05:15
@WerWolv
Copy link
Owner

WerWolv commented Jul 2, 2024

My god the fedora packages take two full weeks to get to stable... Could you possibly give the packages a thumb up like I did? @btzy Then they should hopefully get added right away

@btzy
Copy link
Contributor Author

btzy commented Jul 2, 2024

I think it's supposed to take a week, which is now over. It's in the "testing->stable" state now, so if I'm understanding it correctly, it should be pushed to stable within the next 24 hours.

@jonathanspw
Copy link
Contributor

My god the fedora packages take two full weeks to get to stable... Could you possibly give the packages a thumb up like I did? @btzy Then they should hopefully get added right away

It only takes a week.

I think it's supposed to take a week, which is now over. It's in the "testing->stable" state now, so if I'm understanding it correctly, it should be pushed to stable within the next 24 hours.

Correct, testing -> stable means it will hit stable in tonight's compose. This time tomorrow it will be in stable repos.

@WerWolv
Copy link
Owner

WerWolv commented Jul 2, 2024

Ahh sorry, I saw 7 days down at the bottom and though it reset when it switched stage

@WerWolv WerWolv merged commit dd60762 into WerWolv:master Jul 3, 2024
18 checks passed
@WerWolv
Copy link
Owner

WerWolv commented Jul 3, 2024

Finally merged! Thanks a lot to everybody

WerWolv added a commit that referenced this pull request Jul 9, 2024
This PR updates the nativefiledialog submodule and uses its new feature
to set the ImHex main window as the parent of the dialog window. This
ensures that the dialog stays on top of the main window. This is
currently supported by NFDe on Windows, macOS, and Linux/X11.
Linux/Wayland behaves as it did previously due to limitations in NFDe.

Note that macOS file dialogs have already been parented properly as NFDe
previously used the key window (the window currently receiving keyboard
events) on macOS. However, it's probably better to do the correct thing
and pass the main window to NFDe even on macOS.

### Problem description
The file dialog go behind the main window if the main window is clicked
while the file dialog is open.

### Implementation description
Update nativefiledialog and pass the `GLFWwindow*` of the main window to
the library function.

### Screenshots
Before:


https://github.com/WerWolv/ImHex/assets/6948096/589c3401-702a-4b0a-99ed-02d3e4d9080e

After:


https://github.com/WerWolv/ImHex/assets/6948096/8fef4900-eedc-48d5-8a4e-7bd81e37e3c0

### Additional things
I have tested this on Windows and Linux/X11, but did not test this on
macOS. It would be ideal if someone can help with this. (But as far as
NFDe is concerned, macOS `NSWindow*` handles have been tested (with
SDL2) and works.)

Co-authored-by: Nik <werwolv98@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants