Skip to content

[Merged by Bors] - change the default width and height of Size to Val::Auto#7475

Closed
ickshonpe wants to merge 1 commit intobevyengine:mainfrom
ickshonpe:make-size-width-and-height-default-auto
Closed

[Merged by Bors] - change the default width and height of Size to Val::Auto#7475
ickshonpe wants to merge 1 commit intobevyengine:mainfrom
ickshonpe:make-size-width-and-height-default-auto

Conversation

@ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Feb 2, 2023

Objective

In CSS Flexbox width and height are auto by default, whereas in Bevy their default is Size::Undefined.
This means that, unlike in CSS, if you elide a height or width value for a node it will be given zero length (unless it has an explicitly sized child node). This has misled users into falsely assuming that they have to explicitly set a value for both height and width all the time.

relevant issue: #7120

Solution

Change the Size width and height default values to Val::Auto

Changelog

  • Changed the Size width and height default values to Val::Auto

Migration Guide

The default values for Size width and height have been changed from Val::Undefined to Val::Auto.
It's unlikely to cause any issues with existing code.

@ickshonpe ickshonpe changed the title changed Size default width and height to Val::Auto change Size default width and height to Val::Auto Feb 2, 2023
@alice-i-cecile alice-i-cecile added A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Feb 2, 2023
@alice-i-cecile alice-i-cecile added the M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Feb 2, 2023
@ickshonpe ickshonpe changed the title change Size default width and height to Val::Auto change the default width and height of Size to Val::Auto Feb 2, 2023
Copy link
Contributor

@nicoburns nicoburns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems unproblematic to me. However, I believe it will have no effect as IIRC Taffy versions that have both Undefined and Auto treat them both as Auto anyway.

@ickshonpe
Copy link
Contributor Author

ickshonpe commented Feb 2, 2023

Undefined and Auto definitely aren't the same.

Ultra minimal example, red window:

use bevy::prelude::*;
fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_startup_system(|mut commands: Commands| {
            commands.spawn(Camera2dBundle::default());
            commands.spawn(NodeBundle {
                style: Style {
                    size: Size::new(Val::Percent(100.), Val::Auto),
                    ..Default::default()
                },
                background_color: BackgroundColor(Color::RED),
                ..Default::default()
            });
        })
        .run();
}

Grey window:

use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_startup_system(|mut commands: Commands| {
            commands.spawn(Camera2dBundle::default());
            commands.spawn(NodeBundle {
                style: Style {
                    size: Size::new(Val::Percent(100.), Val::Undefined),
                    ..Default::default()
                },
                background_color: BackgroundColor(Color::RED),
                ..Default::default()
            });
        })
        .run();

The Taffy 2.x compute source is full of predicates like child_style.cross_size(constants.dir) == Dimension::Auto etc.

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Feb 3, 2023
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Feb 3, 2023
# Objective

In CSS Flexbox width and height are auto by default, whereas in Bevy their default is `Size::Undefined`.
This means that, unlike in CSS, if you elide a height or width value for a node it will be given zero length (unless it has an explicitly sized child node). This has misled users into falsely assuming that they have to explicitly set a value for both height and width all the time.

relevant issue: #7120

## Solution

Change the `Size` `width` and `height` default values to `Val::Auto`

## Changelog

* Changed the `Size` `width` and `height` default values to `Val::Auto`

## Migration Guide

The default values for `Size` `width` and `height` have been changed from `Val::Undefined` to `Val::Auto`.
It's unlikely to cause any issues with existing code.
@bors
Copy link
Contributor

bors bot commented Feb 3, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Feb 3, 2023
# Objective

In CSS Flexbox width and height are auto by default, whereas in Bevy their default is `Size::Undefined`.
This means that, unlike in CSS, if you elide a height or width value for a node it will be given zero length (unless it has an explicitly sized child node). This has misled users into falsely assuming that they have to explicitly set a value for both height and width all the time.

relevant issue: #7120

## Solution

Change the `Size` `width` and `height` default values to `Val::Auto`

## Changelog

* Changed the `Size` `width` and `height` default values to `Val::Auto`

## Migration Guide

The default values for `Size` `width` and `height` have been changed from `Val::Undefined` to `Val::Auto`.
It's unlikely to cause any issues with existing code.
@bors bors bot changed the title change the default width and height of Size to Val::Auto [Merged by Bors] - change the default width and height of Size to Val::Auto Feb 3, 2023
@bors bors bot closed this Feb 3, 2023
@ickshonpe ickshonpe mentioned this pull request Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants