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

UI node bundle comment fix #9404

Merged
merged 5 commits into from
Aug 28, 2023
Merged

UI node bundle comment fix #9404

merged 5 commits into from
Aug 28, 2023

Conversation

ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Aug 10, 2023 •
edited
Loading

Objective

Doc comment for the global_transform field in NodeBundle says:

/// This field is automatically managed by the UI layout system.

The GlobalTransform component is the thing being managed, not the global_transform field, and the TransformPropagate systems do the managing, not the UI layout system.

@ickshonpe ickshonpe changed the title UI node bundle comment correction UI node bundle comment fix Aug 10, 2023
@nicopap nicopap added C-Docs An addition or correction to our documentation A-UI Graphical user interfaces, styles, layouts, and widgets labels Aug 10, 2023
rparrett
rparrett approved these changes Aug 10, 2023
View reviewed changes
Copy link
Contributor

@rparrett rparrett left a comment

Choose a reason for hiding this comment

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

Checked the rendered docs. Looks good / makes sense to me.

While reviewing I noticed that the docs for the transform fields for these various bundles all make references to NodeBundle.

@hymm hymm 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 Aug 10, 2023
Weibye
Weibye approved these changes Aug 11, 2023
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.

I agree this is more correct 👍

@ickshonpe
Copy link
Contributor Author

Checked the rendered docs. Looks good / makes sense to me.

While reviewing I noticed that the docs for the transform fields for these various bundles all make references to NodeBundle.

Oh yep I see, better fix that too.

@ickshonpe
Copy link
Contributor Author

ickshonpe commented Aug 11, 2023 •
edited
Loading

/// To alter the position of the `NodeBundle`, use the properties of the [`Style`] component.

Changed this line for the Transform comments so it uses the name of the specific bundle.
For GlobalTransform I deleted it, as it seems odd there when GlobalTransform isn't used for mutations.

@ickshonpe
Changed NodeBundle to the name of the specific bundle in the `trans…
d65921f 
…form` field doc comment.

Deleted the same line for `GlobalTransform` as it doesn't really make sense there.
@ickshonpe
Merge branch 'ui-node-bundle-comment-correction' of https://github.co…
3b9698a 
…m/ickshonpe/bevy into ui-node-bundle-comment-correction
rparrett
rparrett reviewed Aug 11, 2023
View reviewed changes
crates/bevy_ui/src/node_bundles.rs Outdated
@@ -75,7 +75,7 @@ impl Default for NodeBundle {
pub struct ImageBundle {
/// Describes the logical size of the node
///
/// This field is automatically managed by the UI layout system.
/// This component is automatically managed by the UI layout system.
/// To alter the position of the `NodeBundle`, use the properties of the [`Style`] component.
Copy link
Contributor

@rparrett rparrett Aug 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

There's also a copy of this "NodeBundle" comment here, associated with the node field, with the same copy/paste mistake.

Seems safe to get rid of this one too to me. Not every bundle has it. But maybe it was intended that it say "to alter the size"?

@alice-i-cecile alice-i-cecile mentioned this pull request Aug 14, 2023
Closed
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Aug 14, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 14, 2023
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Aug 14, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 14, 2023
@alice-i-cecile
Copy link
Member

@ickshonpe once merge conflicts are cleaned up, I'll merge this.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Aug 28, 2023
Merged via the queue into bevyengine:main with commit 05b7f60 Aug 28, 2023
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
@ickshonpe
UI node bundle comment fix (bevyengine#9404)
1e1a9c7 
# Objective

Doc comment for the `global_transform` field in `NodeBundle` says:

```
/// This field is automatically managed by the UI layout system.
```

The `GlobalTransform` component is the thing being managed, not the
`global_transform` field, and the `TransformPropagate` systems do the
managing, not the UI layout system.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rparrett rparrett rparrett approved these changes

@djeedai djeedai djeedai approved these changes

@mockersf mockersf mockersf approved these changes

@Weibye Weibye Weibye approved these changes

@nicopap nicopap nicopap approved these changes

Assignees
No one assigned
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Docs An addition or correction to our documentation 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.
8 participants
@ickshonpe @alice-i-cecile @rparrett @djeedai @mockersf @Weibye @nicopap @hymm