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

Add programming interface for modal dialogs #57

Closed
wants to merge 1 commit into from
Closed

Add programming interface for modal dialogs #57

wants to merge 1 commit into from

Conversation

mhjlam
Copy link

@mhjlam mhjlam commented Aug 26, 2018

Greetings! I noticed that nfd does not currently support modal dialogs; windows that force the user to interact with it before returning to a parent application. As I needed this behavior I have adapted the code of this library rather than write the whole thing myself. Perhaps the changes are useful for others as well.

Note that I have only tested this on Windows 10. The changes made for the GTK version should be correct as well, looking at the API documentation. Furthermore, since I have no experience with Cocoa, I am uncertain how modals work on OSX, but a quick glance at the API suggests that calling function runModal should already open a modal window as is in the current version.

@mlabbe
Copy link
Owner

mlabbe commented Dec 30, 2018

Thanks for sharing your contribution. An acceptable pull request would meet the guidelines in the submitting pull requests.md: https://github.com/mlabbe/nativefiledialog/blob/master/docs/submitting_pull_requests.md which highlights that the code has to be fully tested on all platforms. Testing on all platforms is hard which is why the api implementer has to do it; it spares the programmers.

In an acceptable PR, I would also avoid doubling the API surface area, and consider a function to enable or disable modality globally as an alternative.

I think the idea of modal dialogs is useful and I hope a pull request that meets the standards laid out comes up eventually.

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.

2 participants