WerWolv / ImHex

πŸ” A Hex Editor for Reverse Engineers, Programmers and people who value their retinas when working at 3 AM.
https://imhex.werwolv.net
GNU General Public License v2.0
43.94k stars 1.92k forks source link

Update nativefiledialog and keep dialogs on top #1771

Closed btzy closed 2 months ago

btzy commented 3 months ago

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.)

WerWolv commented 3 months ago

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

btzy commented 3 months ago

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 commented 3 months ago

Hey @jonathanspw, could you by any chance bump the Fedora package of nativefiledialog-extended? πŸ₯ΊπŸ™

jonathanspw commented 3 months ago

Hey @jonathanspw, could you by any chance bump the Fedora package of nativefiledialog-extended? πŸ₯ΊπŸ™

Sure can! Will work on it now.

jonathanspw commented 3 months ago

https://bodhi.fedoraproject.org/updates/?search=nativefiledialog-extended-1.2.0

WerWolv commented 3 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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

WerWolv commented 2 months ago

Finally merged! Thanks a lot to everybody