Skip to content

Commit

Permalink
Use Outcome::Focused to match existing semantics better
Browse files Browse the repository at this point in the history
  • Loading branch information
dabreegster committed Oct 15, 2021
1 parent b024fa9 commit 698e26a
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 41 deletions.
2 changes: 1 addition & 1 deletion widgetry/src/app_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ impl<A: 'static> State<A> for SimpleStateWrapper<A> {
.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)
}
}
Expand Down
13 changes: 3 additions & 10 deletions widgetry/src/widgets/containers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}
}
Expand Down
4 changes: 3 additions & 1 deletion widgetry/src/widgets/drag_drop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,9 @@ impl<T: 'static + Copy + PartialEq> WidgetImpl for DragDrop<T> {
} => 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());
}
}
_ => {}
}
Expand Down
2 changes: 1 addition & 1 deletion widgetry/src/widgets/dropdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ impl<T: 'static + Clone> WidgetImpl for Dropdown<T> {
}

if self.menu.is_some() {
output.steal_focus(self.label.clone());
output.outcome = Outcome::Focused(self.label.clone());
}
}

Expand Down
26 changes: 3 additions & 23 deletions widgetry/src/widgets/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand All @@ -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<String>,
/// 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<String>,
}

impl WidgetOutput {
Expand All @@ -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);
Expand Down
6 changes: 5 additions & 1 deletion widgetry/src/widgets/panel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ pub struct Panel {
container_dims: ScreenDims,
clip_rect: Option<ScreenRectangle>,

// TODO Currently this only works for widgets in the same Panel, but this should handle all
// Panels on-screen.
focus_owned_by: Option<String>,
}

Expand Down Expand Up @@ -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
}
Expand Down
6 changes: 2 additions & 4 deletions widgetry/src/widgets/persistent_split.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,6 @@ impl<T: 'static + Clone + PartialEq> WidgetImpl for PersistentSplit<T> {

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 {
Expand All @@ -126,6 +122,8 @@ impl<T: 'static + Clone + PartialEq> WidgetImpl for PersistentSplit<T> {
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());
}
}

Expand Down

0 comments on commit 698e26a

Please sign in to comment.