From 2dc1c08b80b2d755a4adac962a6f6b214714119f Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Fri, 19 Jul 2024 15:14:38 +0100 Subject: [PATCH] Add a CSS `gap` inspired Flex property. (#437) See https://developer.mozilla.org/en-US/docs/Web/CSS/gap Since #428, we no longer have default spacing between flex elements in Masonry. This makes the examples quite ugly. Choices made based on [#xilem > Porting Taffy Integration to New Xilem](https://xi.zulipchat.com/#narrow/stream/354396-xilem/topic/Porting.20Taffy.20Integration.20to.20New.20Xilem). Of particular note is the choice to not have this be overwritten by spacers, due to: > My first thought is that users are going to be very confused when they add a space between two items and the result is the items are closer together. There is no such concept of a spacer in CSS parlance --- masonry/examples/calc.rs | 2 + masonry/src/widget/flex.rs | 122 +++++++++++++++++++++++++++++++++---- xilem/src/view/flex.rs | 34 ++++++++++- 3 files changed, 144 insertions(+), 14 deletions(-) diff --git a/masonry/examples/calc.rs b/masonry/examples/calc.rs index f099d7cd..e4e61b04 100644 --- a/masonry/examples/calc.rs +++ b/masonry/examples/calc.rs @@ -300,6 +300,7 @@ fn flex_row( w4: impl Widget + 'static, ) -> impl Widget { Flex::row() + .gap(0.0) .with_flex_child(w1, 1.0) .with_spacer(1.0) .with_flex_child(w2, 1.0) @@ -312,6 +313,7 @@ fn flex_row( fn build_calc() -> impl Widget { let display = Label::new(String::new()).with_text_size(32.0); Flex::column() + .gap(0.0) .with_flex_spacer(0.2) .with_child(display) .with_flex_spacer(0.2) diff --git a/masonry/src/widget/flex.rs b/masonry/src/widget/flex.rs index b8313518..9a5636ac 100644 --- a/masonry/src/widget/flex.rs +++ b/masonry/src/widget/flex.rs @@ -28,6 +28,7 @@ pub struct Flex { fill_major_axis: bool, children: Vec, old_bc: BoxConstraints, + gap: Option, } /// Optional parameters for an item in a [`Flex`] container (row or column). @@ -105,6 +106,7 @@ impl Flex { main_alignment: MainAxisAlignment::Start, fill_major_axis: false, old_bc: BoxConstraints::tight(Size::ZERO), + gap: None, } } @@ -142,6 +144,52 @@ impl Flex { self } + /// Builder-style method for setting the spacing along the + /// major axis between any two elements in logical pixels. + /// + /// Equivalent to the css [gap] property. + /// This gap is also present between spacers. + /// + /// See also [`default_gap`](Self::default_gap). + /// + /// ## Panics + /// + /// If `gap` is not a non-negative finite value. + /// + /// [gap]: https://developer.mozilla.org/en-US/docs/Web/CSS/gap + // TODO: Semantics - should this include fixed spacers? + pub fn gap(mut self, gap: f64) -> Self { + if gap.is_finite() && gap >= 0.0 { + self.gap = Some(gap); + } else { + panic!("Invalid `gap` {gap}, expected a non-negative finite value.") + } + self + } + + /// Builder-style method to use the default gap value. + /// + /// This is [`WIDGET_PADDING_VERTICAL`] for a flex column and + /// [`WIDGET_PADDING_HORIZONTAL`] for flex row. + /// + /// See also [`gap`](Self::gap) + /// + /// [`WIDGET_PADDING_VERTICAL`]: crate::theme::WIDGET_PADDING_VERTICAL + /// [`WIDGET_PADDING_HORIZONTAL`]: crate::theme::WIDGET_PADDING_VERTICAL + pub fn default_gap(mut self) -> Self { + self.gap = None; + self + } + + /// Equivalent to [`gap`](Self::gap) if `gap` is `Some`, or + /// [`default_gap`](Self::default_gap) otherwise. + /// + /// Does not perform validation of the provided value. + pub fn raw_gap(mut self, gap: Option) -> Self { + self.gap = gap; + self + } + /// Builder-style variant of `add_child`. /// /// Convenient for assembling a group of widgets in a single expression. @@ -189,10 +237,7 @@ impl Flex { /// The actual value of this spacer depends on whether this container is /// a row or column, as well as theme settings. pub fn with_default_spacer(self) -> Self { - let key = match self.direction { - Axis::Vertical => crate::theme::WIDGET_PADDING_VERTICAL, - Axis::Horizontal => crate::theme::WIDGET_PADDING_HORIZONTAL, - }; + let key = axis_default_spacer(self.direction); self.with_spacer(key) } @@ -262,6 +307,50 @@ impl<'a> WidgetMut<'a, Flex> { self.ctx.request_layout(); } + /// Set the spacing along the major axis between any two elements in logical pixels. + /// + /// Equivalent to the css [gap] property. + /// This gap is also present between spacers. + /// + /// [gap]: https://developer.mozilla.org/en-US/docs/Web/CSS/gap + /// + /// ## Panics + /// + /// If `gap` is not a non-negative finite value. + /// + /// See also [`use_default_gap`](Self::use_default_gap). + pub fn set_gap(&mut self, gap: f64) { + if gap.is_finite() && gap >= 0.0 { + self.widget.gap = Some(gap); + } else { + panic!("Invalid `gap` {gap}, expected a non-negative finite value.") + } + self.ctx.request_layout(); + } + + /// Use the default gap value. + /// + /// This is [`WIDGET_PADDING_VERTICAL`] for a flex column and + /// [`WIDGET_PADDING_HORIZONTAL`] for flex row. + /// + /// See also [`set_gap`](Self::set_gap) + /// + /// [`WIDGET_PADDING_VERTICAL`]: crate::theme::WIDGET_PADDING_VERTICAL + /// [`WIDGET_PADDING_HORIZONTAL`]: crate::theme::WIDGET_PADDING_VERTICAL + pub fn use_default_gap(&mut self) { + self.widget.gap = None; + self.ctx.request_layout(); + } + + /// Equivalent to [`set_gap`](Self::set_gap) if `gap` is `Some`, or + /// [`use_default_gap`](Self::use_default_gap) otherwise. + /// + /// Does not perform validation of the provided value. + pub fn set_raw_gap(&mut self, gap: Option) { + self.widget.gap = gap; + self.ctx.request_layout(); + } + /// Add a non-flex child widget. /// /// See also [`with_child`]. @@ -299,10 +388,7 @@ impl<'a> WidgetMut<'a, Flex> { /// The actual value of this spacer depends on whether this container is /// a row or column, as well as theme settings. pub fn add_default_spacer(&mut self) { - let key = match self.widget.direction { - Axis::Vertical => crate::theme::WIDGET_PADDING_VERTICAL, - Axis::Horizontal => crate::theme::WIDGET_PADDING_HORIZONTAL, - }; + let key = axis_default_spacer(self.widget.direction); self.add_spacer(key); self.ctx.request_layout(); } @@ -384,10 +470,7 @@ impl<'a> WidgetMut<'a, Flex> { /// The actual value of this spacer depends on whether this container is /// a row or column, as well as theme settings. pub fn insert_default_spacer(&mut self, idx: usize) { - let key = match self.widget.direction { - Axis::Vertical => crate::theme::WIDGET_PADDING_VERTICAL, - Axis::Horizontal => crate::theme::WIDGET_PADDING_HORIZONTAL, - }; + let key = axis_default_spacer(self.widget.direction); self.insert_spacer(idx, key); self.ctx.request_layout(); } @@ -511,6 +594,14 @@ impl<'a> WidgetMut<'a, Flex> { } } +/// The size in logical pixels of the default spacer for an axis. +fn axis_default_spacer(axis: Axis) -> f64 { + match axis { + Axis::Vertical => crate::theme::WIDGET_PADDING_VERTICAL, + Axis::Horizontal => crate::theme::WIDGET_PADDING_HORIZONTAL, + } +} + fn new_flex_child(params: FlexParams, widget: WidgetPod>) -> Child { if let Some(flex) = params.flex { if flex.is_normal() && flex > 0.0 { @@ -580,8 +671,11 @@ impl Widget for Flex { let mut any_changed = bc_changed; self.old_bc = *bc; + let gap = self.gap.unwrap_or(axis_default_spacer(self.direction)); + // The gaps are only between the items, so 2 children means 1 gap. + let total_gap = self.children.len().saturating_sub(1) as f64 * gap; // Measure non-flex children. - let mut major_non_flex = 0.0; + let mut major_non_flex = total_gap; let mut flex_sum = 0.0; for child in &mut self.children { match child { @@ -756,10 +850,12 @@ impl Widget for Flex { child_paint_rect = child_paint_rect.union(ctx.widget_state.paint_rect()); major += self.direction.major(child_size).expand(); major += spacing.next().unwrap_or(0.); + major += gap; } Child::FlexedSpacer(_, calculated_size) | Child::FixedSpacer(_, calculated_size) => { major += *calculated_size; + major += gap; } } } diff --git a/xilem/src/view/flex.rs b/xilem/src/view/flex.rs index 30caf048..041211cb 100644 --- a/xilem/src/view/flex.rs +++ b/xilem/src/view/flex.rs @@ -24,6 +24,7 @@ pub fn flex(sequence: Seq) -> Flex { cross_axis_alignment: CrossAxisAlignment::Center, main_axis_alignment: MainAxisAlignment::Start, fill_major_axis: false, + gap: None, } } @@ -34,6 +35,7 @@ pub struct Flex { main_axis_alignment: MainAxisAlignment, fill_major_axis: bool, phantom: PhantomData Marker>, + gap: Option, } impl Flex { @@ -55,6 +57,32 @@ impl Flex { self.fill_major_axis = fill_major_axis; self } + + /// Set the spacing along the major axis between any two elements in logical pixels. + /// + /// Equivalent to the css [gap] property. + /// This gap is also present between spacers. + /// + /// Leave unset to use the default spacing, which is [`WIDGET_PADDING_VERTICAL`] for a flex + /// column and [`WIDGET_PADDING_HORIZONTAL`] for flex row. + /// + /// ## Panics + /// + /// If `gap` is not a non-negative finite value. + /// + /// [gap]: https://developer.mozilla.org/en-US/docs/Web/CSS/gap + /// [`WIDGET_PADDING_VERTICAL`]: masonry::theme::WIDGET_PADDING_VERTICAL + /// [`WIDGET_PADDING_HORIZONTAL`]: masonry::theme::WIDGET_PADDING_VERTICAL + #[track_caller] + pub fn gap(mut self, gap: f64) -> Self { + if gap.is_finite() && gap >= 0.0 { + self.gap = Some(gap); + } else { + // TODO: Don't panic here, for future editor scenarios. + panic!("Invalid `gap` {gap}, expected a non-negative finite value.") + } + self + } } impl View for Flex @@ -68,6 +96,7 @@ where fn build(&self, ctx: &mut ViewCtx) -> (Self::Element, Self::ViewState) { let mut elements = AppendVec::default(); let mut widget = widget::Flex::for_axis(self.axis) + .raw_gap(self.gap) .cross_axis_alignment(self.cross_axis_alignment) .must_fill_main_axis(self.fill_major_axis) .main_axis_alignment(self.main_axis_alignment); @@ -107,7 +136,10 @@ where element.set_must_fill_main_axis(self.fill_major_axis); ctx.mark_changed(); } - + if prev.gap != self.gap { + element.set_raw_gap(self.gap); + ctx.mark_changed(); + } // TODO: Re-use scratch space? let mut splice = FlexSplice::new(element); self.sequence