Resolve mismatch between label width and alignment

masonry/src/text/text_layout.rs

@@ -255,6 +255,9 @@ impl TextLayout {
     }
 
     /// Set the [`Alignment`] for this layout.
+    ///
+    /// This alignment can only be meaningful when a
+    /// [maximum width](Self::set_max_advance) is provided.
     pub fn set_text_alignment(&mut self, alignment: Alignment) {
         if self.alignment != alignment {
             self.alignment = alignment;
@@ -313,20 +316,43 @@ impl TextLayout {
         &self.layout
     }
 
-    /// The size of the laid-out text, excluding any trailing whitespace.
+    /// The size of the region the text should be drawn in,
+    /// excluding any trailing whitespace if present.
+    ///
+    /// Should be used for the drawing of non-interactive text (as the
+    /// trailing whitespace is selectable for interactive text).
     ///
     /// This is not meaningful until [`Self::rebuild`] has been called.
     pub fn size(&self) -> Size {
         self.assert_rebuilt("size");
-        Size::new(self.layout.width().into(), self.layout.height().into())
+        Size::new(
+            self.layout_width(self.layout.width()).into(),
+            self.layout.height().into(),
+        )
     }
 
     /// The size of the laid-out text, including any trailing whitespace.
     ///
+    /// Should be used for the drawing of interactive text.
+    ///
     /// This is not meaningful until [`Self::rebuild`] has been called.
     pub fn full_size(&self) -> Size {
         self.assert_rebuilt("full_size");
-        Size::new(self.layout.full_width().into(), self.layout.height().into())
+        Size::new(
+            self.layout_width(self.layout.full_width()).into(),
+            self.layout.height().into(),
+        )
+    }
+
+    /// If performing layout `max_advance` to calculate text alignment, the only
+    /// reasonable behaviour is to take up the entire available width.
+    ///
+    /// The coherent way to have multiple items laid out on the same line is for them to
+    /// be inside the same text layout object "region". This is currently deferred.
+    /// As an interim solution, we allow multiple items to be on the same line if the `max_advance` wasn't used
+    /// (and therefore the text alignment argument is effectively ignored).
+    fn layout_width(&self, width: f32) -> f32 {
+        self.max_advance.unwrap_or(width)
     }
 
     /// Return the text's [`LayoutMetrics`].

masonry/src/widget/label.rs

@@ -20,8 +20,9 @@ use crate::{
     RegisterCtx, TextEvent, Update, UpdateCtx, Widget, WidgetId,
 };
 
-// added padding between the edges of the widget and the text.
-pub(super) const LABEL_X_PADDING: f64 = 2.0;
+/// Added padding between each horizontal edge of the widget
+/// and the text in logical pixels.
+const LABEL_X_PADDING: f64 = 2.0;
 
 /// Options for handling lines that are too wide for the label.
 #[derive(Debug, Clone, Copy, PartialEq)]
@@ -202,7 +203,9 @@ impl Widget for Label {
                 .rebuild(font_ctx, layout_ctx, &self.text, self.text_changed);
             self.text_changed = false;
         }
-        // We ignore trailing whitespace for a label
+        // We would like to ignore trailing whitespace for a label.
+        // However, Parley doesn't make that an option when using `max_advance`.
+        // If we aren't wrapping words, we can safely ignore this, however.
         let text_size = self.text_layout.size();
         let label_size = Size {
             height: text_size.height,
@@ -265,7 +268,7 @@ mod tests {
     use crate::assert_render_snapshot;
     use crate::testing::TestHarness;
     use crate::theme::{PRIMARY_DARK, PRIMARY_LIGHT};
-    use crate::widget::{Flex, SizedBox};
+    use crate::widget::{CrossAxisAlignment, Flex, SizedBox};
 
     #[test]
     fn simple_label() {
@@ -291,6 +294,35 @@ mod tests {
         assert_render_snapshot!(harness, "styled_label");
     }
 
+    #[test]
+    /// A wrapping label's alignment should be respected, regardkess of
+    /// its parent's alignment.
+    fn label_alignment_flex() {
+        fn base_label() -> Label {
+            Label::new("Hello")
+                .with_text_size(10.0)
+                .with_line_break_mode(LineBreaking::WordWrap)
+        }
+        let label1 = base_label().with_text_alignment(Alignment::Start);
+        let label2 = base_label().with_text_alignment(Alignment::Middle);
+        let label3 = base_label().with_text_alignment(Alignment::End);
+        let label4 = base_label().with_text_alignment(Alignment::Start);
+        let label5 = base_label().with_text_alignment(Alignment::Middle);
+        let label6 = base_label().with_text_alignment(Alignment::End);
+        let flex = Flex::column()
+            .with_flex_child(label1, CrossAxisAlignment::Start)
+            .with_flex_child(label2, CrossAxisAlignment::Start)
+            .with_flex_child(label3, CrossAxisAlignment::Start)
+            .with_flex_child(label4, CrossAxisAlignment::Center)
+            .with_flex_child(label5, CrossAxisAlignment::Center)
+            .with_flex_child(label6, CrossAxisAlignment::Center)
+            .gap(0.0);
+
+        let mut harness = TestHarness::create_with_size(flex, Size::new(80.0, 80.0));
+
+        assert_render_snapshot!(harness, "label_alignment_flex");
+    }
+
     #[test]
     fn line_break_modes() {
         let widget = Flex::column()

masonry/src/widget/prose.rs

@@ -11,13 +11,16 @@ use vello::peniko::BlendMode;
 use vello::Scene;
 
 use crate::text::{ArcStr, TextBrush, TextWithSelection};
-use crate::widget::label::LABEL_X_PADDING;
 use crate::widget::{LineBreaking, WidgetMut};
 use crate::{
     AccessCtx, AccessEvent, BoxConstraints, CursorIcon, EventCtx, LayoutCtx, PaintCtx,
     PointerEvent, QueryCtx, RegisterCtx, TextEvent, Update, UpdateCtx, Widget, WidgetId,
 };
 
+/// Added padding between each horizontal edge of the widget
+/// and the text in logical pixels.
+const PROSE_X_PADDING: f64 = 2.0;
+
 /// The prose widget is a widget which displays text which can be
 /// selected with keyboard and mouse, and which can be copied from,
 /// but cannot be modified by the user.
@@ -136,7 +139,7 @@ impl Prose {
 impl Widget for Prose {
     fn on_pointer_event(&mut self, ctx: &mut EventCtx, event: &PointerEvent) {
         let window_origin = ctx.widget_state.window_origin();
-        let inner_origin = Point::new(window_origin.x + LABEL_X_PADDING, window_origin.y);
+        let inner_origin = Point::new(window_origin.x + PROSE_X_PADDING, window_origin.y);
         match event {
             PointerEvent::PointerDown(button, state) => {
                 if !ctx.is_disabled() {
@@ -223,7 +226,7 @@ impl Widget for Prose {
             None
         } else if bc.max().width.is_finite() {
             // TODO: Does Prose have different needs here?
-            Some(bc.max().width as f32 - 2. * LABEL_X_PADDING as f32)
+            Some(bc.max().width as f32 - 2. * PROSE_X_PADDING as f32)
         } else if bc.min().width.is_sign_negative() {
             Some(0.0)
         } else {
@@ -234,11 +237,11 @@ impl Widget for Prose {
             let (font_ctx, layout_ctx) = ctx.text_contexts();
             self.text_layout.rebuild(font_ctx, layout_ctx);
         }
-        // We ignore trailing whitespace for a label
-        let text_size = self.text_layout.size();
+        // We include trailing whitespace for prose, as it can be selected.
+        let text_size = self.text_layout.full_size();
         let label_size = Size {
             height: text_size.height,
-            width: text_size.width + 2. * LABEL_X_PADDING,
+            width: text_size.width + 2. * PROSE_X_PADDING,
         };
         bc.constrain(label_size)
     }
@@ -255,7 +258,7 @@ impl Widget for Prose {
             scene.push_layer(BlendMode::default(), 1., Affine::IDENTITY, &clip_rect);
         }
         self.text_layout
-            .draw(scene, Point::new(LABEL_X_PADDING, 0.0));
+            .draw(scene, Point::new(PROSE_X_PADDING, 0.0));
 
         if self.line_break_mode == LineBreaking::Clip {
             scene.pop_layer();
@@ -289,4 +292,45 @@ impl Widget for Prose {
     }
 }
 
-// TODO - Add tests
+// TODO - Add more tests
+#[cfg(test)]
+mod tests {
+    use parley::layout::Alignment;
+    use vello::kurbo::Size;
+
+    use crate::{
+        assert_render_snapshot,
+        testing::TestHarness,
+        widget::{CrossAxisAlignment, Flex, LineBreaking, Prose},
+    };
+
+    #[test]
+    /// A wrapping prose's alignment should be respected, regardkess of
+    /// its parent's alignment.
+    fn prose_alignment_flex() {
+        fn base_label() -> Prose {
+            // Trailing whitespace is displayed when laying out prose.
+            Prose::new("Hello  ")
+                .with_text_size(10.0)
+                .with_line_break_mode(LineBreaking::WordWrap)
+        }
+        let label1 = base_label().with_text_alignment(Alignment::Start);
+        let label2 = base_label().with_text_alignment(Alignment::Middle);
+        let label3 = base_label().with_text_alignment(Alignment::End);
+        let label4 = base_label().with_text_alignment(Alignment::Start);
+        let label5 = base_label().with_text_alignment(Alignment::Middle);
+        let label6 = base_label().with_text_alignment(Alignment::End);
+        let flex = Flex::column()
+            .with_flex_child(label1, CrossAxisAlignment::Start)
+            .with_flex_child(label2, CrossAxisAlignment::Start)
+            .with_flex_child(label3, CrossAxisAlignment::Start)
+            .with_flex_child(label4, CrossAxisAlignment::Center)
+            .with_flex_child(label5, CrossAxisAlignment::Center)
+            .with_flex_child(label6, CrossAxisAlignment::Center)
+            .gap(0.0);
+
+        let mut harness = TestHarness::create_with_size(flex, Size::new(80.0, 80.0));
+
+        assert_render_snapshot!(harness, "prose_alignment_flex");
+    }
+}

xilem/examples/http_cats.rs

@@ -13,8 +13,8 @@ use winit::window::Window;
 use xilem::core::fork;
 use xilem::core::one_of::OneOf3;
 use xilem::view::{
-    button, flex, image, portal, prose, sized_box, spinner, worker, Axis, CrossAxisAlignment,
-    FlexExt, FlexSpacer,
+    button, flex, image, inline_prose, portal, prose, sized_box, spinner, worker, Axis, FlexExt,
+    FlexSpacer,
 };
 use xilem::{Color, EventLoop, EventLoopBuilder, TextAlignment, WidgetView, Xilem};
 
@@ -102,12 +102,10 @@ impl HttpCats {
                     portal(sized_box(info_area).expand_width()).flex(1.),
                 ))
                 .direction(Axis::Horizontal)
-                .cross_axis_alignment(CrossAxisAlignment::Fill)
                 .must_fill_major_axis(true)
                 .flex(1.),
             ))
-            .must_fill_major_axis(true)
-            .cross_axis_alignment(CrossAxisAlignment::Fill),
+            .must_fill_major_axis(true),
             worker(
                 worker_value,
                 |proxy, mut rx| async move {
@@ -164,8 +162,8 @@ impl Status {
         let code = self.code;
         flex((
             // TODO: Reduce allocations here?
-            prose(self.code.to_string()),
-            prose(self.message),
+            inline_prose(self.code.to_string()),
+            inline_prose(self.message),
             FlexSpacer::Flex(1.),
             // TODO: Spinner if image pending?
             // TODO: Tick if image loaded?
@@ -199,7 +197,6 @@ impl Status {
             prose("Copyright ©️ https://http.cat      ").alignment(TextAlignment::End),
         ))
         .main_axis_alignment(xilem::view::MainAxisAlignment::Start)
-        .cross_axis_alignment(CrossAxisAlignment::Fill)
     }
 }
 

xilem/examples/mason.rs

@@ -61,9 +61,7 @@ fn app_logic(data: &mut AppData) -> impl WidgetView<AppData> {
     fork(
         flex((
             flex((
-                label("Label")
-                    .brush(Color::REBECCA_PURPLE)
-                    .alignment(TextAlignment::Start),
+                label("Label").brush(Color::REBECCA_PURPLE),
                 label("Bold Label").weight(TextWeight::BOLD),
                 // TODO masonry doesn't allow setting disabled manually anymore?
                 // label("Disabled label").disabled(),