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

Focus-chain update #1724

Merged
merged 8 commits into from
Apr 19, 2021
Merged
Show file tree
Hide file tree
Changes from 7 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ You can find its changes [documented below](#070---2021-01-01).
- GTK Shell: Prevent mangling of newline characters in clipboard ([#1695] by [@ForLoveOfCats])
- Use correct fill rule when rendering SVG paths ([#1606] by [@SecondFlight])
- Correctly capture and use stroke properties when rendering SVG paths ([#1647] by [@SecondFlight])
- focus-chain now only includes non hidden (`should_propagate_to_hidden()` on `Event` and `Lifecylce`) widgets ([#1724] by [@xarvic])

### Visual

Expand Down Expand Up @@ -680,6 +681,7 @@ Last release without a changelog :(
[#1698]: https://github.com/linebender/druid/pull/1698
[#1702]: https://github.com/linebender/druid/pull/1702
[#1713]: https://github.com/linebender/druid/pull/1713
[#1724]: https://github.com/linebender/druid/pull/1724

[Unreleased]: https://github.com/linebender/druid/compare/v0.7.0...master
[0.7.0]: https://github.com/linebender/druid/compare/v0.6.0...v0.7.0
Expand Down
17 changes: 17 additions & 0 deletions druid/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -912,6 +912,8 @@ impl<T: Data, W: Widget<T>> WidgetPod<T, W> {
// we may send an extra event after the actual event
let mut extra_event = None;

let had_focus = self.state.has_focus;

let recurse = match event {
LifeCycle::Internal(internal) => match internal {
InternalLifeCycle::RouteWidgetAdded => {
Expand Down Expand Up @@ -1035,6 +1037,10 @@ impl<T: Data, W: Widget<T>> WidgetPod<T, W> {
}
LifeCycle::BuildFocusChain => {
if self.state.update_focus_chain {
// Replace has_focus to check if the value changed in the meantime
let is_focused = ctx.state.focus_widget == Some(self.state.id);
self.state.has_focus = is_focused;

self.state.focus_chain.clear();
true
} else {
Expand Down Expand Up @@ -1082,6 +1088,17 @@ impl<T: Data, W: Widget<T>> WidgetPod<T, W> {
// Update focus-chain of our parent
LifeCycle::BuildFocusChain => {
self.state.update_focus_chain = false;

// had_focus is the old focus value. state.has_focus was repaced with ctx.is_focused().
// Therefore if had_focus is true but state.has_focus is false then the widget which is
// currently focused is not part of the functional tree anymore
// (Lifecycle::BuildFocusChain.should_propagate_to_hidden() is false!) and should
// resign the focus.
if had_focus && !self.state.has_focus {
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused; we haven't done merge_up yet, so how would has_focus have changed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The merge_up of our children happened already in line 1058 / 1062. If someone hides a focused widget and then calls children_changed(), we clear has_focus in BuildFocusChain if has_clear is not set after the calls in 1058 and 1062 we know, that the focused widget is not reachable

self.state.request_focus = Some(FocusChange::Resign);
}
self.state.has_focus = had_focus;

if !self.state.is_disabled() {
ctx.widget_state.focus_chain.extend(&self.state.focus_chain);
}
Expand Down
9 changes: 5 additions & 4 deletions druid/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,10 +428,11 @@ impl LifeCycle {
pub fn should_propagate_to_hidden(&self) -> bool {
match self {
LifeCycle::Internal(internal) => internal.should_propagate_to_hidden(),
LifeCycle::WidgetAdded | LifeCycle::DisabledChanged(_) | LifeCycle::BuildFocusChain => {
true
}
LifeCycle::Size(_) | LifeCycle::HotChanged(_) | LifeCycle::FocusChanged(_) => false,
LifeCycle::WidgetAdded | LifeCycle::DisabledChanged(_) => true,
LifeCycle::Size(_)
| LifeCycle::HotChanged(_)
| LifeCycle::FocusChanged(_)
| LifeCycle::BuildFocusChain => false,
}
}
}
Expand Down