fix(core): 🐛 padding should not alter the child's size, as it may dis…

…rupt child's work

CHANGELOG.md

@@ -29,16 +29,17 @@ Please only add new entries below the [Unreleased](#unreleased---releasedate) he
 
 - **core**: Added `Measure` to enable support for percentage values for position, and `Anchor` now supports percentage values. (#672 @M-Adoo)
 - **core**: Add APIs `AppCtx::once_next_frame`, `Window::once_next_frame`, `Window::once_frame_finished` and `Window::once_before_layout`. (#672 @M-Adoo)
-- **painter**: Typography now supports baselines (middle and alphabetic). (#pr @M-Adoo)
+- **painter**: Typography now supports baselines (middle and alphabetic). (#674 @M-Adoo)
 
 ### Fixed
 
 - **core**: Fix set opacity zero no work to it's children. (#671 @wjian23)
 - **core**: Fix TextStyle cause providers mismatched (#671 @wjian23)
 - **core**: Running an animation that is already in progress does not trigger a smooth transition. (#672 @M-Adoo)
 - **core**: The framework incorrectly clamps the layout result of the render widget. (#672 @M-Adoo)
-- **painter**: Fixed text line height does not work correctly. (#pr @M-Adoo)
-- **painter**: Fixed issue with text not being drawn at the middle baseline by default. (#pr @M-Adoo)
+- **core**: Padding will not change the child's size; otherwise, child elements like `Icon` may not work correctly. (#674 @M-Adoo)
+- **painter**: Fixed text line height does not work correctly. (#674 @M-Adoo)
+- **painter**: Fixed issue with text not being drawn at the middle baseline by default. (#674 @M-Adoo)
 
 ## [0.4.0-alpha.19] - 2024-12-18
 

core/src/builtin_widgets/margin.rs

@@ -8,7 +8,28 @@ pub struct EdgeInsets {
   pub top: f32,
 }
 
-/// A widget that create space around its child.
+/// The widget utilizes empty space to surround the child widget.
+///
+/// Both `Padding` and `Margin` utilize empty space to surround the child
+/// widget. The difference lies in the fact that when used as a built-in widget,
+/// padding makes the empty space appear as a part of the child widget, whereas
+/// margin does not. For example:
+///
+/// ```
+/// use ribir::prelude::*;
+///
+/// let _padding = text! {
+///   text: "Background includes the empty space",
+///   padding: EdgeInsets::all(10.),
+///   background: Color::GREEN,
+/// };
+///
+/// let _margin = text! {
+///   text: "Background does not include the empty space",
+///   margin: EdgeInsets::all(10.),
+///   background: Color::GREEN,
+/// };
+/// ```
 #[derive(SingleChild, Default, Clone, PartialEq)]
 pub struct Margin {
   pub margin: EdgeInsets,
@@ -22,24 +43,8 @@ impl Declare for Margin {
 
 impl Render for Margin {
   fn perform_layout(&self, clamp: BoxClamp, ctx: &mut LayoutCtx) -> Size {
-    let thickness = self.margin.thickness();
-    let zero = Size::zero();
-    let min = (clamp.min - thickness).max(zero);
-    let max = (clamp.max - thickness).max(zero);
-    let child_clamp = BoxClamp { min, max };
-
-    let child = ctx.assert_single_child();
-    let size = ctx.perform_child_layout(child, child_clamp);
-    ctx.update_position(child, Point::new(self.margin.left, self.margin.top));
-
-    size + thickness
+    space_around_layout(&self.margin, &clamp, ctx)
   }
-
-  #[inline]
-  fn only_sized_by_parent(&self) -> bool { false }
-
-  #[inline]
-  fn paint(&self, _: &mut PaintingCtx) {}
 }
 
 impl Margin {
@@ -112,6 +117,28 @@ impl std::ops::AddAssign for EdgeInsets {
   }
 }
 
+pub(crate) fn space_around_layout(
+  edges: &EdgeInsets, clamp: &BoxClamp, ctx: &mut LayoutCtx,
+) -> Size {
+  let child = match ctx.single_child() {
+    Some(c) => c,
+    None => return Size::zero(),
+  };
+
+  let thickness = edges.thickness();
+  let zero = Size::zero();
+  let min = (clamp.min - thickness).max(zero);
+  let max = (clamp.max - thickness).max(zero);
+
+  // Shrink the clamp of child.
+  let child_clamp = BoxClamp { min, max };
+  let size = ctx.assert_perform_single_child_layout(child_clamp);
+  ctx.update_position(child, Point::new(edges.left, edges.top));
+
+  // Expand the size, so the child have padding.
+  clamp.clamp(size + thickness)
+}
+
 #[cfg(test)]
 mod tests {
   use ribir_dev_helper::*;

core/src/builtin_widgets/padding.rs

@@ -1,7 +1,28 @@
-use crate::{prelude::*, wrap_render::*};
+use crate::prelude::*;
 
-/// A widget that insets its child by the given padding.
-#[derive(Default)]
+/// The widget utilizes empty space to surround the child widget.
+///
+/// Both `Padding` and `Margin` utilize empty space to surround the child
+/// widget. The difference lies in the fact that when used as a built-in widget,
+/// padding makes the empty space appear as a part of the child widget, whereas
+/// margin does not. For example:
+///
+/// ```
+/// use ribir::prelude::*;
+///
+/// let _padding = text! {
+///   text: "Background includes the empty space",
+///   padding: EdgeInsets::all(10.),
+///   background: Color::GREEN,
+/// };
+///
+/// let _margin = text! {
+///   text: "Background does not include the empty space",
+///   margin: EdgeInsets::all(10.),
+///   background: Color::GREEN,
+/// };
+/// ```
+#[derive(Default, SingleChild)]
 pub struct Padding {
   pub padding: EdgeInsets,
 }
@@ -12,37 +33,9 @@ impl Declare for Padding {
   fn declarer() -> Self::Builder { FatObj::new(()) }
 }
 
-impl_compose_child_for_wrap_render!(Padding);
-
-impl WrapRender for Padding {
-  fn perform_layout(&self, clamp: BoxClamp, host: &dyn Render, ctx: &mut LayoutCtx) -> Size {
-    let thickness = self.padding.thickness();
-    let zero = Size::zero();
-    // Reset children position before layout
-    let (ctx, children) = ctx.split_children();
-    for c in children {
-      ctx.update_position(c, Point::zero());
-    }
-
-    let min = (clamp.min - thickness).max(zero);
-    let max = (clamp.max - thickness).max(zero);
-    // Shrink the clamp of child.
-    let child_clamp = BoxClamp { min, max };
-    let mut size = host.perform_layout(child_clamp, ctx);
-
-    size = clamp.clamp(size + thickness);
-
-    let (ctx, children) = ctx.split_children();
-    // Update the children's positions to ensure they are correctly positioned after
-    // expansion with padding.
-    for c in children {
-      if let Some(pos) = ctx.widget_box_pos(c) {
-        let pos = pos + Vector::new(self.padding.left, self.padding.top);
-        ctx.update_position(c, pos);
-      }
-    }
-
-    size
+impl Render for Padding {
+  fn perform_layout(&self, clamp: BoxClamp, ctx: &mut LayoutCtx) -> Size {
+    space_around_layout(&self.padding, &clamp, ctx)
   }
 }
 
@@ -68,11 +61,32 @@ mod tests {
         }
       }
     }),
-    // padding widget
+    // Padding widget
     LayoutCase::default().with_size(Size::new(101., 100.)),
-    // MockBox
+    // MockMulti widget
     LayoutCase::new(&[0, 0])
       .with_size(Size::new(100., 100.))
-      .with_x(1.)
+      .with_x(1.),
+    // MockBox
+    LayoutCase::new(&[0, 0, 0])
+      .with_size(Size::new(100., 100.))
+      .with_x(0.)
   );
+
+  #[test]
+  #[cfg(not(target_arch = "wasm32"))]
+  fn fix_padding_draw() {
+    crate::reset_test_env!();
+
+    assert_widget_eq_image!(
+      WidgetTester::new(text! {
+        padding: EdgeInsets::all(10.),
+        background: Color::GREEN,
+        text: "Hello, Ribir!"
+      })
+      .with_wnd_size(Size::new(128., 48.))
+      .with_comparison(0.000025),
+      "padding_draw"
+    );
+  }
 }