From 5b4dba493f8bea94acee0429946469b6b33d817a Mon Sep 17 00:00:00 2001 From: Christoph Date: Mon, 7 Feb 2022 17:01:11 +0100 Subject: [PATCH 1/9] - added scroll_to_view handling to Clip and Tabs. - Fixed unused notification warning. --- druid/src/command.rs | 23 ++++++++++++++-- druid/src/contexts.rs | 19 +++++++++++++- druid/src/core.rs | 2 +- druid/src/widget/clip_box.rs | 51 ++++++++++++++++++++++++++++++++---- druid/src/widget/scroll.rs | 2 +- druid/src/widget/tabs.rs | 18 ++++++++++--- druid/src/window.rs | 2 ++ 7 files changed, 103 insertions(+), 14 deletions(-) diff --git a/druid/src/command.rs b/druid/src/command.rs index 32f8dd1256..b5b78e0c21 100644 --- a/druid/src/command.rs +++ b/druid/src/command.rs @@ -104,6 +104,7 @@ pub struct Notification { payload: Arc, source: WidgetId, route: WidgetId, + known_target: bool, } /// A wrapper type for [`Command`] payloads that should only be used once. @@ -343,11 +344,15 @@ pub mod sys { /// Widgets which hide their children, should always call `ctx.set_handled()` in response to /// avoid unintended behaviour from widgets further down the tree. /// If possible the widget should move its children to bring the area into view and then submit - /// a new notification with the region translated by the amount, the child it contained was - /// translated. + /// a new `SCROLL_TO_VIEW` notification with the same region relative to the new child position. + /// + /// When building a new widget using ClipBox take a look at [`ClipBox::managed`] and + /// [`ClipBox::default_scroll_to_view_handling`]. /// /// [`scroll_to_view`]: crate::EventCtx::scroll_to_view() /// [`scroll_area_to_view`]: crate::EventCtx::scroll_area_to_view() + /// [`ClipBox::managed`]: crate::widget::ClipBox::managed() + /// [`ClipBox::default_scroll_to_view_handling`]: crate::widget::ClipBox::default_scroll_to_view_handling() pub const SCROLL_TO_VIEW: Selector = Selector::new("druid-builtin.scroll-to"); /// A change that has occured to text state, and needs to be @@ -430,6 +435,7 @@ impl Command { payload: self.payload, source, route: source, + known_target: true, } } @@ -552,6 +558,19 @@ impl Notification { self.source } + /// If set to false this notification will not produce a warning when reaching the root widget. + /// + /// The default is true. + pub fn known_target(mut self, known_target: bool) -> Self { + self.known_target = known_target; + self + } + + /// Returns whether this notification was sent to a specific widget. + pub fn has_known_target(&self) -> bool { + self.known_target + } + /// Change the route id pub(crate) fn with_route(mut self, widget_id: WidgetId) -> Self { self.route = widget_id; diff --git a/druid/src/contexts.rs b/druid/src/contexts.rs index 6d71a3223e..0a85ee57f0 100644 --- a/druid/src/contexts.rs +++ b/druid/src/contexts.rs @@ -465,8 +465,11 @@ impl_context_method!(EventCtx<'_, '_>, UpdateCtx<'_, '_>, LifeCycleCtx<'_, '_>, /// /// If the widget is [`hidden`], this method has no effect. /// + /// This functionality is achieved by sending a [`SCROLL_TO_VIEW`] notification. + /// /// [`Scroll`]: crate::widget::Scroll /// [`hidden`]: crate::Event::should_propagate_to_hidden + /// [`SCROLL_TO_VIEW`]: crate::commands::SCROLL_TO_VIEW pub fn scroll_to_view(&mut self) { self.scroll_area_to_view(self.size().to_rect()) } @@ -545,6 +548,20 @@ impl EventCtx<'_, '_> { self.notifications.push_back(note); } + /// Submit a [`Notification`] with unknown target. + /// + /// In contrast to [`submit_notification`], calling this method will result in an + /// "unhandled notification" warning. + /// + /// [`submit_notification`]: crate::EventCtx::submit_notification + //TODO: decide if we should use a known_target flag on submit_notification instead, + // which would be a breaking change. + pub fn submit_notification_unknown_target(&mut self, note: impl Into) { + trace!("submit_notification"); + let note = note.into().into_notification(self.widget_state.id).known_target(false); + self.notifications.push_back(note); + } + /// Set the "active" state of the widget. /// /// See [`EventCtx::is_active`](struct.EventCtx.html#method.is_active). @@ -716,7 +733,7 @@ impl EventCtx<'_, '_> { /// [`hidden`]: crate::Event::should_propagate_to_hidden pub fn scroll_area_to_view(&mut self, area: Rect) { //TODO: only do something if this widget is not hidden - self.submit_notification(SCROLL_TO_VIEW.with(area + self.window_origin().to_vec2())); + self.submit_notification_unknown_target(SCROLL_TO_VIEW.with(area + self.window_origin().to_vec2())); } } diff --git a/druid/src/core.rs b/druid/src/core.rs index 03478b5dcd..d4af1230ba 100644 --- a/druid/src/core.rs +++ b/druid/src/core.rs @@ -849,7 +849,7 @@ impl> WidgetPod { // Submit the SCROLL_TO notification if it was used from a update or lifecycle // call. let rect = cmd.get_unchecked(SCROLL_TO_VIEW); - inner_ctx.submit_notification(SCROLL_TO_VIEW.with(*rect)); + inner_ctx.submit_notification_unknown_target(SCROLL_TO_VIEW.with(*rect)); ctx.is_handled = true; } _ => { diff --git a/druid/src/widget/clip_box.rs b/druid/src/widget/clip_box.rs index 683e83b4af..bcb29922a9 100644 --- a/druid/src/widget/clip_box.rs +++ b/druid/src/widget/clip_box.rs @@ -133,6 +133,9 @@ pub struct ClipBox { constrain_horizontal: bool, constrain_vertical: bool, must_fill: bool, + + //This ClipBox is wrapped by a widget which manages the viewport_offset + managed: bool, } impl ClipBox { @@ -173,6 +176,22 @@ impl ClipBox { self } + /// Builder-style method to set whether this Clipbox's viewport_offset is managed by another + /// widget. + /// + /// This method should only be used when creating your own widget, which uses ClipBox + /// internally. + /// + /// When set the `ClipBox` will forward [`SCROLL_TO_VIEW`] notifications to its parent unchanged. + /// In this case the parent has to handle said notification itself. By default the ClipBox will + /// filter out [`SCROLL_TO_VIEW`] notifications which refer to areas not visible. + /// + /// [`SCROLL_TO_VIEW`]: crate::commands::SCROLL_TO_VIEW + pub fn managed(mut self) -> Self { + self.managed = true; + self + } + /// Returns a reference to the child widget. pub fn child(&self) -> &W { self.child.widget() @@ -244,6 +263,7 @@ impl> ClipBox { constrain_horizontal: false, constrain_vertical: false, must_fill: false, + managed: false, } } @@ -341,17 +361,38 @@ impl> ClipBox { }); viewport_changed } + + fn fixed_scroll_to_view_handling(&self, ctx: &mut EventCtx, global_highlight_rect: Rect) { + let global_viewport_rect = self.viewport_size().to_rect() + ctx.window_origin().to_vec2(); + let clipped_highlight_rect = global_highlight_rect.intersect(global_viewport_rect); + + if clipped_highlight_rect.size() > 0.0 { + ctx.submit_notification(SCROLL_TO_VIEW.with(clipped_highlight_rect)); + } + } } impl> Widget for ClipBox { #[instrument(name = "ClipBox", level = "trace", skip(self, ctx, event, data, env))] fn event(&mut self, ctx: &mut EventCtx, event: &Event, data: &mut T, env: &Env) { - let viewport = ctx.size().to_rect(); - let force_event = self.child.is_hot() || self.child.has_active(); - if let Some(child_event) = + if let Event::Notification(notification) = event { + if let Some(global_highlight_rect) = notification.get(SCROLL_TO_VIEW) { + if !self.managed { + // If the parent widget does not handle SCROLL_TO_VIEW notifications, we + // prevent unexpected behaviour, by clipping SCROLL_TO_VIEW notifications + // to this ClipBox's viewport. + ctx.set_handled(); + self.fixed_scroll_to_view_handling(ctx, *global_highlight_rect); + } + } + } else { + let viewport = ctx.size().to_rect(); + let force_event = self.child.is_hot() || self.child.has_active(); + if let Some(child_event) = event.transform_scroll(self.viewport_origin().to_vec2(), viewport, force_event) - { - self.child.event(ctx, &child_event, data, env); + { + self.child.event(ctx, &child_event, data, env); + } } } diff --git a/druid/src/widget/scroll.rs b/druid/src/widget/scroll.rs index c0a9c0165e..ab5e33b8a4 100644 --- a/druid/src/widget/scroll.rs +++ b/druid/src/widget/scroll.rs @@ -46,7 +46,7 @@ impl> Scroll { /// [horizontal](#method.horizontal) methods to limit scrolling to a specific axis. pub fn new(child: W) -> Scroll { Scroll { - clip: ClipBox::new(child), + clip: ClipBox::new(child).managed(), scroll_component: ScrollComponent::new(), } } diff --git a/druid/src/widget/tabs.rs b/druid/src/widget/tabs.rs index d4753d727e..2fa5c0e2e5 100644 --- a/druid/src/widget/tabs.rs +++ b/druid/src/widget/tabs.rs @@ -26,6 +26,7 @@ use crate::kurbo::{Circle, Line}; use crate::widget::prelude::*; use crate::widget::{Axis, Flex, Label, LabelText, LensScopeTransfer, Painter, Scope, ScopePolicy}; use crate::{theme, Affine, Data, Insets, Lens, Point, SingleUse, WidgetExt, WidgetPod}; +use crate::commands::SCROLL_TO_VIEW; type TabsScope = Scope, Box>>>; type TabBodyPod = WidgetPod<::Input, ::BodyWidget>; @@ -579,12 +580,21 @@ impl TabsBody { impl Widget> for TabsBody { #[instrument(name = "TabsBody", level = "trace", skip(self, ctx, event, data, env))] fn event(&mut self, ctx: &mut EventCtx, event: &Event, data: &mut TabsState, env: &Env) { - if event.should_propagate_to_hidden() { - for child in self.child_pods() { + if let Event::Notification(notification) = event { + if notification.is(SCROLL_TO_VIEW) && + Some(notification.route()) != self.active_child(data).map(|w|w.id()) + { + // Ignore SCROLL_TO_VIEW requests from every widget except the active. + ctx.set_handled(); + } + } else { + if event.should_propagate_to_hidden() { + for child in self.child_pods() { + child.event(ctx, event, &mut data.inner, env); + } + } else if let Some(child) = self.active_child(data) { child.event(ctx, event, &mut data.inner, env); } - } else if let Some(child) = self.active_child(data) { - child.event(ctx, event, &mut data.inner, env); } if let (Some(t_state), Event::AnimFrame(interval)) = (&mut self.transition_state, event) { diff --git a/druid/src/window.rs b/druid/src/window.rs index 7dd2d42fe2..388aaf0f89 100644 --- a/druid/src/window.rs +++ b/druid/src/window.rs @@ -290,11 +290,13 @@ impl Window { self.root.event(&mut ctx, &event, data, env); } + ctx.notifications.retain(|n|n.has_known_target()); if !ctx.notifications.is_empty() { info!("{} unhandled notifications:", ctx.notifications.len()); for (i, n) in ctx.notifications.iter().enumerate() { info!("{}: {:?}", i, n); } + info!("if this was intended use EventCtx::submit_notification_unknown_target instead"); } Handled::from(ctx.is_handled) }; From 92d2676f33333fb39db31e8d857e60a83b27737d Mon Sep 17 00:00:00 2001 From: Christoph Date: Mon, 7 Feb 2022 18:39:37 +0100 Subject: [PATCH 2/9] update CHANGELOG.md --- CHANGELOG.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a21d82c89..433c93feca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,6 +66,9 @@ You can find its changes [documented below](#070---2021-01-01). - `RangeSlider` and `Annotated` ([#1979] by [@xarvic]) - Add `Checkbox::from_label` constructor ([#2111] by [@maurerdietmar]) - fix content_insets for gtk backend ([#2117] by [@maurerdietmar]) +- `ClipBox::managed`, `Notification::known_target` and `Notification::has_known_target` ([#2141] by [@xarvic]) +- `ClipBox` and `Tabs` handle SCROLL_TO_VIEW ([#2141] by [@xarvic]) +- `EventCtx::submit_notification_unknown_target` ([#2141] by [@xarvic]) ### Changed @@ -91,6 +94,7 @@ You can find its changes [documented below](#070---2021-01-01). - Closures passed to `Label::new` can now return any type that implements `Into` ([#2064] by [@jplatte]) - `AppDelegate::window_added` now receives the new window's `WindowHandle`. ([#2119] by [@zedseven]) - Removed line of code that prevented window miximalization. ([#2113] by [@Pavel-N]) +- Dont warn about unhandled `Notification`s which have `known_target` set to false ([#2141] by [@xarvic]) ### Deprecated @@ -826,8 +830,9 @@ Last release without a changelog :( [#2119]: https://github.com/linebender/druid/pull/2119 [#2111]: https://github.com/linebender/druid/pull/2111 [#2117]: https://github.com/linebender/druid/pull/2117 +[#2117]: https://github.com/linebender/druid/pull/2141 -[Unreleased]: https://github.com/linebender/druid/compare/v0.7.0...master +[Unreleased]: https://github.com/linebender/druid/compare/v0.7.0...masteru [0.7.0]: https://github.com/linebender/druid/compare/v0.6.0...v0.7.0 [0.6.0]: https://github.com/linebender/druid/compare/v0.5.0...v0.6.0 [0.5.0]: https://github.com/linebender/druid/compare/v0.4.0...v0.5.0 From d954310067b27587da881e915f7d8d2c8eb3ea9f Mon Sep 17 00:00:00 2001 From: Christoph Date: Mon, 7 Feb 2022 18:41:37 +0100 Subject: [PATCH 3/9] fix clippy warnings --- CHANGELOG.md | 2 +- druid/src/contexts.rs | 9 +++++++-- druid/src/widget/clip_box.rs | 4 ++-- druid/src/widget/tabs.rs | 16 +++++++--------- druid/src/window.rs | 6 ++++-- 5 files changed, 21 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 433c93feca..0c20de287b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -832,7 +832,7 @@ Last release without a changelog :( [#2117]: https://github.com/linebender/druid/pull/2117 [#2117]: https://github.com/linebender/druid/pull/2141 -[Unreleased]: https://github.com/linebender/druid/compare/v0.7.0...masteru +[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 [0.6.0]: https://github.com/linebender/druid/compare/v0.5.0...v0.6.0 [0.5.0]: https://github.com/linebender/druid/compare/v0.4.0...v0.5.0 diff --git a/druid/src/contexts.rs b/druid/src/contexts.rs index 0a85ee57f0..e7ea237b3a 100644 --- a/druid/src/contexts.rs +++ b/druid/src/contexts.rs @@ -558,7 +558,10 @@ impl EventCtx<'_, '_> { // which would be a breaking change. pub fn submit_notification_unknown_target(&mut self, note: impl Into) { trace!("submit_notification"); - let note = note.into().into_notification(self.widget_state.id).known_target(false); + let note = note + .into() + .into_notification(self.widget_state.id) + .known_target(false); self.notifications.push_back(note); } @@ -733,7 +736,9 @@ impl EventCtx<'_, '_> { /// [`hidden`]: crate::Event::should_propagate_to_hidden pub fn scroll_area_to_view(&mut self, area: Rect) { //TODO: only do something if this widget is not hidden - self.submit_notification_unknown_target(SCROLL_TO_VIEW.with(area + self.window_origin().to_vec2())); + self.submit_notification_unknown_target( + SCROLL_TO_VIEW.with(area + self.window_origin().to_vec2()), + ); } } diff --git a/druid/src/widget/clip_box.rs b/druid/src/widget/clip_box.rs index bcb29922a9..c2028946cd 100644 --- a/druid/src/widget/clip_box.rs +++ b/druid/src/widget/clip_box.rs @@ -366,7 +366,7 @@ impl> ClipBox { let global_viewport_rect = self.viewport_size().to_rect() + ctx.window_origin().to_vec2(); let clipped_highlight_rect = global_highlight_rect.intersect(global_viewport_rect); - if clipped_highlight_rect.size() > 0.0 { + if clipped_highlight_rect.area() > 0.0 { ctx.submit_notification(SCROLL_TO_VIEW.with(clipped_highlight_rect)); } } @@ -389,7 +389,7 @@ impl> Widget for ClipBox { let viewport = ctx.size().to_rect(); let force_event = self.child.is_hot() || self.child.has_active(); if let Some(child_event) = - event.transform_scroll(self.viewport_origin().to_vec2(), viewport, force_event) + event.transform_scroll(self.viewport_origin().to_vec2(), viewport, force_event) { self.child.event(ctx, &child_event, data, env); } diff --git a/druid/src/widget/tabs.rs b/druid/src/widget/tabs.rs index 2fa5c0e2e5..adde669cb7 100644 --- a/druid/src/widget/tabs.rs +++ b/druid/src/widget/tabs.rs @@ -22,11 +22,11 @@ use std::marker::PhantomData; use std::rc::Rc; use tracing::{instrument, trace}; +use crate::commands::SCROLL_TO_VIEW; use crate::kurbo::{Circle, Line}; use crate::widget::prelude::*; use crate::widget::{Axis, Flex, Label, LabelText, LensScopeTransfer, Painter, Scope, ScopePolicy}; use crate::{theme, Affine, Data, Insets, Lens, Point, SingleUse, WidgetExt, WidgetPod}; -use crate::commands::SCROLL_TO_VIEW; type TabsScope = Scope, Box>>>; type TabBodyPod = WidgetPod<::Input, ::BodyWidget>; @@ -581,20 +581,18 @@ impl Widget> for TabsBody { #[instrument(name = "TabsBody", level = "trace", skip(self, ctx, event, data, env))] fn event(&mut self, ctx: &mut EventCtx, event: &Event, data: &mut TabsState, env: &Env) { if let Event::Notification(notification) = event { - if notification.is(SCROLL_TO_VIEW) && - Some(notification.route()) != self.active_child(data).map(|w|w.id()) + if notification.is(SCROLL_TO_VIEW) + && Some(notification.route()) != self.active_child(data).map(|w| w.id()) { // Ignore SCROLL_TO_VIEW requests from every widget except the active. ctx.set_handled(); } - } else { - if event.should_propagate_to_hidden() { - for child in self.child_pods() { - child.event(ctx, event, &mut data.inner, env); - } - } else if let Some(child) = self.active_child(data) { + } else if event.should_propagate_to_hidden() { + for child in self.child_pods() { child.event(ctx, event, &mut data.inner, env); } + } else if let Some(child) = self.active_child(data) { + child.event(ctx, event, &mut data.inner, env); } if let (Some(t_state), Event::AnimFrame(interval)) = (&mut self.transition_state, event) { diff --git a/druid/src/window.rs b/druid/src/window.rs index 388aaf0f89..eb9256353a 100644 --- a/druid/src/window.rs +++ b/druid/src/window.rs @@ -290,13 +290,15 @@ impl Window { self.root.event(&mut ctx, &event, data, env); } - ctx.notifications.retain(|n|n.has_known_target()); + ctx.notifications.retain(|n| n.has_known_target()); if !ctx.notifications.is_empty() { info!("{} unhandled notifications:", ctx.notifications.len()); for (i, n) in ctx.notifications.iter().enumerate() { info!("{}: {:?}", i, n); } - info!("if this was intended use EventCtx::submit_notification_unknown_target instead"); + info!( + "if this was intended use EventCtx::submit_notification_unknown_target instead" + ); } Handled::from(ctx.is_handled) }; From 8320fbaaf606f1c035ba6ad1ae19b4ecb02e1026 Mon Sep 17 00:00:00 2001 From: Christoph Date: Thu, 24 Feb 2022 18:17:27 +0100 Subject: [PATCH 4/9] quick_safe --- druid/src/widget/clip_box.rs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/druid/src/widget/clip_box.rs b/druid/src/widget/clip_box.rs index c2028946cd..1f1e30e1e3 100644 --- a/druid/src/widget/clip_box.rs +++ b/druid/src/widget/clip_box.rs @@ -123,6 +123,11 @@ impl Viewport { let new_origin = self.view_origin + Vec2::new(delta_x, delta_y); self.pan_to(new_origin) } + + pub fn default_scroll_to_view_handling(&mut self, ctx: &mut EventCtx, global_highlight_rect: Rect) { + let global_viewport_rect = self.viewport_size().to_rect() + ctx.window_origin().to_vec2(); + + } } /// A widget exposing a rectangular view into its child, which can be used as a building block for @@ -345,19 +350,7 @@ impl> ClipBox { ) -> bool { let mut viewport_changed = false; self.with_port(|port| { - let global_content_offset = ctx.window_origin().to_vec2() - port.view_origin.to_vec2(); - let content_highlight_rect = global_highlight_rect - global_content_offset; - - if port.pan_to_visible(content_highlight_rect) { - ctx.request_paint(); - viewport_changed = true; - } - // This is a new value since view_origin has changed in the meantime - let global_content_offset = ctx.window_origin().to_vec2() - port.view_origin.to_vec2(); - ctx.submit_notification( - SCROLL_TO_VIEW.with(content_highlight_rect + global_content_offset), - ); }); viewport_changed } From 9517dcd88608b110a320b189713cb022cfa175b2 Mon Sep 17 00:00:00 2001 From: Christoph Date: Sun, 27 Feb 2022 11:09:15 +0100 Subject: [PATCH 5/9] - changed function names - moved default_scroll_to_view_handling to Viewport --- druid/src/command.rs | 16 ++++----- druid/src/contexts.rs | 8 ++--- druid/src/widget/clip_box.rs | 66 +++++++++++++++++++++++------------- druid/src/widget/scroll.rs | 29 ++++++++-------- druid/src/window.rs | 2 +- 5 files changed, 71 insertions(+), 50 deletions(-) diff --git a/druid/src/command.rs b/druid/src/command.rs index b5b78e0c21..254c193060 100644 --- a/druid/src/command.rs +++ b/druid/src/command.rs @@ -104,7 +104,7 @@ pub struct Notification { payload: Arc, source: WidgetId, route: WidgetId, - known_target: bool, + warn_if_unused: bool, } /// A wrapper type for [`Command`] payloads that should only be used once. @@ -435,7 +435,7 @@ impl Command { payload: self.payload, source, route: source, - known_target: true, + warn_if_unused: true, } } @@ -558,17 +558,17 @@ impl Notification { self.source } - /// If set to false this notification will not produce a warning when reaching the root widget. + /// Builder-style method to set warn_if_unused. /// /// The default is true. - pub fn known_target(mut self, known_target: bool) -> Self { - self.known_target = known_target; + pub fn warn_if_unused(mut self, warn_if_unused: bool) -> Self { + self.warn_if_unused = warn_if_unused; self } - /// Returns whether this notification was sent to a specific widget. - pub fn has_known_target(&self) -> bool { - self.known_target + /// Returns whether there should be a warning when no widget handles this notification. + pub fn warn_if_unused_set(&self) -> bool { + self.warn_if_unused } /// Change the route id diff --git a/druid/src/contexts.rs b/druid/src/contexts.rs index e7ea237b3a..a15081e1df 100644 --- a/druid/src/contexts.rs +++ b/druid/src/contexts.rs @@ -548,20 +548,20 @@ impl EventCtx<'_, '_> { self.notifications.push_back(note); } - /// Submit a [`Notification`] with unknown target. + /// Submit a [`Notification`] without warning. /// - /// In contrast to [`submit_notification`], calling this method will result in an + /// In contrast to [`submit_notification`], calling this method will not result in an /// "unhandled notification" warning. /// /// [`submit_notification`]: crate::EventCtx::submit_notification //TODO: decide if we should use a known_target flag on submit_notification instead, // which would be a breaking change. - pub fn submit_notification_unknown_target(&mut self, note: impl Into) { + pub fn submit_notification_without_warning(&mut self, note: impl Into) { trace!("submit_notification"); let note = note .into() .into_notification(self.widget_state.id) - .known_target(false); + .warn_if_unused(false); self.notifications.push_back(note); } diff --git a/druid/src/widget/clip_box.rs b/druid/src/widget/clip_box.rs index 1f1e30e1e3..4530ff4925 100644 --- a/druid/src/widget/clip_box.rs +++ b/druid/src/widget/clip_box.rs @@ -18,7 +18,7 @@ use crate::kurbo::{Affine, Point, Rect, Size, Vec2}; use crate::widget::prelude::*; use crate::widget::Axis; use crate::{Data, WidgetPod}; -use tracing::{instrument, trace}; +use tracing::{instrument, trace, info}; /// Represents the size and position of a rectangular "viewport" into a larger area. #[derive(Clone, Copy, Default, Debug, PartialEq)] @@ -124,9 +124,20 @@ impl Viewport { self.pan_to(new_origin) } - pub fn default_scroll_to_view_handling(&mut self, ctx: &mut EventCtx, global_highlight_rect: Rect) { - let global_viewport_rect = self.viewport_size().to_rect() + ctx.window_origin().to_vec2(); - + pub fn default_scroll_to_view_handling(&mut self, ctx: &mut EventCtx, global_highlight_rect: Rect) -> bool { + let mut viewport_changed = false; + let global_content_offset = ctx.window_origin().to_vec2() - port.view_origin.to_vec2(); + let content_highlight_rect = global_highlight_rect - global_content_offset; + if self.pan_to_visible(content_highlight_rect) { + ctx.request_paint(); + viewport_changed = true; + } + // This is a new value since view_origin has changed in the meantime + let global_content_offset = ctx.window_origin().to_vec2() - port.view_origin.to_vec2(); + ctx.submit_notification( + SCROLL_TO_VIEW.with(content_highlight_rect + global_content_offset), + ); + viewport_changed } } @@ -181,22 +192,6 @@ impl ClipBox { self } - /// Builder-style method to set whether this Clipbox's viewport_offset is managed by another - /// widget. - /// - /// This method should only be used when creating your own widget, which uses ClipBox - /// internally. - /// - /// When set the `ClipBox` will forward [`SCROLL_TO_VIEW`] notifications to its parent unchanged. - /// In this case the parent has to handle said notification itself. By default the ClipBox will - /// filter out [`SCROLL_TO_VIEW`] notifications which refer to areas not visible. - /// - /// [`SCROLL_TO_VIEW`]: crate::commands::SCROLL_TO_VIEW - pub fn managed(mut self) -> Self { - self.managed = true; - self - } - /// Returns a reference to the child widget. pub fn child(&self) -> &W { self.child.widget() @@ -261,7 +256,30 @@ impl ClipBox { impl> ClipBox { /// Creates a new `ClipBox` wrapping `child`. - pub fn new(child: W) -> Self { + /// + /// This method should only be used when creating your own widget, which uses ClipBox + /// internally. + /// + /// `ClipBox` will forward [`SCROLL_TO_VIEW`] notifications to its parent unchanged. + /// In this case the parent has to handle said notification itself. By default the ClipBox will + /// filter out [`SCROLL_TO_VIEW`] notifications which refer to areas not visible. + /// + /// [`SCROLL_TO_VIEW`]: crate::commands::SCROLL_TO_VIEW + pub fn managed(child: W) -> Self { + ClipBox { + child: WidgetPod::new(child), + port: Default::default(), + constrain_horizontal: false, + constrain_vertical: false, + must_fill: false, + managed: true, + } + } + + /// Creates a new unmanaged `ClipBox` wrapping `child`. + /// + /// This method should be used when you are using ClipBox in the widget-hierachie directly. + pub fn unmanaged(child: W) -> Self { ClipBox { child: WidgetPod::new(child), port: Default::default(), @@ -355,12 +373,14 @@ impl> ClipBox { viewport_changed } - fn fixed_scroll_to_view_handling(&self, ctx: &mut EventCtx, global_highlight_rect: Rect) { + fn fixed_scroll_to_view_handling(&self, ctx: &mut EventCtx, global_highlight_rect: Rect, source: WidgetId) { let global_viewport_rect = self.viewport_size().to_rect() + ctx.window_origin().to_vec2(); let clipped_highlight_rect = global_highlight_rect.intersect(global_viewport_rect); if clipped_highlight_rect.area() > 0.0 { ctx.submit_notification(SCROLL_TO_VIEW.with(clipped_highlight_rect)); + } else { + info!("Hidden Widget({}) in unmanaged clip used SCROLL_TO_VIEW. The request is ignored!", source.to_raw()); } } } @@ -375,7 +395,7 @@ impl> Widget for ClipBox { // prevent unexpected behaviour, by clipping SCROLL_TO_VIEW notifications // to this ClipBox's viewport. ctx.set_handled(); - self.fixed_scroll_to_view_handling(ctx, *global_highlight_rect); + self.fixed_scroll_to_view_handling(ctx, *global_highlight_rect, notification.source()); } } } else { diff --git a/druid/src/widget/scroll.rs b/druid/src/widget/scroll.rs index ab5e33b8a4..0120f39de1 100644 --- a/druid/src/widget/scroll.rs +++ b/druid/src/widget/scroll.rs @@ -46,7 +46,7 @@ impl> Scroll { /// [horizontal](#method.horizontal) methods to limit scrolling to a specific axis. pub fn new(child: W) -> Scroll { Scroll { - clip: ClipBox::new(child).managed(), + clip: ClipBox::managed(child), scroll_component: ScrollComponent::new(), } } @@ -189,23 +189,24 @@ impl> Widget for Scroll { // scrolling. self.clip.with_port(|port| { scroll_component.handle_scroll(port, ctx, event, env); - }); - if !self.scroll_component.are_bars_held() { - // We only scroll to the component if the user is not trying to move the scrollbar. - if let Event::Notification(notification) = event { - if let Some(&global_highlight_rect) = notification.get(SCROLL_TO_VIEW) { - ctx.set_handled(); - let view_port_changed = self - .clip - .default_scroll_to_view_handling(ctx, global_highlight_rect); - if view_port_changed { - self.scroll_component - .reset_scrollbar_fade(|duration| ctx.request_timer(duration), env); + if !scroll_component.are_bars_held() { + // We only scroll to the component if the user is not trying to move the scrollbar. + if let Event::Notification(notification) = event { + if let Some(&global_highlight_rect) = notification.get(SCROLL_TO_VIEW) { + ctx.set_handled(); + let view_port_changed = port + .default_scroll_to_view_handling(ctx, global_highlight_rect); + if view_port_changed { + scroll_component + .reset_scrollbar_fade(|duration| ctx.request_timer(duration), env); + } } } } - } + }); + + } #[instrument(name = "Scroll", level = "trace", skip(self, ctx, event, data, env))] diff --git a/druid/src/window.rs b/druid/src/window.rs index eb9256353a..f97a16bcee 100644 --- a/druid/src/window.rs +++ b/druid/src/window.rs @@ -290,7 +290,7 @@ impl Window { self.root.event(&mut ctx, &event, data, env); } - ctx.notifications.retain(|n| n.has_known_target()); + ctx.notifications.retain(|n| n.warn_if_unused_set()); if !ctx.notifications.is_empty() { info!("{} unhandled notifications:", ctx.notifications.len()); for (i, n) in ctx.notifications.iter().enumerate() { From 966667e0379ad9f5c284379c96c274c5eb5931e9 Mon Sep 17 00:00:00 2001 From: Christoph Date: Sun, 27 Feb 2022 15:03:21 +0100 Subject: [PATCH 6/9] - fixed errors - added docs --- druid/src/contexts.rs | 2 +- druid/src/core.rs | 2 +- druid/src/widget/clip_box.rs | 77 +++++++++++++++++++----------------- 3 files changed, 42 insertions(+), 39 deletions(-) diff --git a/druid/src/contexts.rs b/druid/src/contexts.rs index a15081e1df..d5f39fb4ba 100644 --- a/druid/src/contexts.rs +++ b/druid/src/contexts.rs @@ -736,7 +736,7 @@ impl EventCtx<'_, '_> { /// [`hidden`]: crate::Event::should_propagate_to_hidden pub fn scroll_area_to_view(&mut self, area: Rect) { //TODO: only do something if this widget is not hidden - self.submit_notification_unknown_target( + self.submit_notification_without_warning( SCROLL_TO_VIEW.with(area + self.window_origin().to_vec2()), ); } diff --git a/druid/src/core.rs b/druid/src/core.rs index d4af1230ba..e8ade1e9a3 100644 --- a/druid/src/core.rs +++ b/druid/src/core.rs @@ -849,7 +849,7 @@ impl> WidgetPod { // Submit the SCROLL_TO notification if it was used from a update or lifecycle // call. let rect = cmd.get_unchecked(SCROLL_TO_VIEW); - inner_ctx.submit_notification_unknown_target(SCROLL_TO_VIEW.with(*rect)); + inner_ctx.submit_notification_without_warning(SCROLL_TO_VIEW.with(*rect)); ctx.is_handled = true; } _ => { diff --git a/druid/src/widget/clip_box.rs b/druid/src/widget/clip_box.rs index 4530ff4925..dd85941c81 100644 --- a/druid/src/widget/clip_box.rs +++ b/druid/src/widget/clip_box.rs @@ -18,7 +18,7 @@ use crate::kurbo::{Affine, Point, Rect, Size, Vec2}; use crate::widget::prelude::*; use crate::widget::Axis; use crate::{Data, WidgetPod}; -use tracing::{instrument, trace, info}; +use tracing::{instrument, trace, info, warn}; /// Represents the size and position of a rectangular "viewport" into a larger area. #[derive(Clone, Copy, Default, Debug, PartialEq)] @@ -124,21 +124,53 @@ impl Viewport { self.pan_to(new_origin) } + /// The default handling of the [`SCROLL_TO_VIEW`] notification for a scrolling container. + /// + /// The [`SCROLL_TO_VIEW`] notification is send when [`scroll_to_view`] or [`scroll_area_to_view`] + /// are called. + /// + /// [`SCROLL_TO_VIEW`]: crate::commands::SCROLL_TO_VIEW + /// [`scroll_to_view`]: crate::EventCtx::scroll_to_view() + /// [`scroll_area_to_view`]: crate::EventCtx::scroll_area_to_view() pub fn default_scroll_to_view_handling(&mut self, ctx: &mut EventCtx, global_highlight_rect: Rect) -> bool { let mut viewport_changed = false; - let global_content_offset = ctx.window_origin().to_vec2() - port.view_origin.to_vec2(); - let content_highlight_rect = global_highlight_rect - global_content_offset; + let global_content_offset = ctx.window_origin().to_vec2() - self.view_origin.to_vec2(); + let mut content_highlight_rect = global_highlight_rect - global_content_offset; + + if self.content_size.to_rect().intersect(content_highlight_rect) != content_highlight_rect { + warn!("tried to bring area outside of the content to view!"); + } + if self.pan_to_visible(content_highlight_rect) { ctx.request_paint(); viewport_changed = true; } // This is a new value since view_origin has changed in the meantime - let global_content_offset = ctx.window_origin().to_vec2() - port.view_origin.to_vec2(); - ctx.submit_notification( + let global_content_offset = ctx.window_origin().to_vec2() - self.view_origin.to_vec2(); + ctx.submit_notification_without_warning( SCROLL_TO_VIEW.with(content_highlight_rect + global_content_offset), ); viewport_changed } + + /// This method handles SCROLL_TO_VIEW by clipping the view_rect to the content rect. + /// + /// The [`SCROLL_TO_VIEW`] notification is send when [`scroll_to_view`] or [`scroll_area_to_view`] + /// are called. + /// + /// [`SCROLL_TO_VIEW`]: crate::commands::SCROLL_TO_VIEW + /// [`scroll_to_view`]: crate::EventCtx::scroll_to_view() + /// [`scroll_area_to_view`]: crate::EventCtx::scroll_area_to_view() + pub fn fixed_scroll_to_view_handling(&self, ctx: &mut EventCtx, global_highlight_rect: Rect, source: WidgetId) { + let global_viewport_rect = self.view_rect() + ctx.window_origin().to_vec2(); + let clipped_highlight_rect = global_highlight_rect.intersect(global_viewport_rect); + + if clipped_highlight_rect.area() > 0.0 { + ctx.submit_notification_without_warning(SCROLL_TO_VIEW.with(clipped_highlight_rect)); + } else { + info!("Hidden Widget({}) in unmanaged clip requested SCROLL_TO_VIEW. The request is ignored.", source.to_raw()); + } + } } /// A widget exposing a rectangular view into its child, which can be used as a building block for @@ -352,37 +384,6 @@ impl> ClipBox { self.child .set_viewport_offset(self.viewport_origin().to_vec2()); } - - /// The default handling of the [`SCROLL_TO_VIEW`] notification for a scrolling container. - /// - /// The [`SCROLL_TO_VIEW`] notification is send when [`scroll_to_view`] or [`scroll_area_to_view`] - /// are called. - /// - /// [`SCROLL_TO_VIEW`]: crate::commands::SCROLL_TO_VIEW - /// [`scroll_to_view`]: crate::EventCtx::scroll_to_view() - /// [`scroll_area_to_view`]: crate::EventCtx::scroll_area_to_view() - pub fn default_scroll_to_view_handling( - &mut self, - ctx: &mut EventCtx, - global_highlight_rect: Rect, - ) -> bool { - let mut viewport_changed = false; - self.with_port(|port| { - - }); - viewport_changed - } - - fn fixed_scroll_to_view_handling(&self, ctx: &mut EventCtx, global_highlight_rect: Rect, source: WidgetId) { - let global_viewport_rect = self.viewport_size().to_rect() + ctx.window_origin().to_vec2(); - let clipped_highlight_rect = global_highlight_rect.intersect(global_viewport_rect); - - if clipped_highlight_rect.area() > 0.0 { - ctx.submit_notification(SCROLL_TO_VIEW.with(clipped_highlight_rect)); - } else { - info!("Hidden Widget({}) in unmanaged clip used SCROLL_TO_VIEW. The request is ignored!", source.to_raw()); - } - } } impl> Widget for ClipBox { @@ -395,7 +396,9 @@ impl> Widget for ClipBox { // prevent unexpected behaviour, by clipping SCROLL_TO_VIEW notifications // to this ClipBox's viewport. ctx.set_handled(); - self.fixed_scroll_to_view_handling(ctx, *global_highlight_rect, notification.source()); + self.with_port(|port| { + port.fixed_scroll_to_view_handling(ctx, *global_highlight_rect, notification.source()); + }); } } } else { From 14c90b75b74630c5462319b5fde1f62a0e3ac7e2 Mon Sep 17 00:00:00 2001 From: Christoph Date: Sun, 27 Feb 2022 15:05:11 +0100 Subject: [PATCH 7/9] reformat fixed warnings --- druid/src/widget/clip_box.rs | 32 +++++++++++++++++++++++++------- druid/src/widget/scroll.rs | 6 ++---- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/druid/src/widget/clip_box.rs b/druid/src/widget/clip_box.rs index dd85941c81..eda91fe506 100644 --- a/druid/src/widget/clip_box.rs +++ b/druid/src/widget/clip_box.rs @@ -18,7 +18,7 @@ use crate::kurbo::{Affine, Point, Rect, Size, Vec2}; use crate::widget::prelude::*; use crate::widget::Axis; use crate::{Data, WidgetPod}; -use tracing::{instrument, trace, info, warn}; +use tracing::{info, instrument, trace, warn}; /// Represents the size and position of a rectangular "viewport" into a larger area. #[derive(Clone, Copy, Default, Debug, PartialEq)] @@ -132,12 +132,21 @@ impl Viewport { /// [`SCROLL_TO_VIEW`]: crate::commands::SCROLL_TO_VIEW /// [`scroll_to_view`]: crate::EventCtx::scroll_to_view() /// [`scroll_area_to_view`]: crate::EventCtx::scroll_area_to_view() - pub fn default_scroll_to_view_handling(&mut self, ctx: &mut EventCtx, global_highlight_rect: Rect) -> bool { + pub fn default_scroll_to_view_handling( + &mut self, + ctx: &mut EventCtx, + global_highlight_rect: Rect, + ) -> bool { let mut viewport_changed = false; let global_content_offset = ctx.window_origin().to_vec2() - self.view_origin.to_vec2(); - let mut content_highlight_rect = global_highlight_rect - global_content_offset; - - if self.content_size.to_rect().intersect(content_highlight_rect) != content_highlight_rect { + let content_highlight_rect = global_highlight_rect - global_content_offset; + + if self + .content_size + .to_rect() + .intersect(content_highlight_rect) + != content_highlight_rect + { warn!("tried to bring area outside of the content to view!"); } @@ -161,7 +170,12 @@ impl Viewport { /// [`SCROLL_TO_VIEW`]: crate::commands::SCROLL_TO_VIEW /// [`scroll_to_view`]: crate::EventCtx::scroll_to_view() /// [`scroll_area_to_view`]: crate::EventCtx::scroll_area_to_view() - pub fn fixed_scroll_to_view_handling(&self, ctx: &mut EventCtx, global_highlight_rect: Rect, source: WidgetId) { + pub fn fixed_scroll_to_view_handling( + &self, + ctx: &mut EventCtx, + global_highlight_rect: Rect, + source: WidgetId, + ) { let global_viewport_rect = self.view_rect() + ctx.window_origin().to_vec2(); let clipped_highlight_rect = global_highlight_rect.intersect(global_viewport_rect); @@ -397,7 +411,11 @@ impl> Widget for ClipBox { // to this ClipBox's viewport. ctx.set_handled(); self.with_port(|port| { - port.fixed_scroll_to_view_handling(ctx, *global_highlight_rect, notification.source()); + port.fixed_scroll_to_view_handling( + ctx, + *global_highlight_rect, + notification.source(), + ); }); } } diff --git a/druid/src/widget/scroll.rs b/druid/src/widget/scroll.rs index 0120f39de1..231428b63b 100644 --- a/druid/src/widget/scroll.rs +++ b/druid/src/widget/scroll.rs @@ -195,8 +195,8 @@ impl> Widget for Scroll { if let Event::Notification(notification) = event { if let Some(&global_highlight_rect) = notification.get(SCROLL_TO_VIEW) { ctx.set_handled(); - let view_port_changed = port - .default_scroll_to_view_handling(ctx, global_highlight_rect); + let view_port_changed = + port.default_scroll_to_view_handling(ctx, global_highlight_rect); if view_port_changed { scroll_component .reset_scrollbar_fade(|duration| ctx.request_timer(duration), env); @@ -205,8 +205,6 @@ impl> Widget for Scroll { } } }); - - } #[instrument(name = "Scroll", level = "trace", skip(self, ctx, event, data, env))] From a2dac7a59a6fe632badd397ded25d983b5d82066 Mon Sep 17 00:00:00 2001 From: Christoph Date: Wed, 2 Mar 2022 18:18:13 +0100 Subject: [PATCH 8/9] updated names --- CHANGELOG.md | 4 ++-- druid/src/command.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c20de287b..5e65625ad0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,9 +66,9 @@ You can find its changes [documented below](#070---2021-01-01). - `RangeSlider` and `Annotated` ([#1979] by [@xarvic]) - Add `Checkbox::from_label` constructor ([#2111] by [@maurerdietmar]) - fix content_insets for gtk backend ([#2117] by [@maurerdietmar]) -- `ClipBox::managed`, `Notification::known_target` and `Notification::has_known_target` ([#2141] by [@xarvic]) +- `ClipBox::managed`, `Notification::warn_if_ununsed` and `Notification::warn_if_ununsed_set` ([#2141] by [@xarvic]) - `ClipBox` and `Tabs` handle SCROLL_TO_VIEW ([#2141] by [@xarvic]) -- `EventCtx::submit_notification_unknown_target` ([#2141] by [@xarvic]) +- `EventCtx::submit_notification_without_warning` ([#2141] by [@xarvic]) ### Changed diff --git a/druid/src/command.rs b/druid/src/command.rs index 254c193060..67b65145c4 100644 --- a/druid/src/command.rs +++ b/druid/src/command.rs @@ -352,7 +352,7 @@ pub mod sys { /// [`scroll_to_view`]: crate::EventCtx::scroll_to_view() /// [`scroll_area_to_view`]: crate::EventCtx::scroll_area_to_view() /// [`ClipBox::managed`]: crate::widget::ClipBox::managed() - /// [`ClipBox::default_scroll_to_view_handling`]: crate::widget::ClipBox::default_scroll_to_view_handling() + /// [`Viewport::default_scroll_to_view_handling`]: crate::widget::Viewport::default_scroll_to_view_handling() pub const SCROLL_TO_VIEW: Selector = Selector::new("druid-builtin.scroll-to"); /// A change that has occured to text state, and needs to be From 1bb6dbb13bc8c84a38c42f93ff179275fb4d0f15 Mon Sep 17 00:00:00 2001 From: Christoph Date: Wed, 2 Mar 2022 18:44:34 +0100 Subject: [PATCH 9/9] updated names --- druid/src/command.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/druid/src/command.rs b/druid/src/command.rs index 67b65145c4..8ccba37a56 100644 --- a/druid/src/command.rs +++ b/druid/src/command.rs @@ -347,7 +347,7 @@ pub mod sys { /// a new `SCROLL_TO_VIEW` notification with the same region relative to the new child position. /// /// When building a new widget using ClipBox take a look at [`ClipBox::managed`] and - /// [`ClipBox::default_scroll_to_view_handling`]. + /// [`Viewport::default_scroll_to_view_handling`]. /// /// [`scroll_to_view`]: crate::EventCtx::scroll_to_view() /// [`scroll_area_to_view`]: crate::EventCtx::scroll_area_to_view()