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.

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

UI Display and Visibility Example #7629

Merged
merged 31 commits into from
Jun 19, 2023

Conversation

ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Feb 11, 2023

Objective

An example demonstrating how Display and Visibility work in Bevy UI.

fixes #5380
related #5368

Bevy App 15_02_2023 20_40_46

Changelog

  • Added the example flex_display.rs.

* Added an example demonstrating how Display and Visibility work in the UI.
@alice-i-cecile alice-i-cecile added C-Examples An addition or correction to our examples A-UI Graphical user interfaces, styles, layouts, and widgets labels Feb 11, 2023
@ickshonpe ickshonpe changed the title UI Display and Visbility Example UI Display and Visibility Example Feb 11, 2023
Cargo.toml Outdated Show resolved Hide resolved
* split up setup function
* added instructions text to example
@github-actions
Copy link
Contributor

You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.

@rparrett
Copy link
Contributor

rparrett commented Feb 15, 2023

With all of the nodes arranged in a hierarchy as they are, aren't Display::None and Visibility::Hidden behaving pretty identically here?

Would it be more interesting to show the clickable nodes in a single parent flex container?

@ickshonpe
Copy link
Contributor Author

With all of the nodes arranged in a hierarchy as they are, aren't Display::None and Visibility::Hidden behaving pretty identically here?

Would it be more interesting to show the clickable nodes in a single parent flex container?

I'm not quite sure what you mean, the behaviour is quite different isn't it? Display::None hides the node and all its descendants, while Visibility::Hidden hides itself and only its children that are Visibility::Inherited.

@rparrett
Copy link
Contributor

I'm not quite sure what you mean, the behaviour is quite different isn't it? Display::None hides the node and all its descendants, while Visibility::Hidden hides itself and only its children that are Visibility::Inherited.

That's fair.

When I think of the difference between Display::None and Visibility::Hidden though, the first thing that comes to mind is that Visibility::Hidden doesn't affect layout, but Display::None does.

Although I'm not sure how to demonstrate both properties simultaneously.

"2" is Visibility::Hidden
+-----------+          
|+-+     +-+|          
||1|     |3||          
|+-+     +-+|          
+-----------+
          
"2" is Display::None
+-----------+          
|+-+ +-+    |          
||1| |3|    |          
|+-+ +-+    |          
+-----------+  

@ickshonpe
Copy link
Contributor Author

ickshonpe commented Feb 15, 2023

Oh I get it, you are right. It needs to demonstrate how Display::None collapses the layout too, yes.

…o that it collapses when `Display::None` is set on its child.
@ickshonpe
Copy link
Contributor Author

display_collapse

Should be able to do something like that with the inner nodes too somehow.

@ickshonpe
Copy link
Contributor Author

ickshonpe commented Feb 15, 2023

Oh I know I can stuff each node with a second invisible panel.

display_collapse_2

collapse_last

That's much better.

Copy link

@omwda omwda left a comment

Choose a reason for hiding this comment

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

Looking good :)

@james7132 james7132 added this to the 0.11 milestone Mar 4, 2023
Copy link
Contributor

@doup doup left a comment

Choose a reason for hiding this comment

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

Approved with a couple of nits that I think are worth adding… but, up to you.
Nice example 👍

examples/ui/display_and_visibility.rs Show resolved Hide resolved
examples/ui/display_and_visibility.rs Outdated Show resolved Hide resolved
@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 Jun 19, 2023
@alice-i-cecile
Copy link
Member

@ickshonpe can you fix the conflicts and make a call on the nits? I think they're an improvement, but I won't block on them.

@ickshonpe
Copy link
Contributor Author

@ickshonpe can you fix the conflicts and make a call on the nits? I think they're an improvement, but I won't block on them.

Yep I'll take a look

@github-actions
Copy link
Contributor

You added a new example but didn't update the readme. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

1 similar comment
@github-actions
Copy link
Contributor

You added a new example but didn't update the readme. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

@github-actions
Copy link
Contributor

You added a new example but didn't update the readme. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

1 similar comment
@github-actions
Copy link
Contributor

You added a new example but didn't update the readme. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

… is red when properties set to `Display::None` or `Visibility::Hidden`.
@github-actions
Copy link
Contributor

You added a new example but didn't update the readme. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 19, 2023
@alice-i-cecile alice-i-cecile removed this pull request from the merge queue due to a manual request Jun 19, 2023
@ickshonpe
Copy link
Contributor Author

Everything resolved, should be ready as long as its passes CI checks.

@alice-i-cecile alice-i-cecile enabled auto-merge June 19, 2023 23:05
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 19, 2023
Merged via the queue into bevyengine:main with commit 50c50cd Jun 19, 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-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
Development

Successfully merging this pull request may close these issues.

Create a UI example demonstrating the difference between node Visibility::is_visible and Display::None
7 participants