From 698e26a831ad3a2921618f46e98eae2ca656b855 Mon Sep 17 00:00:00 2001 From: Dustin Carlino Date: Thu, 14 Oct 2021 13:33:51 -0700 Subject: [PATCH] Use Outcome::Focused to match existing semantics better --- widgetry/src/app_state.rs | 2 +- widgetry/src/widgets/containers.rs | 13 +++--------- widgetry/src/widgets/drag_drop.rs | 4 +++- widgetry/src/widgets/dropdown.rs | 2 +- widgetry/src/widgets/mod.rs | 26 +++--------------------- widgetry/src/widgets/panel.rs | 6 +++++- widgetry/src/widgets/persistent_split.rs | 6 ++---- 7 files changed, 18 insertions(+), 41 deletions(-) diff --git a/widgetry/src/app_state.rs b/widgetry/src/app_state.rs index 113bb70632..5cd34521d5 100644 --- a/widgetry/src/app_state.rs +++ b/widgetry/src/app_state.rs @@ -282,7 +282,7 @@ impl State for SimpleStateWrapper { .inner .panel_changed(ctx, app, &mut self.panel) .unwrap_or_else(|| self.inner.other_event(ctx, app)), - Outcome::DragDropReleased(_, _, _) | Outcome::Nothing => { + Outcome::DragDropReleased(_, _, _) | Outcome::Focused(_) | Outcome::Nothing => { self.inner.other_event(ctx, app) } } diff --git a/widgetry/src/widgets/containers.rs b/widgetry/src/widgets/containers.rs index 12386ec3f7..d97fcdbf7f 100644 --- a/widgetry/src/widgets/containers.rs +++ b/widgetry/src/widgets/containers.rs @@ -48,12 +48,7 @@ impl WidgetImpl for Container { fn event(&mut self, ctx: &mut EventCtx, output: &mut WidgetOutput) { for w in &mut self.members { - // If both are filled out, they'll be the same - if let Some(id) = output - .prev_focus_owned_by - .as_ref() - .or(output.current_focus_owned_by.as_ref()) - { + if let Some(id) = output.prev_focus_owned_by.as_ref() { // Container is the only place that needs to actually enforce focus. If a Panel // consists of only one top-level widget, then there's nothing else to conflict // with focus. And in the common case, we have a tree of Containers, with @@ -63,10 +58,8 @@ impl WidgetImpl for Container { } } w.widget.event(ctx, output); - // If the widget produced an outcome or currently has focus, short-circuit. - if !matches!(output.outcome, Outcome::Nothing) - || output.current_focus_owned_by.is_some() - { + // If the widget produced an outcome, short-circuit. + if !matches!(output.outcome, Outcome::Nothing) { return; } } diff --git a/widgetry/src/widgets/drag_drop.rs b/widgetry/src/widgets/drag_drop.rs index 7d075064e2..48108e8baa 100644 --- a/widgetry/src/widgets/drag_drop.rs +++ b/widgetry/src/widgets/drag_drop.rs @@ -347,7 +347,9 @@ impl WidgetImpl for DragDrop { } => ctx.cursor_grabbable(), State::Dragging { .. } => { ctx.cursor_grabbing(); - output.steal_focus(self.label.clone()); + if matches!(output.outcome, Outcome::Nothing) { + output.outcome = Outcome::Focused(self.label.clone()); + } } _ => {} } diff --git a/widgetry/src/widgets/dropdown.rs b/widgetry/src/widgets/dropdown.rs index 41aab62d96..1118e4960e 100644 --- a/widgetry/src/widgets/dropdown.rs +++ b/widgetry/src/widgets/dropdown.rs @@ -126,7 +126,7 @@ impl WidgetImpl for Dropdown { } if self.menu.is_some() { - output.steal_focus(self.label.clone()); + output.outcome = Outcome::Focused(self.label.clone()); } } diff --git a/widgetry/src/widgets/mod.rs b/widgetry/src/widgets/mod.rs index 13393e4431..c6eed16314 100644 --- a/widgetry/src/widgets/mod.rs +++ b/widgetry/src/widgets/mod.rs @@ -80,6 +80,8 @@ pub enum Outcome { /// On a DragDrop widget, a member was clicked on and released. Its position may have changed. /// (name, orig_idx, new_idx) DragDropReleased(String, usize, usize), + /// Some named widget currently holds focus + Focused(String), /// Nothing happened Nothing, } @@ -96,11 +98,8 @@ pub struct WidgetOutput { /// This widget produced an Outcome, and event handling should immediately stop. Most widgets /// shouldn't set this. pub outcome: Outcome, - /// This was the widget who called `steal_focus` last event. Don't set directly. + /// This widget exclusively owned focus as of the last event. Don't modify. pub prev_focus_owned_by: Option, - /// If a widget sets their ID here, don't propagate events to other widgets in the panel. Don't - /// set this directly; use `steal_focus`. - pub current_focus_owned_by: Option, } impl WidgetOutput { @@ -109,27 +108,8 @@ impl WidgetOutput { redo_layout: false, outcome: Outcome::Nothing, prev_focus_owned_by: None, - current_focus_owned_by: None, } } - - /// Indicate that the widget with this ID currently has focus and should prevent other widgets - /// from handling events. - /// - /// TODO Currently this only works for widgets in the same Panel, but this should handle all - /// Panels on-screen. - pub fn steal_focus(&mut self, id: String) { - if let Some(ref existing) = self.prev_focus_owned_by { - if id.as_str() == existing { - // Renew the focus, so prev_focus_owned_by is set for the next event - self.current_focus_owned_by = Some(id); - return; - } - panic!("{} can't steal focus; {} already has it", id, existing); - } - assert!(self.current_focus_owned_by.is_none()); - self.current_focus_owned_by = Some(id); - } } downcast_rs::impl_downcast!(WidgetImpl); diff --git a/widgetry/src/widgets/panel.rs b/widgetry/src/widgets/panel.rs index b4854f0a09..cb81a579f2 100644 --- a/widgetry/src/widgets/panel.rs +++ b/widgetry/src/widgets/panel.rs @@ -32,6 +32,8 @@ pub struct Panel { container_dims: ScreenDims, clip_rect: Option, + // TODO Currently this only works for widgets in the same Panel, but this should handle all + // Panels on-screen. focus_owned_by: Option, } @@ -320,7 +322,9 @@ impl Panel { } // Remember this for the next event - self.focus_owned_by = output.current_focus_owned_by; + if let Outcome::Focused(ref id) = output.outcome { + self.focus_owned_by = Some(id.clone()); + } output.outcome } diff --git a/widgetry/src/widgets/persistent_split.rs b/widgetry/src/widgets/persistent_split.rs index 7f12c8cbdf..117018c80d 100644 --- a/widgetry/src/widgets/persistent_split.rs +++ b/widgetry/src/widgets/persistent_split.rs @@ -109,10 +109,6 @@ impl WidgetImpl for PersistentSplit { let mut tmp_output = WidgetOutput::new(); self.dropdown.event(ctx, &mut tmp_output); - if tmp_output.current_focus_owned_by.is_some() { - // The dropdown's label is a dummy value - output.steal_focus(self.btn.action.clone()); - } let new_value = self.dropdown.current_value(); if new_value != self.current_value { @@ -126,6 +122,8 @@ impl WidgetImpl for PersistentSplit { self.btn = button_builder.build(ctx, &label); output.redo_layout = true; output.outcome = Outcome::Changed(label); + } else if let Outcome::Focused(_) = tmp_output.outcome { + output.outcome = Outcome::Focused(self.btn.action.clone()); } }