Skip to content

Commit

Permalink
Fix the Size helper functions using the wrong default value and imp…
Browse files Browse the repository at this point in the history 
…rove the UI examples (#7626)

# Objective
`Size::width` sets the `height` field to `Val::DEFAULT` which is `Val::Undefined`, but the default for `Size` `height` is `Val::Auto`. 
`Size::height` has the same problem, but with the `width` field. 

The UI examples specify numeric values in many places where they could either be elided or replaced by composition of the Flex enum properties.

related: #7468
fixes: #6498

## Solution
Change `Size::width` so it sets `height` to `Val::AUTO` and change `Size::height` so it sets `width` to `Val::AUTO`.
Added some tests so this doesn't happen again.

## Changelog
Changed `Size::width` so it sets the `height` to `Val::AUTO`.
Changed `Size::height` so it sets the `width` to `Val::AUTO`.
Added tests to `geometry.rs` for `Size` and `UiRect` to ensure correct behaviour.
Simplified the UI examples. Replaced numeric values with the Flex property enums or elided them where possible, and removed the remaining use of auto margins.

## Migration Guide
The `Size::width` constructor function now sets the `height` to `Val::Auto` instead of `Val::Undefined`.
The `Size::height` constructor function now sets the `width` to `Val::Auto` instead of `Val::Undefined`.
  • Loading branch information
@ickshonpe
ickshonpe committed Feb 11, 2023
1 parent a1e4114 commit eaac730
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 45 deletions.
71 changes: 68 additions & 3 deletions crates/bevy_ui/src/geometry.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ pub struct Size {
}

impl Size {
pub const DEFAULT: Self = Self::all(Val::Auto);
pub const DEFAULT: Self = Self::AUTO;

/// Creates a new [`Size`] from a width and a height.
///
Expand Down Expand Up @@ -369,14 +369,14 @@ impl Size {
pub const fn width(width: Val) -> Self {
Self {
width,
height: Val::DEFAULT,
height: Val::Auto,
}
}

/// Creates a new [`Size`] where `height` takes the given value.
pub const fn height(height: Val) -> Self {
Self {
width: Val::DEFAULT,
width: Val::Auto,
height,
}
}
Expand Down Expand Up @@ -443,6 +443,20 @@ impl DivAssign<f32> for Size {
mod tests {
use super::*;

#[test]
fn uirect_default_equals_const_default() {
assert_eq!(
UiRect::default(),
UiRect {
left: Val::Undefined,
right: Val::Undefined,
top: Val::Undefined,
bottom: Val::Undefined
}
);
assert_eq!(UiRect::default(), UiRect::DEFAULT);
}

#[test]
fn test_size_from() {
let size: Size = (Val::Px(20.), Val::Px(30.)).into();
Expand Down Expand Up @@ -476,4 +490,55 @@ mod tests {
size /= 2.;
assert_eq!(size, Size::new(Val::Px(10.), Val::Px(10.)));
}

#[test]
fn test_size_all() {
let length = Val::Px(10.);

assert_eq!(
Size::all(length),
Size {
width: length,
height: length
}
);
}

#[test]
fn test_size_width() {
let width = Val::Px(10.);

assert_eq!(
Size::width(width),
Size {
width,
..Default::default()
}
);
}

#[test]
fn test_size_height() {
let height = Val::Px(7.);

assert_eq!(
Size::height(height),
Size {
height,
..Default::default()
}
);
}

#[test]
fn size_default_equals_const_default() {
assert_eq!(
Size::default(),
Size {
width: Val::Auto,
height: Val::Auto
}
);
assert_eq!(Size::default(), Size::DEFAULT);
}
}
2 changes: 1 addition & 1 deletion examples/ui/button.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
commands
.spawn(NodeBundle {
style: Style {
size: Size::new(Val::Percent(100.0), Val::Percent(100.0)),
size: Size::width(Val::Percent(100.0)),
align_items: AlignItems::Center,
justify_content: JustifyContent::Center,
..default()
Expand Down
6 changes: 3 additions & 3 deletions examples/ui/relative_cursor_position.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
commands
.spawn(NodeBundle {
style: Style {
size: Size::new(Val::Percent(100.0), Val::Percent(100.0)),
size: Size::width(Val::Percent(100.)),
align_items: AlignItems::Center,
justify_content: JustifyContent::Center,
flex_direction: FlexDirection::Column,
Expand All @@ -30,8 +30,8 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
parent
.spawn(NodeBundle {
style: Style {
size: Size::new(Val::Px(250.0), Val::Px(250.0)),
margin: UiRect::new(Val::Px(0.), Val::Px(0.), Val::Px(0.), Val::Px(15.)),
size: Size::all(Val::Px(250.0)),
margin: UiRect::bottom(Val::Px(15.)),
..default()
},
background_color: Color::rgb(235., 35., 12.).into(),
Expand Down
16 changes: 2 additions & 14 deletions examples/ui/transparency_ui.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
commands
.spawn(NodeBundle {
style: Style {
size: Size::new(Val::Percent(50.0), Val::Percent(100.0)),
size: Size::width(Val::Percent(100.0)),
align_items: AlignItems::Center,
justify_content: JustifyContent::Center,
justify_content: JustifyContent::SpaceAround,
..default()
},
..default()
Expand Down Expand Up @@ -49,19 +49,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
},
));
});
});

commands
.spawn(NodeBundle {
style: Style {
size: Size::new(Val::Percent(50.0), Val::Percent(100.0)),
align_items: AlignItems::Center,
justify_content: JustifyContent::Center,
..default()
},
..default()
})
.with_children(|parent| {
// Button with a different color,
// to demonstrate the text looks different due to its transparency.
parent
Expand Down
39 changes: 16 additions & 23 deletions examples/ui/ui.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
commands
.spawn(NodeBundle {
style: Style {
size: Size::new(Val::Percent(100.0), Val::Percent(100.0)),
size: Size::width(Val::Percent(100.0)),
justify_content: JustifyContent::SpaceBetween,
..default()
},
Expand All @@ -35,7 +35,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
parent
.spawn(NodeBundle {
style: Style {
size: Size::new(Val::Px(200.0), Val::Percent(100.0)),
size: Size::width(Val::Px(200.0)),
border: UiRect::all(Val::Px(2.0)),
..default()
},
Expand All @@ -47,7 +47,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
parent
.spawn(NodeBundle {
style: Style {
size: Size::new(Val::Percent(100.0), Val::Percent(100.0)),
size: Size::width(Val::Percent(100.0)),
..default()
},
background_color: Color::rgb(0.15, 0.15, 0.15).into(),
Expand Down Expand Up @@ -77,7 +77,8 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
style: Style {
flex_direction: FlexDirection::Column,
justify_content: JustifyContent::Center,
size: Size::new(Val::Px(200.0), Val::Percent(100.0)),
align_items: AlignItems::Center,
size: Size::width(Val::Px(200.0)),
..default()
},
background_color: Color::rgb(0.15, 0.15, 0.15).into(),
Expand All @@ -95,12 +96,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
},
)
.with_style(Style {
size: Size::new(Val::Undefined, Val::Px(25.)),
margin: UiRect {
left: Val::Auto,
right: Val::Auto,
..default()
},
size: Size::height(Val::Px(25.)),
..default()
}),
);
Expand All @@ -109,8 +105,8 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
.spawn(NodeBundle {
style: Style {
flex_direction: FlexDirection::Column,
align_self: AlignSelf::Center,
size: Size::new(Val::Percent(100.0), Val::Percent(50.0)),
align_self: AlignSelf::Stretch,
size: Size::height(Val::Percent(50.0)),
overflow: Overflow::Hidden,
..default()
},
Expand All @@ -126,6 +122,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
flex_direction: FlexDirection::Column,
flex_grow: 1.0,
max_size: Size::UNDEFINED,
align_items: AlignItems::Center,
..default()
},
..default()
Expand All @@ -148,11 +145,6 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
.with_style(Style {
flex_shrink: 0.,
size: Size::new(Val::Undefined, Val::Px(20.)),
margin: UiRect {
left: Val::Auto,
right: Val::Auto,
..default()
},
..default()
}),
);
Expand Down Expand Up @@ -211,7 +203,8 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
.with_children(|parent| {
parent.spawn(NodeBundle {
style: Style {
size: Size::new(Val::Px(100.0), Val::Px(100.0)),
// Take the size of the parent node.
size: Size::all(Val::Percent(100.)),
position_type: PositionType::Absolute,
position: UiRect {
left: Val::Px(20.0),
Expand All @@ -225,7 +218,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
});
parent.spawn(NodeBundle {
style: Style {
size: Size::new(Val::Px(100.0), Val::Px(100.0)),
size: Size::all(Val::Percent(100.)),
position_type: PositionType::Absolute,
position: UiRect {
left: Val::Px(40.0),
Expand All @@ -239,7 +232,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
});
parent.spawn(NodeBundle {
style: Style {
size: Size::new(Val::Px(100.0), Val::Px(100.0)),
size: Size::all(Val::Percent(100.)),
position_type: PositionType::Absolute,
position: UiRect {
left: Val::Px(60.0),
Expand All @@ -254,7 +247,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
// alpha test
parent.spawn(NodeBundle {
style: Style {
size: Size::new(Val::Px(100.0), Val::Px(100.0)),
size: Size::all(Val::Percent(100.)),
position_type: PositionType::Absolute,
position: UiRect {
left: Val::Px(80.0),
Expand All @@ -272,7 +265,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
parent
.spawn(NodeBundle {
style: Style {
size: Size::new(Val::Percent(100.0), Val::Percent(100.0)),
size: Size::width(Val::Percent(100.)),
position_type: PositionType::Absolute,
justify_content: JustifyContent::Center,
align_items: AlignItems::FlexStart,
Expand All @@ -284,7 +277,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
// bevy logo (image)
parent.spawn(ImageBundle {
style: Style {
size: Size::new(Val::Px(500.0), Val::Auto),
size: Size::width(Val::Px(500.0)),
..default()
},
image: asset_server.load("branding/bevy_logo_dark_big.png").into(),
Expand Down
2 changes: 1 addition & 1 deletion examples/ui/z_index.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ fn setup(mut commands: Commands) {
commands
.spawn(NodeBundle {
style: Style {
size: Size::new(Val::Percent(100.0), Val::Percent(100.0)),
size: Size::width(Val::Percent(100.0)),
align_items: AlignItems::Center,
justify_content: JustifyContent::Center,
..default()
Expand Down

0 comments on commit eaac730

Please sign in to comment.