Various changes to comments and documentation

Author: PoignardAzur

masonry/src/action.rs

@@ -5,7 +5,8 @@ use std::any::Any;
 
 use crate::event::PointerButton;
 
-// TODO - Refactor - See issue https://github.com/linebender/xilem/issues/335
+// TODO - Replace actions with an associated type on the Widget trait
+// See https://github.com/linebender/xilem/issues/664
 
 // TODO - TextCursor changed, ImeChanged, EnterKey, MouseEnter
 #[non_exhaustive]

masonry/src/contexts.rs

@@ -3,8 +3,6 @@
 
 //! The context types that are passed into various widget methods.
 
-use std::time::Duration;
-
 use accesskit::TreeUpdate;
 use dpi::LogicalPosition;
 use parley::{FontContext, LayoutContext};
@@ -709,14 +707,6 @@ impl_context_method!(
                 .push_back(RenderRootSignal::ShowWindowMenu(position));
         }
 
-        /// Request a timer event.
-        ///
-        /// The return value is a token, which can be used to associate the
-        /// request with the event.
-        pub fn request_timer(&mut self, _deadline: Duration) -> TimerToken {
-            todo!("request_timer");
-        }
-
         /// Mark child widget as stashed.
         ///
         /// If `stashed` is true, the child will not be painted or listed in the accessibility tree.
@@ -737,9 +727,6 @@ impl_context_method!(
     }
 );
 
-// FIXME - Remove
-pub struct TimerToken;
-
 impl EventCtx<'_> {
     // TODO - clearly document all semantics of pointer capture when they've been decided on
     // TODO - Figure out cases where widget should be notified of pointer capture

masonry/src/event.rs

