Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Merged by Bors] - Remove auto-margin properties from the examples #6535

Closed
wants to merge 1 commit into from
Closed

[Merged by Bors] - Remove auto-margin properties from the examples #6535

wants to merge 1 commit into from

Conversation

garychia
Copy link
Contributor

Objective

Fixes #6498.

Solution

Adding a parent node with properties AlignItems::Center and JustifyContent::Center to centered child nodes and removing their auto-margin properties.

@alice-i-cecile alice-i-cecile added A-UI Graphical user interfaces, styles, layouts, and widgets C-Code-Quality A section of code that is hard to understand or change labels Nov 10, 2022
@alice-i-cecile alice-i-cecile added the C-Examples An addition or correction to our examples label Nov 10, 2022
@alice-i-cecile
Copy link
Member

Awesome, thank you very much!

TimJentzsch
TimJentzsch approved these changes Nov 10, 2022
View reviewed changes
Copy link
Contributor

@TimJentzsch TimJentzsch left a comment

Choose a reason for hiding this comment

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

Definitely a bit more verbose, but this should be the correct way to do this.

Maybe we can introduce some helper methods in the future that make common stuff like this easier.

@Weibye Weibye mentioned this pull request Nov 10, 2022
Closed
Weibye
Weibye approved these changes Nov 11, 2022
View reviewed changes
Copy link
Contributor

@Weibye Weibye left a comment

Choose a reason for hiding this comment

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

Ran all relevant examples and verified that they look the same 👍

@Weibye Weibye 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 Nov 11, 2022
@alice-i-cecile alice-i-cecile mentioned this pull request Nov 21, 2022
Closed
@alice-i-cecile
Copy link
Member

@garychia hit merge conflicts, sorry! Can you resolve these?

@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Nov 21, 2022
@garychia
Remove auto-margin properties from the examples (#6535)
585dac0 
# Objective

Fixes #6498.

## Solution

Adding a parent node with properties AlignItems::Center and JustifyContent::Center to centered child nodes and removing their auto-margin properties.
@bors
Copy link
Contributor

bors bot commented Nov 21, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Remove auto-margin properties from the examples [Merged by Bors] - Remove auto-margin properties from the examples Nov 21, 2022
@bors bors bot closed this Nov 21, 2022
@garychia garychia deleted the remove-auto-margin branch November 25, 2022 02:24
taiyoungjang pushed a commit to taiyoungjang/bevy that referenced this pull request Dec 15, 2022
@garychia @taiyoungjang
Remove auto-margin properties from the examples (bevyengine#6535)
b5eb147 
# Objective

Fixes bevyengine#6498.

## Solution

Adding a parent node with properties AlignItems::Center and JustifyContent::Center to centered child nodes and removing their auto-margin properties.
alradish pushed a commit to alradish/bevy that referenced this pull request Jan 22, 2023
@garychia @alradish
Remove auto-margin properties from the examples (bevyengine#6535)
27447db 
# Objective

Fixes bevyengine#6498.

## Solution

Adding a parent node with properties AlignItems::Center and JustifyContent::Center to centered child nodes and removing their auto-margin properties.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
@garychia @ItsDoot
Remove auto-margin properties from the examples (bevyengine#6535)
b2e1081 
# Objective

Fixes bevyengine#6498.

## Solution

Adding a parent node with properties AlignItems::Center and JustifyContent::Center to centered child nodes and removing their auto-margin properties.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TimJentzsch TimJentzsch TimJentzsch approved these changes

@Weibye Weibye Weibye approved these changes

Assignees
No one assigned
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Code-Quality A section of code that is hard to understand or change C-Examples An addition or correction to our examples 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Stop using auto-margin to center UI elements in our examples
4 participants
@garychia @alice-i-cecile @Weibye @TimJentzsch