diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c3ec1591c..3e176ea9ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ You can find its changes [documented below](#070---2021-01-01). - Padding is generic over child widget, impls WidgetWrapper ([#1634] by [@cmyr]) - Menu support was rewritten with support for `Data` ([#1625] by [@jneem]) - Update to piet v0.4.0 (rich text on linux!) ([#1677] by [@cmyr]) +- Flex values that are less than 0.0 will default to 0.0 and warn in release. It will panic in debug mode. ([#1691] by [@arthmis]) ### Deprecated @@ -443,6 +444,7 @@ Last release without a changelog :( [@SecondFlight]: https://github.com/SecondFlight [@lord]: https://github.com/lord [@Lejero]: https://github.com/Lejero +[@arthmis]: https://github.com/arthmis [@ccqpein]: https://github.com/ccqpein [#599]: https://github.com/linebender/druid/pull/599 @@ -660,6 +662,7 @@ Last release without a changelog :( [#1660]: https://github.com/linebender/druid/pull/1660 [#1662]: https://github.com/linebender/druid/pull/1662 [#1677]: https://github.com/linebender/druid/pull/1677 +[#1691]: https://github.com/linebender/druid/pull/1691 [#1698]: https://github.com/linebender/druid/pull/1698 [Unreleased]: https://github.com/linebender/druid/compare/v0.7.0...master diff --git a/druid/src/widget/flex.rs b/druid/src/widget/flex.rs index a91cfdc410..f8254666ae 100644 --- a/druid/src/widget/flex.rs +++ b/druid/src/widget/flex.rs @@ -338,11 +338,13 @@ impl FlexParams { /// can pass an `f64` to any of the functions that take `FlexParams`. /// /// By default, the widget uses the alignment of its parent [`Flex`] container. - /// - /// - /// [`Flex`]: struct.Flex.html - /// [`CrossAxisAlignment`]: enum.CrossAxisAlignment.html pub fn new(flex: f64, alignment: impl Into>) -> Self { + if flex <= 0.0 { + debug_panic!("Flex value should be > 0.0. Flex given was: {}", flex); + } + + let flex = flex.max(0.0); + FlexParams { flex, alignment: alignment.into(), @@ -416,7 +418,7 @@ impl Flex { /// /// Convenient for assembling a group of widgets in a single expression. pub fn with_child(mut self, child: impl Widget + 'static) -> Self { - self.add_flex_child(child, 0.0); + self.add_child(child); self } @@ -505,9 +507,13 @@ impl Flex { /// /// See also [`with_child`]. /// - /// [`with_child`]: #method.with_child + /// [`with_child`]: Flex::with_child pub fn add_child(&mut self, child: impl Widget + 'static) { - self.add_flex_child(child, 0.0); + let child = Child::Fixed { + widget: WidgetPod::new(Box::new(child)), + alignment: None, + }; + self.children.push(child); } /// Add a flexible child widget. @@ -533,24 +539,24 @@ impl Flex { /// my_row.add_flex_child(Slider::new(), FlexParams::new(1.0, CrossAxisAlignment::End)); /// ``` /// - /// [`FlexParams`]: struct.FlexParams.html - /// [`with_flex_child`]: #method.with_flex_child + /// [`with_flex_child`]: Flex::with_flex_child pub fn add_flex_child( &mut self, child: impl Widget + 'static, params: impl Into, ) { let params = params.into(); - let child = if params.flex == 0.0 { - Child::Fixed { + let child = if params.flex > 0.0 { + Child::Flex { widget: WidgetPod::new(Box::new(child)), alignment: params.alignment, + flex: params.flex, } } else { - Child::Flex { + tracing::warn!("Flex value should be > 0.0. To add a non-flex child use the add_child or with_child methods.\nSee the docs for more information: https://docs.rs/druid/0.7.0/druid/widget/struct.Flex.html"); + Child::Fixed { widget: WidgetPod::new(Box::new(child)), - alignment: params.alignment, - flex: params.flex, + alignment: None, } }; self.children.push(child); @@ -573,15 +579,33 @@ impl Flex { /// If you are laying out standard controls in this container, you should /// generally prefer to use [`add_default_spacer`]. /// - /// [`add_default_spacer`]: #method.add_default_spacer + /// [`add_default_spacer`]: Flex::add_default_spacer pub fn add_spacer(&mut self, len: impl Into>) { - let value = len.into(); + let mut value = len.into(); + if let KeyOrValue::Concrete(ref mut len) = value { + if *len < 0.0 { + tracing::warn!("Provided spacer length was less than 0. Value was: {}", len); + } + *len = len.clamp(0.0, f64::MAX); + } + let new_child = Child::FixedSpacer(value, 0.0); self.children.push(new_child); } /// Add an empty spacer widget with a specific `flex` factor. pub fn add_flex_spacer(&mut self, flex: f64) { + let flex = if flex >= 0.0 { + flex + } else { + debug_assert!( + flex >= 0.0, + "flex value for space should be greater than equal to 0, received: {}", + flex + ); + tracing::warn!("Provided flex value was less than 0: {}", flex); + 0.0 + }; let new_child = Child::FlexedSpacer(flex, 0.0); self.children.push(new_child); } @@ -652,6 +676,10 @@ impl Widget for Flex { } Child::FixedSpacer(kv, calculated_siz) => { *calculated_siz = kv.resolve(env); + if *calculated_siz < 0.0 { + tracing::warn!("Length provided to fixed spacer was less than 0"); + } + *calculated_siz = calculated_siz.max(0.0); major_non_flex += *calculated_siz; } Child::Flex { flex, .. } | Child::FlexedSpacer(flex, _) => flex_sum += *flex, @@ -941,10 +969,7 @@ impl Iterator for Spacing { impl From for FlexParams { fn from(flex: f64) -> FlexParams { - FlexParams { - flex, - alignment: None, - } + FlexParams::new(flex, None) } } @@ -1051,4 +1076,18 @@ mod tests { assert_eq!(vec(a, 38., 5), vec![4., 7., 8., 8., 7., 4.]); assert_eq!(vec(a, 39., 5), vec![4., 8., 7., 8., 8., 4.]); } + + #[test] + #[should_panic] + fn test_invalid_flex_params() { + use float_cmp::approx_eq; + let params = FlexParams::new(0.0, None); + approx_eq!(f64, params.flex, 1.0, ulps = 2); + + let params = FlexParams::new(-0.0, None); + approx_eq!(f64, params.flex, 1.0, ulps = 2); + + let params = FlexParams::new(-1.0, None); + approx_eq!(f64, params.flex, 1.0, ulps = 2); + } }