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

Replace generic commands with methods on EventCtx and DelegateCtx #931

Merged
merged 8 commits into from
May 15, 2020
Merged
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ This means that druid no longer requires cairo on macOS and uses Core Graphics i
- `Event::Wheel` now contains a `MouseEvent` structure. ([#895] by [@teddemunnik])
- `AppDelegate::command` now receives a `Target` instead of a `&Target`. ([#909] by [@xStrom])
- `SHOW_WINDOW` and `CLOSE_WINDOW` commands now only use `Target` to determine the affected window. ([#928] by [@finnerale])
- Replaced `NEW_WINDOW`, `SET_MENU` and `SHOW_CONTEXT_MENU` commands with methods on `EventCtx` and `DelegateCtx`. ([#931] by [@finnerale])

### Deprecated

Expand Down Expand Up @@ -187,6 +188,7 @@ This means that druid no longer requires cairo on macOS and uses Core Graphics i
[#925]: https://github.com/xi-editor/druid/pull/925
[#928]: https://github.com/xi-editor/druid/pull/928
[#930]: https://github.com/xi-editor/druid/pull/930
[#931]: https://github.com/xi-editor/druid/pull/931
[#940]: https://github.com/xi-editor/druid/pull/940
[#942]: https://github.com/xi-editor/druid/pull/942
[#943]: https://github.com/xi-editor/druid/pull/943
Expand Down
57 changes: 19 additions & 38 deletions druid/examples/multiwin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
//! Opening and closing windows and using window and context menus.
use druid::widget::prelude::*;
use druid::widget::{Align, BackgroundBrush, Button, Flex, Label, Padding};
use druid::widget::{
Align, BackgroundBrush, Button, Controller, ControllerHost, Flex, Label, Padding,
};
use druid::{
commands as sys_cmds, AppDelegate, AppLauncher, Application, Color, Command, ContextMenu, Data,
DelegateCtx, LocalizedString, MenuDesc, MenuItem, Selector, Target, WindowDesc, WindowId,
Expand Down Expand Up @@ -49,19 +51,6 @@ pub fn main() {
.expect("launch failed");
}

// this is just an experiment for how we might reduce boilerplate.
trait EventCtxExt {
fn set_menu<T: 'static>(&mut self, menu: MenuDesc<T>);
}

impl EventCtxExt for EventCtx<'_> {
fn set_menu<T: 'static>(&mut self, menu: MenuDesc<T>) {
let cmd = Command::new(druid::commands::SET_MENU, menu);
let target = self.window_id();
self.submit_command(cmd, target);
}
}

fn ui_builder() -> impl Widget<State> {
let text = LocalizedString::new("hello-counter")
.with_arg("count", |data: &State, _env| data.menu_count.into());
Expand Down Expand Up @@ -91,7 +80,8 @@ fn ui_builder() -> impl Widget<State> {
row.add_child(Padding::new(5.0, new_button));
row.add_child(Padding::new(5.0, quit_button));
col.add_flex_child(Align::centered(row), 1.0);
Glow::new(col)
let content = ControllerHost::new(col, ContextMenuController);
Glow::new(content)
}

struct Glow<W> {
Expand Down Expand Up @@ -141,28 +131,23 @@ impl<W: Widget<State>> Widget<State> for Glow<W> {
}
}

struct Delegate;
struct ContextMenuController;

impl AppDelegate<State> for Delegate {
fn event(
&mut self,
ctx: &mut DelegateCtx,
window_id: WindowId,
event: Event,
_data: &mut State,
_env: &Env,
) -> Option<Event> {
impl<T, W: Widget<T>> Controller<T, W> for ContextMenuController {
fn event(&mut self, child: &mut W, ctx: &mut EventCtx, event: &Event, data: &mut T, env: &Env) {
match event {
Event::MouseDown(ref mouse) if mouse.button.is_right() => {
let menu = ContextMenu::new(make_context_menu::<State>(), mouse.pos);
let cmd = Command::new(druid::commands::SHOW_CONTEXT_MENU, menu);
ctx.submit_command(cmd, Target::Window(window_id));
None
ctx.show_context_menu(menu);
}
other => Some(other),
_ => child.event(ctx, event, data, env),
}
}
}

struct Delegate;

impl AppDelegate<State> for Delegate {
fn command(
&mut self,
ctx: &mut DelegateCtx,
Expand All @@ -173,34 +158,30 @@ impl AppDelegate<State> for Delegate {
) -> bool {
match (target, &cmd.selector) {
(_, &sys_cmds::NEW_FILE) => {
let new_win = WindowDesc::new(ui_builder)
let window = WindowDesc::new(ui_builder)
.menu(make_menu(data))
.window_size((data.selected as f64 * 100.0 + 300.0, 500.0));
let command = Command::one_shot(sys_cmds::NEW_WINDOW, new_win);
ctx.submit_command(command, Target::Global);
ctx.new_window(window);
false
}
(Target::Window(id), &MENU_COUNT_ACTION) => {
data.selected = *cmd.get_object().unwrap();
let menu = make_menu::<State>(data);
let cmd = Command::new(druid::commands::SET_MENU, menu);
ctx.submit_command(cmd, id);
ctx.set_menu(menu, id);
false
}
// wouldn't it be nice if a menu (like a button) could just mutate state
// directly if desired?
(Target::Window(id), &MENU_INCREMENT_ACTION) => {
data.menu_count += 1;
let menu = make_menu::<State>(data);
let cmd = Command::new(druid::commands::SET_MENU, menu);
ctx.submit_command(cmd, id);
ctx.set_menu(menu, id);
false
}
(Target::Window(id), &MENU_DECREMENT_ACTION) => {
data.menu_count = data.menu_count.saturating_sub(1);
let menu = make_menu::<State>(data);
let cmd = Command::new(druid::commands::SET_MENU, menu);
ctx.submit_command(cmd, id);
ctx.set_menu(menu, id);
false
}
(_, &MENU_SWITCH_GLOW_ACTION) => {
Expand Down
48 changes: 46 additions & 2 deletions druid/src/app_delegate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,19 @@

//! Customizing application-level behaviour.

use std::collections::VecDeque;
use std::{any::Any, collections::VecDeque};

use crate::{Command, Data, Env, Event, Target, WindowId};
use crate::{
commands, contexts::StateTypes, Command, Data, Env, Event, MenuDesc, Target, WindowDesc,
WindowId,
};

/// A context passed in to [`AppDelegate`] functions.
///
/// [`AppDelegate`]: trait.AppDelegate.html
pub struct DelegateCtx<'a> {
pub(crate) command_queue: &'a mut VecDeque<(Target, Command)>,
pub(crate) state_types: StateTypes,
}

impl<'a> DelegateCtx<'a> {
Expand All @@ -43,6 +47,46 @@ impl<'a> DelegateCtx<'a> {
let target = target.into().unwrap_or(Target::Global);
self.command_queue.push_back((target, command))
}

/// Create a new window.
/// `T` must represent the application state provided to [`AppLauncher::launch`].
luleyleo marked this conversation as resolved.
Show resolved Hide resolved
///
/// [`AppLauncher::launch`]: struct.AppLauncher.html#method.launch
pub fn new_window<T: Any>(&mut self, desc: WindowDesc<T>) {
if self.state_types.check_window_desc::<T>() {
luleyleo marked this conversation as resolved.
Show resolved Hide resolved
self.submit_command(
Command::one_shot(commands::NEW_WINDOW, desc),
Target::Global,
);
} else {
const MSG: &str = "All application windows must represent the same type of state.";
luleyleo marked this conversation as resolved.
Show resolved Hide resolved
if cfg!(debug_assertions) {
panic!(MSG);
} else {
log::error!("DelegateCtx::new_window: {}", MSG)
}
}
}

/// Set the windows menu.
luleyleo marked this conversation as resolved.
Show resolved Hide resolved
/// `T` must represent the application state provided to [`AppLauncher::launch`].
luleyleo marked this conversation as resolved.
Show resolved Hide resolved
///
/// [`AppLauncher::launch`]: struct.AppLauncher.html#method.launch
pub fn set_menu<T: Any>(&mut self, menu: MenuDesc<T>, window: WindowId) {
if self.state_types.check_menu_desc::<T>() {
self.submit_command(
Command::new(commands::SET_MENU, menu),
Target::Window(window),
);
} else {
const MSG: &str = "Menus must represent the application state.";
luleyleo marked this conversation as resolved.
Show resolved Hide resolved
if cfg!(debug_assertions) {
panic!(MSG);
} else {
log::error!("DelegateCtx::set_menu: {}", MSG)
}
}
}
}

/// A type that provides hooks for handling and modifying top-level events.
Expand Down
6 changes: 3 additions & 3 deletions druid/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ pub mod sys {
pub const HIDE_OTHERS: Selector = Selector::new("druid-builtin.menu-hide-others");

/// The selector for a command to create a new window.
pub const NEW_WINDOW: Selector = Selector::new("druid-builtin.new-window");
pub(crate) const NEW_WINDOW: Selector = Selector::new("druid-builtin.new-window");

/// The selector for a command to close a window.
///
Expand All @@ -139,13 +139,13 @@ pub mod sys {
/// object to be displayed.
///
/// [`ContextMenu`]: ../struct.ContextMenu.html
pub const SHOW_CONTEXT_MENU: Selector = Selector::new("druid-builtin.show-context-menu");
pub(crate) const SHOW_CONTEXT_MENU: Selector = Selector::new("druid-builtin.show-context-menu");

/// The selector for a command to set the window's menu. The argument should
/// be a [`MenuDesc`] object.
///
/// [`MenuDesc`]: ../struct.MenuDesc.html
pub const SET_MENU: Selector = Selector::new("druid-builtin.set-menu");
pub(crate) const SET_MENU: Selector = Selector::new("druid-builtin.set-menu");

/// Show the application preferences.
pub const SHOW_PREFERENCES: Selector = Selector::new("druid-builtin.menu-show-preferences");
Expand Down
94 changes: 91 additions & 3 deletions druid/src/contexts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,27 @@
//! The context types that are passed into various widget methods.

use std::ops::{Deref, DerefMut};
use std::time::Duration;
use std::{
luleyleo marked this conversation as resolved.
Show resolved Hide resolved
any::{Any, TypeId},
time::Duration,
};

use crate::core::{BaseState, CommandQueue, FocusChange};
use crate::piet::Piet;
use crate::piet::RenderContext;
use crate::{
Affine, Command, Cursor, Insets, Point, Rect, Size, Target, Text, TimerToken, Vec2, WidgetId,
WindowHandle, WindowId,
commands, Affine, Command, ContextMenu, Cursor, Insets, MenuDesc, Point, Rect, Size, Target,
Text, TimerToken, Vec2, WidgetId, WindowDesc, WindowHandle, WindowId,
};

/// These allow type checking new windows and menus at runtime
#[derive(Clone, Copy)]
pub(crate) struct StateTypes {
window_desc: TypeId,
menu_desc: TypeId,
context_menu: TypeId,
}

/// A mutable context provided to event handling methods of widgets.
///
/// Widgets should call [`request_paint`] whenever an event causes a change
Expand All @@ -43,6 +54,7 @@ pub struct EventCtx<'a> {
pub(crate) focus_widget: Option<WidgetId>,
pub(crate) is_handled: bool,
pub(crate) is_root: bool,
pub(crate) state_types: StateTypes,
}

/// A mutable context provided to the [`lifecycle`] method on widgets.
Expand Down Expand Up @@ -127,6 +139,28 @@ pub struct PaintCtx<'a, 'b: 'a> {
#[derive(Debug, Clone)]
pub struct Region(Rect);

impl StateTypes {
pub fn new<T: Any>() -> Self {
StateTypes {
window_desc: TypeId::of::<WindowDesc<T>>(),
menu_desc: TypeId::of::<MenuDesc<T>>(),
context_menu: TypeId::of::<ContextMenu<T>>(),
}
}

pub fn check_window_desc<T: Any>(&self) -> bool {
self.window_desc == TypeId::of::<WindowDesc<T>>()
}

pub fn check_menu_desc<T: Any>(&self) -> bool {
self.menu_desc == TypeId::of::<MenuDesc<T>>()
}

pub fn check_context_menu<T: Any>(&self) -> bool {
self.context_menu == TypeId::of::<ContextMenu<T>>()
}
}

impl<'a> EventCtx<'a> {
#[deprecated(since = "0.5.0", note = "use request_paint instead")]
pub fn invalidate(&mut self) {
Expand Down Expand Up @@ -242,6 +276,60 @@ impl<'a> EventCtx<'a> {
&self.window
}

/// Create a new window.
/// `T` must represent the application state provided to [`AppLauncher::launch`].
luleyleo marked this conversation as resolved.
Show resolved Hide resolved
///
/// [`AppLauncher::launch`]: struct.AppLauncher.html#method.launch
pub fn new_window<T: Any>(&mut self, desc: WindowDesc<T>) {
if self.state_types.check_window_desc::<T>() {
self.submit_command(
Command::one_shot(commands::NEW_WINDOW, desc),
Target::Global,
);
} else {
const MSG: &str = "All application windows must represent the same type of state.";
luleyleo marked this conversation as resolved.
Show resolved Hide resolved
if cfg!(debug_assertions) {
panic!(MSG);
} else {
log::error!("EventCtx::new_window: {}", MSG)
}
}
}

/// Set the menu of the window containing the event handling widget.
luleyleo marked this conversation as resolved.
Show resolved Hide resolved
/// `T` must represent the application state provided to [`AppLauncher::launch`].
luleyleo marked this conversation as resolved.
Show resolved Hide resolved
///
/// [`AppLauncher::launch`]: struct.AppLauncher.html#method.launch
pub fn set_menu<T: Any>(&mut self, menu: MenuDesc<T>) {
if self.state_types.check_menu_desc::<T>() {
self.submit_command(Command::new(commands::SET_MENU, menu), None);
luleyleo marked this conversation as resolved.
Show resolved Hide resolved
} else {
const MSG: &str = "Menus must represent the application state.";
luleyleo marked this conversation as resolved.
Show resolved Hide resolved
if cfg!(debug_assertions) {
panic!(MSG);
} else {
log::error!("EventCtx::set_menu: {}", MSG)
}
}
}

/// Show the context menu in the window containing the event handling widget.
/// `T` must represent the application state provided to [`AppLauncher::launch`].
///
/// [`AppLauncher::launch`]: struct.AppLauncher.html#method.launch
pub fn show_context_menu<T: Any>(&mut self, menu: ContextMenu<T>) {
if self.state_types.check_context_menu::<T>() {
self.submit_command(Command::new(commands::SHOW_CONTEXT_MENU, menu), None);
} else {
const MSG: &str = "Context menus must represent the application state.";
luleyleo marked this conversation as resolved.
Show resolved Hide resolved
if cfg!(debug_assertions) {
Copy link
Member

Choose a reason for hiding this comment

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

if this becomes a pattern we could consider a custom macro, like log_or_panic!(...).

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 would like it if this becomes a pattern (or log_or_panic!) as panic! makes debugging easier due to the back trace while log::error! does not crash the app once you ship it.

panic!(MSG);
} else {
log::error!("EventCtx::show_context_menu: {}", MSG)
}
}
}

/// Set the event as "handled", which stops its propagation to other
/// widgets.
pub fn set_handled(&mut self) {
Expand Down
1 change: 1 addition & 0 deletions druid/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,7 @@ impl<T: Data, W: Widget<T>> WidgetPod<T, W> {
is_handled: false,
is_root: false,
focus_widget: ctx.focus_widget,
state_types: ctx.state_types,
};

let rect = child_ctx.base_state.layout_rect.unwrap_or_default();
Expand Down
7 changes: 5 additions & 2 deletions druid/src/win_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use crate::{
WindowId,
};

use crate::command::sys as sys_cmd;
use crate::{command::sys as sys_cmd, contexts::StateTypes};

pub(crate) const RUN_COMMANDS_TOKEN: IdleToken = IdleToken::new(1);

Expand Down Expand Up @@ -187,7 +187,10 @@ impl<T: Data> Inner<T> {
ref env,
..
} = self;
let mut ctx = DelegateCtx { command_queue };
let mut ctx = DelegateCtx {
command_queue,
state_types: StateTypes::new::<T>(),
};
if let Some(delegate) = delegate {
Some(f(delegate, data, env, &mut ctx))
} else {
Expand Down
Loading