Skip to content

Add closures to AppDelegate #268

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Add closures to AppDelegate #268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 41 additions & 34 deletions druid/examples/multiwin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl EventCtxExt for EventCtx<'_, '_> {
}
}

impl EventCtxExt for DelegateCtx<'_, '_> {
impl EventCtxExt for DelegateCtx<'_> {
fn set_menu<T: 'static>(&mut self, menu: MenuDesc<T>) {
let cmd = Command::new(druid::command::sys::SET_MENU, menu);
self.submit_command(cmd, None);
Expand Down Expand Up @@ -82,40 +82,47 @@ fn ui_builder() -> impl Widget<State> {
}

fn make_delegate() -> AppDelegate<State> {
AppDelegate::new().event_handler(|event, data, _env, ctx| {
match event {
Event::Command(ref cmd) if cmd.selector == druid::command::sys::NEW_FILE => {
let new_win = WindowDesc::new(ui_builder).menu(make_menu(data));
let command = Command::new(druid::command::sys::NEW_WINDOW, new_win);
ctx.submit_command(command, None);
None
AppDelegate::new()
.event_handler(|event, data, _env, delegate_ctx, _| {
match event {
Event::Command(ref cmd) if cmd.selector == druid::command::sys::NEW_FILE => {
let new_win = WindowDesc::new(ui_builder).menu(make_menu(data));
let command = Command::new(druid::command::sys::NEW_WINDOW, new_win);
delegate_ctx.submit_command(command, None);
None
}
Event::Command(ref cmd) if cmd.selector == MENU_COUNT_ACTION => {
data.selected = *cmd.get_object().unwrap();
delegate_ctx.set_menu(make_menu::<State>(data));
None
}
// wouldn't it be nice if a menu (like a button) could just mutate state
// directly if desired?
Event::Command(ref cmd) if cmd.selector == MENU_INCREMENT_ACTION => {
data.menu_count += 1;
delegate_ctx.set_menu(make_menu::<State>(data));
None
}
Event::Command(ref cmd) if cmd.selector == MENU_DECREMENT_ACTION => {
data.menu_count = data.menu_count.saturating_sub(1);
delegate_ctx.set_menu(make_menu::<State>(data));
None
}
Event::MouseDown(ref mouse) if mouse.button.is_right() => {
let menu = ContextMenu::new(make_context_menu::<State>(), mouse.pos);
let cmd = Command::new(druid::command::sys::SHOW_CONTEXT_MENU, menu);
delegate_ctx.submit_command(cmd, None);
None
}
other => Some(other),
}
Event::Command(ref cmd) if cmd.selector == MENU_COUNT_ACTION => {
data.selected = *cmd.get_object().unwrap();
ctx.set_menu(make_menu::<State>(data));
None
}
// wouldn't it be nice if a menu (like a button) could just mutate state
// directly if desired?
Event::Command(ref cmd) if cmd.selector == MENU_INCREMENT_ACTION => {
data.menu_count += 1;
ctx.set_menu(make_menu::<State>(data));
None
}
Event::Command(ref cmd) if cmd.selector == MENU_DECREMENT_ACTION => {
data.menu_count = data.menu_count.saturating_sub(1);
ctx.set_menu(make_menu::<State>(data));
None
}
Event::MouseDown(ref mouse) if mouse.button.is_right() => {
let menu = ContextMenu::new(make_context_menu::<State>(), mouse.pos);
let cmd = Command::new(druid::command::sys::SHOW_CONTEXT_MENU, menu);
ctx.submit_command(cmd, None);
None
}
other => Some(other),
}
})
})
.window_added_handler(|id, _data, _env, _ctx| {
println!("Window added with ID: {:?}", id);
})
.window_removed_handler(|id, _data, _env, _ctx| {
println!("Window removed with ID: {:?}", id);
})
}

#[allow(unused_assignments)]
Expand Down
74 changes: 59 additions & 15 deletions druid/src/app_delegate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,12 @@ use std::collections::VecDeque;
use crate::{Command, Data, Env, Event, WinCtx, WindowId};

/// A context passed in to [`AppDelegate`] functions.
pub struct DelegateCtx<'a, 'b> {
pub struct DelegateCtx<'a> {
pub(crate) source_id: WindowId,
pub(crate) command_queue: &'a mut VecDeque<(WindowId, Command)>,
pub(crate) win_ctx: &'a mut dyn WinCtx<'b>,
}

impl<'a, 'b> DelegateCtx<'a, 'b> {
/// Get the [`WinCtx`].
///
/// [`WinCtx`] trait.WinCtx.html
pub fn win_ctx(&self) -> &dyn WinCtx<'b> {
self.win_ctx
}

impl<'a> DelegateCtx<'a> {
/// Submit a [`Command`] to be run after this event is handled.
///
/// Commands are run in the order they are submitted; all commands
Expand All @@ -56,13 +48,24 @@ impl<'a, 'b> DelegateCtx<'a, 'b> {
///
/// You customize the `AppDelegate` by passing closures during creation.
pub struct AppDelegate<T> {
event_fn: Option<Box<dyn Fn(Event, &mut T, &Env, &mut DelegateCtx) -> Option<Event> + 'static>>,
event_fn: Option<
Copy link
Member

Choose a reason for hiding this comment

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

I agree that this complexity isn't worth it, and we should just back up and make AppDelegate a trait.

If you'd like to try that, it's probably a separate PR, and then we could redo this on top of that? Or I could make that change, whichever is easiest.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I'd like to try my hand at that. I'll leave this as-is for now then.

Box<
dyn Fn(Event, &mut T, &Env, &mut DelegateCtx, &mut dyn WinCtx) -> Option<Event>
+ 'static,
>,
>,
window_added_fn: Option<Box<dyn Fn(WindowId, &mut T, &Env, &mut DelegateCtx)>>,
window_removed_fn: Option<Box<dyn Fn(WindowId, &mut T, &Env, &mut DelegateCtx)>>,
}

impl<T: Data> AppDelegate<T> {
/// Create a new `AppDelegate`.
pub fn new() -> Self {
AppDelegate { event_fn: None }
AppDelegate {
event_fn: None,
window_added_fn: None,
window_removed_fn: None,
}
}

/// Set the `AppDelegate`'s event handler. This function receives all events,
Expand All @@ -73,22 +76,63 @@ impl<T: Data> AppDelegate<T> {
/// the `update` method will be called as usual.
pub fn event_handler<F>(mut self, f: F) -> Self
where
F: Fn(Event, &mut T, &Env, &mut DelegateCtx) -> Option<Event> + 'static,
F: Fn(Event, &mut T, &Env, &mut DelegateCtx, &mut dyn WinCtx) -> Option<Event> + 'static,
{
self.event_fn = Some(Box::new(f));
self
}

pub fn window_added_handler<F>(mut self, f: F) -> Self
Copy link
Member

Choose a reason for hiding this comment

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

The one thing missing in this PR is docs for the public API.

where
F: Fn(WindowId, &mut T, &Env, &mut DelegateCtx) + 'static,
{
self.window_added_fn = Some(Box::new(f));
self
}

pub fn window_removed_handler<F>(mut self, f: F) -> Self
where
F: Fn(WindowId, &mut T, &Env, &mut DelegateCtx) + 'static,
{
self.window_removed_fn = Some(Box::new(f));
self
}

pub(crate) fn event(
&mut self,
event: Event,
data: &mut T,
env: &Env,
ctx: &mut DelegateCtx,
delegate_ctx: &mut DelegateCtx,
win_ctx: &mut dyn WinCtx,
) -> Option<Event> {
match self.event_fn.as_ref() {
Some(f) => (f)(event, data, env, ctx),
Some(f) => (f)(event, data, env, delegate_ctx, win_ctx),
None => Some(event),
}
}

pub(crate) fn window_added(
&mut self,
window_id: WindowId,
data: &mut T,
env: &Env,
ctx: &mut DelegateCtx,
) {
if let Some(f) = self.window_added_fn.as_ref() {
f(window_id, data, env, ctx);
}
}

pub(crate) fn window_removed(
&mut self,
window_id: WindowId,
data: &mut T,
env: &Env,
ctx: &mut DelegateCtx,
) {
if let Some(f) = self.window_removed_fn.as_ref() {
f(window_id, data, env, ctx);
}
}
}
32 changes: 29 additions & 3 deletions druid/src/win_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,14 +306,41 @@ impl<T: Data + 'static> AppState<T> {

fn connect(&mut self, id: WindowId, handle: WindowHandle) {
self.windows.connect(id, handle);
if let Some(delegate) = &mut self.delegate {
Copy link
Member

Choose a reason for hiding this comment

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

So these 10 lines or so are the main place here where I wonder if we can't reduce boilerplate. What I am imagining is a function on self with the signature,

fn with_delegate(&mut self, f: impl Fn(&mut AppDelegate, &mut DelegateCtx));

which internally does this whole dance of destructing self and creating the DelegateCtx, and then calls the passed closure with them.

If we had this, we could replace these 12 lines with,

self.with_delegate(|del, ctx| del.window_added(id, data, env, ctx));

Does that make sense?

let AppState {
ref mut command_queue,
ref mut data,
ref env,
..
} = self;
let mut ctx = DelegateCtx {
source_id: id,
command_queue,
};
delegate.window_added(id, data, env, &mut ctx)
}
}

pub(crate) fn add_window(&mut self, id: WindowId, window: Window<T>) {
self.windows.add(id, window);
}

fn remove_window(&mut self, id: WindowId) -> Option<WindowHandle> {
self.windows.remove(id)
let res = self.windows.remove(id);
if let Some(delegate) = &mut self.delegate {
let AppState {
ref mut command_queue,
ref mut data,
ref env,
..
} = self;
let mut ctx = DelegateCtx {
source_id: id,
command_queue,
};
delegate.window_removed(id, data, env, &mut ctx)
}
res
}

fn assemble_window_state<'a>(
Expand Down Expand Up @@ -413,9 +440,8 @@ impl<T: Data + 'static> AppState<T> {
let mut ctx = DelegateCtx {
source_id,
command_queue,
win_ctx,
};
delegate.event(event, data, env, &mut ctx)
delegate.event(event, data, env, &mut ctx, win_ctx)
}
None => Some(event),
}
Expand Down