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 wrappers for starting and stopping modal windows #129

Open
wants to merge 6 commits into
base: trunk
Choose a base branch
from

Conversation

hwright
Copy link

@hwright hwright commented Oct 3, 2024

Add appropriate wrappers to App for the following underlying NSApplication methods:

This also adds an implementation of Into for ModalResponse, which was needed by the above wrappers.

One open question concerns the location of ModalResponse within cacao. Previous to this change, ModalResponse was only used by the filesystem module, but this extends its use more broadly. Within AppKit, ModalResponse is a subclass of NSApplication, so maybe moving it to the app module in cacao is more appropriate. We don't need to make that change now, just pointing out the inconsistency.

Tested using a local project invoking these methods in place of their own implementations.

@ryanmcgrath
Copy link
Owner

Appreciate the PR. :)

It's been a few years since I touched this particular functionality, but (IIRC) I think I opted not to expose these because modern macOS modal views should really be using the sheets API. The filesystem module uses this API as it just works around the thorny issue of handling file selection in a synchronous manner - this is typically an operation where "stop the world" isn't the end of the world.

That said, I think the PR is probably still useful - but I'd maybe add some additional documentation to the function docs that just notes the above. I also think you're right and that ModalResponse should probably just move out if these methods are public.

…:app module.

It is now being used outside of filesystem, and this more closely mirrors
the location of this enum with AppKit.
@hwright
Copy link
Author

hwright commented Oct 3, 2024

Documentation added, and ModalResponse moved.

Specifically, allow windows with delegates to be used.
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