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

First cut at request_update #1128

Merged
merged 7 commits into from
Aug 20, 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ You can find its changes [documented below](#060---2020-06-01).
- `BoxConstraints::UNBOUNDED` constant. ([#1126] by [@danieldulaney])
- Close requests from the shell can now be intercepted ([#1118] by [@jneem])
- The Lens derive now supports an `ignore` attribute. ([#1133] by [@jneem])
- `request_update` in `EventCtx`. ([#1128] by [@raphlinus])

### Changed

Expand Down Expand Up @@ -389,6 +390,7 @@ Last release without a changelog :(
[#1119]: https://github.com/linebender/druid/pull/1119
[#1120]: https://github.com/linebender/druid/pull/1120
[#1126]: https://github.com/linebender/druid/pull/1120
[#1128]: https://github.com/linebender/druid/pull/1128
[#1133]: https://github.com/linebender/druid/pull/1133

[Unreleased]: https://github.com/linebender/druid/compare/v0.6.0...master
Expand Down
13 changes: 13 additions & 0 deletions druid/src/contexts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,19 @@ impl EventCtx<'_, '_> {
);
}
}

/// Request an update cycle.
///
/// After this, `update` will be called on the widget in the next update cycle, even
/// if there's not a data change.
Copy link
Member

Choose a reason for hiding this comment

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

I think it's worth highlighting that this method is only exposed for special cases, such as when a container widget is synthesizing data for its children, and that in general it should be unnecessary/avoided.

///
/// The use case for this method is when a container widget synthesizes data for its
/// children. This is appropriate in specialized cases, but before reaching for this
/// method, consider whether it might be better to refactor to be more idiomatic, in
/// particular to make that data available in the app state.
pub fn request_update(&mut self) {
self.widget_state.request_update = true;
}
}

impl LifeCycleCtx<'_, '_> {
Expand Down
26 changes: 17 additions & 9 deletions druid/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ pub(crate) struct WidgetState {
/// Any descendant has requested an animation frame.
pub(crate) request_anim: bool,

/// Any descendant has requested update.
pub(crate) request_update: bool,

pub(crate) focus_chain: Vec<WidgetId>,
pub(crate) request_focus: Option<FocusChange>,
pub(crate) children: Bloom<WidgetId>,
Expand Down Expand Up @@ -844,15 +847,17 @@ impl<T: Data, W: Widget<T>> WidgetPod<T, W> {
///
/// [`update`]: trait.Widget.html#tymethod.update
pub fn update(&mut self, ctx: &mut UpdateCtx, data: &T, env: &Env) {
match (self.old_data.as_ref(), self.env.as_ref()) {
(Some(d), Some(e)) if d.same(data) && e.same(env) => return,
(None, _) => {
log::warn!("old_data missing in {:?}, skipping update", self.id());
self.old_data = Some(data.clone());
self.env = Some(env.clone());
return;
if !self.state.request_update {
match (self.old_data.as_ref(), self.env.as_ref()) {
(Some(d), Some(e)) if d.same(data) && e.same(env) => return,
(None, _) => {
log::warn!("old_data missing in {:?}, skipping update", self.id());
self.old_data = Some(data.clone());
self.env = Some(env.clone());
return;
}
_ => (),
}
_ => (),
}

let mut child_ctx = UpdateCtx {
Expand All @@ -865,7 +870,8 @@ impl<T: Data, W: Widget<T>> WidgetPod<T, W> {
self.old_data = Some(data.clone());
self.env = Some(env.clone());

ctx.widget_state.merge_up(&mut self.state)
self.state.request_update = false;
ctx.widget_state.merge_up(&mut self.state);
}
}

Expand Down Expand Up @@ -893,6 +899,7 @@ impl WidgetState {
has_active: false,
has_focus: false,
request_anim: false,
request_update: false,
request_focus: None,
focus_chain: Vec::new(),
children: Bloom::new(),
Expand Down Expand Up @@ -925,6 +932,7 @@ impl WidgetState {
self.has_active |= child_state.has_active;
self.has_focus |= child_state.has_focus;
self.children_changed |= child_state.children_changed;
self.request_update |= child_state.request_update;
self.request_focus = child_state.request_focus.take().or(self.request_focus);
self.timers.extend_drain(&mut child_state.timers);
}
Expand Down
24 changes: 24 additions & 0 deletions druid/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -463,3 +463,27 @@ fn register_after_adding_child() {
assert_eq!(harness.get_state(id_5).children.entry_count(), 5);
})
}

#[test]
/// Test that request_update actually causes the request.
fn request_update() {
const REQUEST_UPDATE: Selector = Selector::new("druid-tests.request_update");
let updated: Rc<Cell<bool>> = Default::default();
let updated_clone = updated.clone();

let widget = ModularWidget::new(())
.event_fn(|_, ctx, event, _data, _env| {
if matches!(event, Event::Command(cmd) if cmd.is(REQUEST_UPDATE)) {
ctx.request_update();
}
})
.update_fn(move |_, _ctx, _old_data, _data, _env| {
updated_clone.set(true);
});
Harness::create_simple((), widget, |harness| {
harness.send_initial_events();
assert!(!updated.get());
harness.submit_command(REQUEST_UPDATE, None);
assert!(updated.get());
})
Copy link
Member

Choose a reason for hiding this comment

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

nice to see how straight forward and readable this is.

}