@@ -181,7 +181,6 @@ impl From<PointerButton> for PointerButtons {
 // TODO - How can RenderRoot express "I started a drag-and-drop op"?
 // TODO - Touchpad, Touch, AxisMotion
 // TODO - How to handle CursorEntered?
-// Note to self: Events like "pointerenter", "pointerleave" are handled differently at the Widget level. But that's weird because WidgetPod can distribute them. Need to think about this again.
 #[derive(Debug, Clone)]
 pub enum PointerEvent {
     PointerDown(PointerButton, PointerState),
@@ -313,6 +312,7 @@ pub enum Update {
 }
 
 impl PointerEvent {
+    /// Create a [`PointerEvent::PointerLeave`] event with dummy values.
     pub fn new_pointer_leave() -> Self {
         // TODO - The fact we're creating so many dummy values might be
         // a sign we should refactor that struct
@@ -328,6 +328,7 @@ impl PointerEvent {
         PointerEvent::PointerLeave(pointer_state)
     }
 
+    /// Returns the [`PointerState`] of the event.
     pub fn pointer_state(&self) -> &PointerState {
         match self {
             PointerEvent::PointerDown(_, state)
@@ -343,13 +344,17 @@ impl PointerEvent {
         }
     }
 
+    /// Returns the position of the pointer event, except for [`PointerEvent::PointerLeave`] and [`PointerEvent::HoverFileCancel`].
     pub fn position(&self) -> Option<LogicalPosition<f64>> {
         match self {
             PointerEvent::PointerLeave(_) | PointerEvent::HoverFileCancel(_) => None,
             _ => Some(self.pointer_state().position),
         }
     }
 
+    /// Short name, for debug logging.
+    ///
+    /// Returns the enum variant name.
     pub fn short_name(&self) -> &'static str {
         match self {
             PointerEvent::PointerDown(_, _) => "PointerDown",
@@ -365,6 +370,10 @@ impl PointerEvent {
         }
     }
 
+    /// Returns true if the event is likely to occur every frame.
+    ///
+    /// Developers should avoid logging during high-density events to avoid
+    /// cluttering the console.
     pub fn is_high_density(&self) -> bool {
         match self {
             PointerEvent::PointerDown(_, _) => false,
@@ -382,20 +391,25 @@ impl PointerEvent {
 }
 
 impl TextEvent {
+    /// Short name, for debug logging.
     pub fn short_name(&self) -> &'static str {
         match self {
-            TextEvent::KeyboardKey(KeyEvent { repeat: true, .. }, _) => "KeyboardKey (repeat)",
+            TextEvent::KeyboardKey(KeyEvent { repeat: true, .. }, _) => "KeyboardKey(repeat)",
             TextEvent::KeyboardKey(_, _) => "KeyboardKey",
             TextEvent::Ime(Ime::Disabled) => "Ime::Disabled",
             TextEvent::Ime(Ime::Enabled) => "Ime::Enabled",
             TextEvent::Ime(Ime::Commit(_)) => "Ime::Commit",
             TextEvent::Ime(Ime::Preedit(s, _)) if s.is_empty() => "Ime::Preedit(\"\")",
-            TextEvent::Ime(Ime::Preedit(_, _)) => "Ime::Preedit",
+            TextEvent::Ime(Ime::Preedit(_, _)) => "Ime::Preedit(\"...\")",
             TextEvent::ModifierChange(_) => "ModifierChange",
             TextEvent::FocusChange(_) => "FocusChange",
         }
     }
 
+    /// Returns true if the event is likely to occur every frame.
+    ///
+    /// Developers should avoid logging during high-density events to avoid
+    /// cluttering the console.
     pub fn is_high_density(&self) -> bool {
         match self {
             TextEvent::KeyboardKey(_, _) => false,
@@ -408,6 +422,9 @@ impl TextEvent {
 }
 
 impl AccessEvent {
+    /// Short name, for debug logging.
+    ///
+    /// Returns the enum variant name.
     pub fn short_name(&self) -> &'static str {
         match self.action {
             accesskit::Action::Click => "Click",
@@ -442,15 +459,6 @@ impl AccessEvent {
 
 impl PointerState {
     pub fn empty() -> Self {
-        #[cfg(FALSE)]
-        #[allow(unsafe_code)]
-        // SAFETY: Uuuuh, unclear. Winit says the dummy id should only be used in
-        // tests and should never be passed to winit. In principle, we're never
-        // passing this id to winit, but it's still visible to custom widgets which
-        // might do so if they tried really hard.
-        // It would be a lot better if winit could just make this constructor safe.
-        let device_id = unsafe { DeviceId::dummy() };
-
         PointerState {
             physical_position: PhysicalPosition::new(0.0, 0.0),
             position: LogicalPosition::new(0.0, 0.0),
@@ -466,7 +474,7 @@ impl PointerState {
 impl Update {
     /// Short name, for debug logging.
     ///
-    /// Essentially returns the enum variant name.
+    /// Returns the enum variant name.
     pub fn short_name(&self) -> &str {
         match self {
             Update::WidgetAdded => "WidgetAdded",

masonry/src/testing/helper_widgets.rs

@@ -16,15 +16,14 @@ use accesskit::{Node, Role};
 use smallvec::SmallVec;
 use tracing::trace_span;
 use vello::Scene;
-use widget::widget::get_child_at_pos;
-use widget::WidgetRef;
 
 use crate::event::{PointerEvent, TextEvent};
-use crate::widget::SizedBox;
-
-// TODO: Expect doesn't work here
-#[allow(clippy::wildcard_imports, reason = "Deferred: Noisy")]
-use crate::*;
+use crate::widget::widget::get_child_at_pos;
+use crate::widget::{SizedBox, WidgetRef};
+use crate::{
+    AccessCtx, AccessEvent, AsAny, BoxConstraints, ComposeCtx, CursorIcon, EventCtx, LayoutCtx,
+    PaintCtx, Point, QueryCtx, RegisterCtx, Size, Update, UpdateCtx, Widget, WidgetId, WidgetPod,
+};
 
 pub type PointerEventFn<S> = dyn FnMut(&mut S, &mut EventCtx, &PointerEvent);
 pub type TextEventFn<S> = dyn FnMut(&mut S, &mut EventCtx, &TextEvent);
@@ -284,13 +283,13 @@ impl<S> ModularWidget<S> {
 
 #[warn(clippy::missing_trait_methods)]
 impl<S: 'static> Widget for ModularWidget<S> {
-    fn on_pointer_event(&mut self, ctx: &mut EventCtx, event: &event::PointerEvent) {
+    fn on_pointer_event(&mut self, ctx: &mut EventCtx, event: &PointerEvent) {
         if let Some(f) = self.on_pointer_event.as_mut() {
             f(&mut self.state, ctx, event);
         }
     }
 
-    fn on_text_event(&mut self, ctx: &mut EventCtx, event: &event::TextEvent) {
+    fn on_text_event(&mut self, ctx: &mut EventCtx, event: &TextEvent) {
         if let Some(f) = self.on_text_event.as_mut() {
             f(&mut self.state, ctx, event);
         }
@@ -441,9 +440,9 @@ impl Widget for ReplaceChild {
         self.child.on_event(ctx, event)
     }
 
-    fn on_pointer_event(&mut self, _ctx: &mut EventCtx, _event: &event::PointerEvent) {}
+    fn on_pointer_event(&mut self, _ctx: &mut EventCtx, _event: &PointerEvent) {}
 
-    fn on_text_event(&mut self, _ctx: &mut EventCtx, _event: &event::TextEvent) {}
+    fn on_text_event(&mut self, _ctx: &mut EventCtx, _event: &TextEvent) {}
 
     fn on_access_event(&mut self, _ctx: &mut EventCtx, _event: &AccessEvent) {}
 
@@ -507,12 +506,12 @@ impl Recording {
 
 #[warn(clippy::missing_trait_methods)]
 impl<W: Widget> Widget for Recorder<W> {
-    fn on_pointer_event(&mut self, ctx: &mut EventCtx, event: &event::PointerEvent) {
+    fn on_pointer_event(&mut self, ctx: &mut EventCtx, event: &PointerEvent) {
         self.recording.push(Record::PE(event.clone()));
         self.child.on_pointer_event(ctx, event);
     }
 
-    fn on_text_event(&mut self, ctx: &mut EventCtx, event: &event::TextEvent) {
+    fn on_text_event(&mut self, ctx: &mut EventCtx, event: &TextEvent) {
         self.recording.push(Record::TE(event.clone()));
         self.child.on_text_event(ctx, event);
     }

masonry/src/text/mod.rs

@@ -4,7 +4,7 @@
 //! Support for text display and rendering
 //!
 //! There are three kinds of text commonly needed:
-//!  1) Entirely display text (e.g. a button)
+//!  1) Non interactive text (e.g. a button's label)
 //!  2) Selectable text (e.g. a paragraph of content)
 //!  3) Editable text (e.g. a search bar)
 //!

masonry/src/widget/align.rs

@@ -163,7 +163,6 @@ fn log_size_warnings(size: Size) {
     }
 }
 
-// --- MARK: TESTS ---
 // --- MARK: TESTS ---
 #[cfg(test)]
 mod tests {

masonry/src/widget/button.rs

@@ -18,7 +18,7 @@ use crate::{
     PointerEvent, QueryCtx, Size, TextEvent, Update, UpdateCtx, Widget, WidgetId,
 };
 
-// the minimum padding added to a button.
+// The minimum padding added to a button.
 // NOTE: these values are chosen to match the existing look of TextBox; these
 // should be reevaluated at some point.
 const LABEL_INSETS: Insets = Insets::uniform_xy(8., 2.);
@@ -207,12 +207,6 @@ impl Widget for Button {
     fn make_trace_span(&self, ctx: &QueryCtx<'_>) -> Span {
         trace_span!("Button", id = ctx.widget_id().trace())
     }
-
-    // FIXME
-    #[cfg(FALSE)]
-    fn get_debug_text(&self) -> Option<String> {
-        Some(self.label.as_ref().text().as_ref().to_string())
-    }
 }
 
 // --- MARK: TESTS ---

masonry/src/widget/flex.rs

@@ -32,10 +32,9 @@ pub struct Flex {
 /// Optional parameters for an item in a [`Flex`] container (row or column).
 ///
 /// Generally, when you would like to add a flexible child to a container,
-/// you can simply call [`with_flex_child`](Flex::with_flex_child), passing the
-/// child and the desired flex factor as a `f64`, which has an impl of
+/// you can simply call [`with_flex_child`](Flex::with_flex_child) or [`add_flex_child`](Flex::add_flex_child),
+/// passing the child and the desired flex factor as a `f64`, which has an impl of
 /// `Into<FlexParams>`.
-// FIXME - "with_flex_child or [`add_flex_child`](FlexMut::add_flex_child)"
 #[derive(Default, Debug, Copy, Clone, PartialEq)]
 pub struct FlexParams {
     flex: Option<f64>,
@@ -540,7 +539,6 @@ impl Flex {
         this.ctx.request_layout();
     }
 
-    // FIXME - Remove Box
     pub fn child_mut<'t>(
         this: &'t mut WidgetMut<'_, Self>,
         idx: usize,

masonry/src/widget/scroll_bar.rs

@@ -17,18 +17,13 @@ use crate::{
     WidgetId,
 };
 
-// RULES
-// -
-
-// TODO - Document names:
-// - grabbybar
-// - empty_space
-// - _z
-// - _length
-
-// TODO - Fade scrollbars? Find out how Linux/MacOS/Windows do it
-// TODO - Rename cursor to oval/rect/bar/grabber/grabbybar
-// TODO - Rename progress to ???
+// TODO
+// - Fade scrollbars? Find out how Linux/MacOS/Windows do it
+// - Rename cursor to oval/rect/bar/grabber/grabbybar
+// - Rename progress to something more descriptive
+// - Document names
+// - Document invariants
+
 pub struct ScrollBar {
     axis: Axis,
     pub(crate) cursor_progress: f64,
@@ -287,41 +282,7 @@ mod tests {
         assert_render_snapshot!(harness, "scrollbar_horizontal_middle");
     }
 
-    // TODO - portal larger than content
-
-    #[cfg(FALSE)]
-    #[test]
-    fn edit_button() {
-        let image_1 = {
-            let button = Button::from_label(
-                Label::new("The quick brown fox jumps over the lazy dog")
-                    .with_text_color(PRIMARY_LIGHT)
-                    .with_text_size(20.0),
-            );
-
-            let mut harness = TestHarness::create_with_size(button, Size::new(50.0, 50.0));
-
-            harness.render()
-        };
-
-        let image_2 = {
-            let button = Button::new("Hello world");
+    // TODO - Add "portal larger than content" test
 
-            let mut harness = TestHarness::create_with_size(button, Size::new(50.0, 50.0));
-
-            harness.edit_root_widget(|mut button, _| {
-                let mut button = button.downcast::<Button>().unwrap();
-                button.set_text("The quick brown fox jumps over the lazy dog");
-
-                let mut label = button.label_mut();
-                label.set_text_color(PRIMARY_LIGHT);
-                label.set_text_size(20.0);
-            });
-
-            harness.render()
-        };
-
-        // We don't use assert_eq because we don't want rich assert
-        assert!(image_1 == image_2);
-    }
+    // TODO - Add WidgetMut tests
 }

masonry/src/widget/sized_box.rs

@@ -324,8 +324,6 @@ impl SizedBox {
         self.padding = padding.into();
         self
     }
-
-    // TODO - child()
 }
 
 // --- MARK: WIDGETMUT ---