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

Adds a hook for intercepting close requests. #1118

Merged
merged 2 commits into from
Aug 17, 2020
Merged

Adds a hook for intercepting close requests. #1118

merged 2 commits into from
Aug 17, 2020

Conversation

jneem
Copy link
Collaborator

@jneem jneem commented Aug 6, 2020

In druid-shell, a close request from the system will result in a call
to WinHandler::request_close instead of just immediately destroying
the window.

In druid, CLOSE_WINDOW commands are passed to the AppDelegate and down
the widget tree. Only if they are unhandled does the window get closed.

Edit: so far the druid-shell part is only implemented for gtk. Other platforms still close the window unconditionally.

Copy link
Collaborator

@luleyleo luleyleo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The missing changlog entry and one comment aside, this looks good, thanks.

Comment on lines 309 to 310
/// Returns `false` if the command was handled.
fn dispatch_cmd(&mut self, target: Target, cmd: Command) -> bool {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a bit backwards to me, I would expect true to indicate handled. This would also be in line with Window::event.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree and so I've changed it, but note that delegate_cmd and AppDelegate::command use the opposite convention.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, maybe we could define

type Handled = bool;
type Propagate = bool;

wonder if there is good prior art / reference for this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's worth having a enum { Handled, NotHandled}? Then there's no ambiguity.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that would be the "cleanest" solution I guess.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to do a PR for that or should I do it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this usage of enums/prefer it over bools,
I know at least petgraph uses these types of enums for edge directions, if people want to see a code base where that patterns been used.

https://docs.rs/petgraph/0.5.1/petgraph/enum.Direction.html

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could probably take a stab at a PR on the weekend, but if you've got time before that then go ahead.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I'll take care of it then.

@luleyleo luleyleo added the S-waiting-on-author waits for changes from the submitter label Aug 17, 2020
In druid-shell, a close request from the system will result in a call
to `WinHandler::request_close` instead of just immediately destroying
the window.

In druid, CLOSE_WINDOW commands are passed to the AppDelegate and down
the widget tree. Only if they are unhandled does the window get closed.
@luleyleo luleyleo removed the S-waiting-on-author waits for changes from the submitter label Aug 17, 2020
@jneem jneem merged commit 7b6f83e into linebender:master Aug 17, 2020
@jneem jneem deleted the quit branch August 17, 2020 14:35
@cmyr cmyr mentioned this pull request Aug 26, 2020
3 tasks
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.

3 participants