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

[Merged by Bors] - Partially document bevy_ui #3526

Closed
wants to merge 16 commits into from

Conversation

Sheepyhead
Copy link
Contributor

Objective

Updated the docs for bevy_ui as requested by #3492

Solution

I have documented the parts I understand. anchors.rs is not in use and should be removed, thus I haven't documented that, and some of the more renderer-heavy code is beyond me and needs input from either cart or someone familiar with bevy rendering

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Jan 2, 2022
@Sheepyhead Sheepyhead changed the title Doc/bevy UI Partially document bevy_ui Jan 2, 2022
crates/bevy_ui/src/ui_node.rs Outdated Show resolved Hide resolved
Auto,
/// If the parent has [`AlignItems::Center`] only this item will be at the start
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you mentioning Center in particular?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The descriptions are heavily inspired by https://cssreference.io/flexbox/#align-items, which uses that as an example to underline how this causes the item to diverge from the parent's align-items property

crates/bevy_ui/src/ui_node.rs Outdated Show resolved Hide resolved
crates/bevy_ui/src/ui_node.rs Outdated Show resolved Hide resolved
Relative,
/// Independent of all other nodes
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's only independent from siblings, but still relative to it's parent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PositionType::Relative actually makes it influenced by seemingly unrelated other nodes if they also have PositionType::Relative, but I've never actually set absolute on a node with a parent, so I'm not sure what happens when you do that 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

IIUC, with PositionType::Absolute, it stops participating in the layout of its parent and siblings, and sets it's local transform and node size according to style.position.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It also stops being influenced by unrelated nodes with relative positioning, I was struggling with layouting because stuff was unexpectedly interacting with each other without being related

Copy link
Contributor

Choose a reason for hiding this comment

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

stuff was unexpectedly interacting with each other without being related

Could you give more details about that?
To be clear, this is what I meant:

  • if style.position_type == PositionType::Relative, the node takes part in it's parent/siblings' flexbox layout, which sets it's default position (and "interacts" with their default position)
  • if style.position_type == PositionType::Absolute, the node does not take part in it's parent/siblings' layout, and it's default position is the position of it's parent (that is, it's local transform is 0)
  • style.position displaces the node from its default position after the layout algo is finished. it doesn't "push" other nodes, regardless of the node's position_type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I mean is that nodes with a relative positioning also influence other nodes with relative positioning that aren't in the same hierarchy at all - this is a counterintuitive issue I had to wrangle for one of my games

Copy link
Contributor

Choose a reason for hiding this comment

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

That's because there is effectively only one layout hierarchy. All the "root" UI nodes are children of a "window" node (which only exists during layout).

@alice-i-cecile alice-i-cecile added A-UI Graphical user interfaces, styles, layouts, and widgets C-Docs An addition or correction to our documentation and removed S-Needs-Triage This issue needs to be labelled labels Jan 4, 2022
bors bot pushed a commit that referenced this pull request Jan 5, 2022
# Objective

- As noticed by @Sheepyhead in #3526, `anchor.rs` is completely unused.

## Solution

- Anchors away!
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Looks good to me. The exact details of the UI positioning aren't super critical at this stage: this is a very important improvement and the fine behavior of layout is likely to get broken in the near future one way or another.

@Sheepyhead
Copy link
Contributor Author

Alright I'll just fix the merge conflicts then

Copy link
Contributor

@killercup killercup left a comment

Choose a reason for hiding this comment

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

Good work! I added some formatting suggestions.

It would be really neat to have some examples here too, but that is a PR in itself I guess.

crates/bevy_ui/src/focus.rs Outdated Show resolved Hide resolved
crates/bevy_ui/src/ui_node.rs Outdated Show resolved Hide resolved
crates/bevy_ui/src/ui_node.rs Outdated Show resolved Hide resolved
crates/bevy_ui/src/ui_node.rs Outdated Show resolved Hide resolved
@Sheepyhead
Copy link
Contributor Author

Good work! I added some formatting suggestions.

It would be really neat to have some examples here too, but that is a PR in itself I guess.

Yeah this pr is only concerned with rustdoc

@killercup
Copy link
Contributor

Yeah this pr is only concerned with rustdoc

I meant example code blocks in the API docs to demonstrate how everything is used together. But yes, not here!

@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Jan 7, 2022
# Objective

Updated the docs for bevy_ui as requested by #3492 

## Solution

I have documented the parts I understand. anchors.rs is not in use and should be removed, thus I haven't documented that, and some of the more renderer-heavy code is beyond me and needs input from either cart or someone familiar with bevy rendering

Co-authored-by: Troels Jessen <[email protected]>
@bors bors bot changed the title Partially document bevy_ui [Merged by Bors] - Partially document bevy_ui Jan 7, 2022
@bors bors bot closed this Jan 7, 2022
@Sheepyhead Sheepyhead deleted the doc/bevy_ui branch January 7, 2022 23:05
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-Docs An addition or correction to our documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants