-
Notifications
You must be signed in to change notification settings - Fork 920
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
feat: Event::Reopen
support for macOS
#3499
Conversation
Reopen
event support for macOSEvent::Reopen
support for macOS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see this being useful, thanks!
Co-authored-by: Mads Marquart <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nack for 0.30, should be good for 0.31 with traits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nack for 0.30, should be good for 0.31 with traits.
I'm not going to block this on 0.31, it's a simple change that we can do now, and generally enables better behaviour on macOS.
And what should we do with all of that on other platforms? The event is macOS specific, but it bleeds into other backend and people will demand its handling, while in reality such event doesn't exist for them. You should never trade quality for short term wins, this is just straight reduces overall quality because the event is not tied to window in particular and leaves the impression that such thing should be handled. |
I think the event is well documented as "Unsupported", which we already have precedent for, I don't see the issue with adding another one. In the end it won't really matter, since we're going to be changing things for 0.31 anyways. But like I said, in the months until that happens, this is a useful feature. |
No it's not, because it'll assume porting it to other backends as a top-level event. If you want this feature, require the new application trait for that and wire the vtable like we've discussed, so it won't bleed into any other backend. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I nominated this specifically because I wanted to discuss this "blocking it because its platform-specific" issue in general, but we didn't get to it unfortunately.
Personally I strongly disagree that this somehow reduces the quality of the Winit API, we already have a big amount of events that are not cross-platform, this doesn't change anything.
That said, I'm glad we are trying to fix/improve this issue in v0.31.
All As I said, the solution is already here even for 0.30, if you want I can write it myself for macOS. |
Instead of exposing this through Winit, #3758 is going to allow listening to |
CHANGELOG.md
if knowledge of this change could be valuable to usersDescription
This event is typically used to handle dock icon clicking event on macOS and uses
applicationShouldHandleReopen:hasVisibleWindows:
callback from AppKit Framework.Use case
It will make it possible to keep application running in backgound when all windows are closed, and create new window after dock icon clicked (application reopen callback).