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

Make has_focus reliable. #819

Merged
merged 4 commits into from
Apr 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 2 additions & 12 deletions druid/src/contexts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,12 +270,7 @@ impl<'a> EventCtx<'a> {
///
/// [`is_focused`]: struct.EventCtx.html#method.is_focused
pub fn has_focus(&self) -> bool {
// The bloom filter we're checking can return false positives.
let is_child = self
.focus_widget
.map(|id| self.base_state.children.may_contain(&id))
.unwrap_or(false);
is_child || self.focus_widget == Some(self.widget_id())
self.base_state.has_focus
}

/// Request keyboard focus.
Expand Down Expand Up @@ -613,12 +608,7 @@ impl<'a, 'b: 'a> PaintCtx<'a, 'b> {
/// [`is_focused`]: #method.is_focused
/// [`EventCtx::is_focused`]: struct.EventCtx.html#method.is_focused
pub fn has_focus(&self) -> bool {
// The bloom filter we're checking can return false positives.
let is_child = self
.focus_widget
.map(|id| self.base_state.children.may_contain(&id))
.unwrap_or(false);
is_child || self.focus_widget == Some(self.widget_id())
self.base_state.has_focus
}

/// Returns the currently visible [`Region`].
Expand Down
28 changes: 21 additions & 7 deletions druid/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ pub(crate) struct BaseState {
/// Any descendant is active.
has_active: bool,

/// In the focused path, starting from window and ending at the focused widget.
/// Descendants of the focused widget are not in the focused path.
pub(crate) has_focus: bool,

/// Any descendant has requested an animation frame.
pub(crate) request_anim: bool,

Expand Down Expand Up @@ -514,20 +518,28 @@ impl<T: Data, W: Widget<T>> WidgetPod<T, W> {
};

if let Some(change) = this_changed {
let event = LifeCycle::FocusChanged(change);
self.inner.lifecycle(ctx, &event, data, env);
// Only send FocusChanged in case there's actual change
if old != new {
xStrom marked this conversation as resolved.
Show resolved Hide resolved
self.state.has_focus = change;
let event = LifeCycle::FocusChanged(change);
self.inner.lifecycle(ctx, &event, data, env);
}
false
} else {
self.state.has_focus = false;
// Recurse when the target widgets could be our descendants.
// The bloom filter we're checking can return false positives.
old.map(|id| self.state.children.may_contain(&id))
.or_else(|| new.map(|id| self.state.children.may_contain(&id)))
.unwrap_or(false)
match (old, new) {
(Some(old), _) if self.state.children.may_contain(old) => true,
(_, Some(new)) if self.state.children.may_contain(new) => true,
_ => false,
}
}
}
LifeCycle::FocusChanged(_) => {
self.state.request_focus = None;
true
// We are a descendant of a widget that has/had focus.
xStrom marked this conversation as resolved.
Show resolved Hide resolved
// Descendants don't inherit focus, so don't recurse.
false
}
#[cfg(test)]
LifeCycle::DebugRequestState { widget, state_cell } => {
Expand Down Expand Up @@ -624,6 +636,7 @@ impl BaseState {
needs_layout: false,
is_active: false,
has_active: false,
has_focus: false,
request_anim: false,
request_timer: false,
request_focus: None,
Expand All @@ -640,6 +653,7 @@ impl BaseState {
self.request_anim |= child_state.request_anim;
self.request_timer |= child_state.request_timer;
self.has_active |= child_state.has_active;
self.has_focus |= child_state.has_focus;
self.children_changed |= child_state.children_changed;
self.request_focus = self.request_focus.or(child_state.request_focus);
}
Expand Down
43 changes: 29 additions & 14 deletions druid/src/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,22 +328,37 @@ impl<T: Data> Window<T> {
match focus {
FocusChange::Resign => None,
FocusChange::Focus(id) => Some(id),
FocusChange::Next => self
.focus
.and_then(|id| self.focus_chain().iter().position(|i| i == &id))
.map(|idx| {
let next_idx = (idx + 1) % self.focus_chain().len();
self.focus_chain()[next_idx]
}),
FocusChange::Previous => self
.focus
.and_then(|id| self.focus_chain().iter().position(|i| i == &id))
FocusChange::Next => self.widget_from_focus_chain(true),
FocusChange::Previous => self.widget_from_focus_chain(false),
}
}

fn widget_from_focus_chain(&self, forward: bool) -> Option<WidgetId> {
Copy link
Member

Choose a reason for hiding this comment

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

👍

self.focus.and_then(|focus| {
self.focus_chain()
.iter()
// Find where the focused widget is in the focus chain
.position(|id| id == &focus)
.map(|idx| {
// Return the id that's next to it in the focus chain
let len = self.focus_chain().len();
let prev_idx = (idx + len - 1) % len;
self.focus_chain()[prev_idx]
}),
}
let new_idx = if forward {
(idx + 1) % len
} else {
(idx + len - 1) % len
};
self.focus_chain()[new_idx]
})
.or_else(|| {
// If the currently focused widget isn't in the focus chain,
// then we'll just return the first/last entry of the chain, if any.
if forward {
self.focus_chain().first().copied()
} else {
self.focus_chain().last().copied()
}
})
})
}
}

Expand Down