[Merged by Bors] - Improve ergonomics and reduce boilerplate around creating text elements. #5343

tim-blackbird · 2022-07-16T20:20:49Z

Objective

Creating UI elements is very boilerplate-y with lots of indentation.
This PR aims to reduce boilerplate around creating text elements.

Changelog

Renamed Text::with_section to from_section.
It no longer takes a TextAlignment as argument, as the vast majority of cases left it Default::default().
Added Text::from_sections which creates a Text from a list of TextSections.
Reduces line-count and reduces indentation by one level.
Added Text::with_alignment.
A builder style method for setting the TextAlignment of a Text.
Added TextSection::new.
Does not reduce line count, but reduces character count and made it easier to read. No more .to_string() calls!
Added TextSection::from_style which creates an empty TextSection with a style.
No more empty strings! Reduces indentation.
Added TextAlignment::CENTER and friends.
Added methods to TextBundle. from_section, from_sections, with_text_alignment and with_style.

Note for reviewers.

Because of the nature of these changes I recommend setting diff view to 'split'.
~~Look for the book icon~~ cog in the top-left of the Files changed tab.

Have fun reviewing ❤️
^>:D

Migration Guide

Text::with_section was renamed to from_section and no longer takes a TextAlignment as argument.
Use with_alignment to set the alignment instead.

alice-i-cecile · 2022-07-16T20:22:51Z

The other related change I really want is a helper method for quickly getting the first section of text, both immutably and mutably. Maybe first and first_mut?

crates/bevy_text/src/text.rs

alice-i-cecile · 2022-07-16T20:27:52Z

Ping @plof27 for review :)

alice-i-cecile · 2022-07-16T20:59:18Z

@Weibye @TimJentzsch, can I get your opinions on this?

TimJentzsch

To be honest, I haven't really used Bevy UI yet so please take this with a grain of salt. But my initial thoughts:

The diffs look nice, definitely a readability improvement. I personally really don't like all the ..default() stuff, so seeing less of that is a plus for me.
Specifying the styles still looks quite annoying, but I'm not sure if we could easily improve that right now, since we don't have any kind of style inheritance AFAIK.
Not having to specify the default style in most cases is nice. However, I wonder if that would actually make a difference in a real setting? I assume that you'll want a custom design for the text in your game and apply that everywhere. So you have a default, but it's not the Default implementation that Bevy provides. I assume you'd then have to create a custom text style and then insert it for every text object you want to create? So then you'd still have to use the old syntax as far as I understand.

crates/bevy_text/src/text.rs

Weibye

I'm very much in the same situation as @TimJentzsch : Not yet having used bevy_ui myself (it's next on my list)

I really like these changes and as it does improve ergonomics around creating text sections. To me this makes sense as an intermediate improvement step, then we should revisit when we're building further abstractions like style inheritance and widgets ++.

What I would be interested in seeing is if it would be possible to further improve so that we don't have to define a font for each of the TextStyle, but default assets seems like a big problem to tackle and not something for this PR.

crates/bevy_text/src/text.rs

examples/transforms/global_vs_local_translation.rs

IceSentry

I like this! It's nothing major, but those things add up so it's nice whenever we can save a few lines or indentation here and there.

This also made me wonder. Could we have a TextBundle::from_section() and TextBundle::from_sections(). It looks like a lot of the places are just creating a TextBundle with a text and default everything else.

tim-blackbird · 2022-07-20T13:12:19Z

I added the methods to TextBundle as @IceSentry suggested. Which makes the diffs even nicer.

@alice-i-cecile I tried experimenting with some getter methods but it didn't really improve much. Mostly because if you needed it text.first() isn't much better than text.sections.first(), and would involve a lot of code duplication.

I do have some questionable_o: API ideas but those will go in another PR.
This is ~~hard~~ obnoxious enough to review as is.

_{I really have to stop updating every example in the API PRs I make. I should just update one or two, Sorry reviewers :c}

alice-i-cecile

These new methods need a pass for const-ability.

Everything else looks good to me!

alice-i-cecile · 2022-07-20T13:59:46Z

bors r+

IceSentry · 2022-07-20T14:01:57Z

That new diff looks great!

…ts. (#5343) # Objective Creating UI elements is very boilerplate-y with lots of indentation. This PR aims to reduce boilerplate around creating text elements. ## Changelog * Renamed `Text::with_section` to `from_section`. It no longer takes a `TextAlignment` as argument, as the vast majority of cases left it `Default::default()`. * Added `Text::from_sections` which creates a `Text` from a list of `TextSections`. Reduces line-count and reduces indentation by one level. * Added `Text::with_alignment`. A builder style method for setting the `TextAlignment` of a `Text`. * Added `TextSection::new`. Does not reduce line count, but reduces character count and made it easier to read. No more `.to_string()` calls! * Added `TextSection::from_style` which creates an empty `TextSection` with a style. No more empty strings! Reduces indentation. * Added `TextAlignment::CENTER` and friends. * Added methods to `TextBundle`. `from_section`, `from_sections`, `with_text_alignment` and `with_style`. ## Note for reviewers. Because of the nature of these changes I recommend setting diff view to 'split'. ~~Look for the book icon~~ cog in the top-left of the Files changed tab. Have fun reviewing ❤️ <sup> >:D </sup> ## Migration Guide `Text::with_section` was renamed to `from_section` and no longer takes a `TextAlignment` as argument. Use `with_alignment` to set the alignment instead. Co-authored-by: devil-ira <justthecooldude@gmail.com>

bors · 2022-07-20T14:33:04Z

Pull request successfully merged into main.

Build succeeded:

…ts. (bevyengine#5343) # Objective Creating UI elements is very boilerplate-y with lots of indentation. This PR aims to reduce boilerplate around creating text elements. ## Changelog * Renamed `Text::with_section` to `from_section`. It no longer takes a `TextAlignment` as argument, as the vast majority of cases left it `Default::default()`. * Added `Text::from_sections` which creates a `Text` from a list of `TextSections`. Reduces line-count and reduces indentation by one level. * Added `Text::with_alignment`. A builder style method for setting the `TextAlignment` of a `Text`. * Added `TextSection::new`. Does not reduce line count, but reduces character count and made it easier to read. No more `.to_string()` calls! * Added `TextSection::from_style` which creates an empty `TextSection` with a style. No more empty strings! Reduces indentation. * Added `TextAlignment::CENTER` and friends. * Added methods to `TextBundle`. `from_section`, `from_sections`, `with_text_alignment` and `with_style`. ## Note for reviewers. Because of the nature of these changes I recommend setting diff view to 'split'. ~~Look for the book icon~~ cog in the top-left of the Files changed tab. Have fun reviewing ❤️ <sup> >:D </sup> ## Migration Guide `Text::with_section` was renamed to `from_section` and no longer takes a `TextAlignment` as argument. Use `with_alignment` to set the alignment instead. Co-authored-by: devil-ira <justthecooldude@gmail.com>

tim-blackbird added 3 commits July 16, 2022 20:24

Do the thing.

68d0655

Add Text::from_sections.

5932baa

Add TextSection::new.

aec9e54

alice-i-cecile requested a review from mockersf July 16, 2022 20:21

alice-i-cecile added A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A simple quality-of-life change that makes Bevy easier to use labels Jul 16, 2022

alice-i-cecile reviewed Jul 16, 2022

View reviewed changes

crates/bevy_text/src/text.rs Show resolved Hide resolved

tim-blackbird added 3 commits July 16, 2022 22:41

Add doc.

47ded0a

More constants.

3a66e12

Remove format! calls.

b13f480

alice-i-cecile requested a review from sixfold-origami July 16, 2022 20:58

TimJentzsch reviewed Jul 16, 2022

View reviewed changes

crates/bevy_text/src/text.rs Show resolved Hide resolved

Weibye approved these changes Jul 17, 2022

View reviewed changes

crates/bevy_text/src/text.rs Show resolved Hide resolved

Cleanup.

3c83943

IceSentry reviewed Jul 20, 2022

View reviewed changes

examples/transforms/global_vs_local_translation.rs Show resolved Hide resolved

IceSentry approved these changes Jul 20, 2022

View reviewed changes

tim-blackbird added 4 commits July 20, 2022 11:42

Add builder methods to TextBundle.

fa53917

Cleanup. Misc changes.

deceb79

Update game_menu example.

7f4affd

Update examples.

5386c1f

Fix comment.

f37877b

alice-i-cecile requested changes Jul 20, 2022

View reviewed changes

const fn where possible.

f6ba159

alice-i-cecile approved these changes Jul 20, 2022

View reviewed changes

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 Jul 20, 2022

bors bot changed the title ~~Improve ergonomics and reduce boilerplate around creating text elements.~~ [Merged by Bors] - Improve ergonomics and reduce boilerplate around creating text elements. Jul 20, 2022

bors bot closed this Jul 20, 2022

alice-i-cecile mentioned this pull request Jul 20, 2022

User UI camera control #5252

Closed

tim-blackbird deleted the text-ergo branch October 21, 2022 15:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Improve ergonomics and reduce boilerplate around creating text elements. #5343

[Merged by Bors] - Improve ergonomics and reduce boilerplate around creating text elements. #5343

tim-blackbird commented Jul 16, 2022 •

edited

Loading

alice-i-cecile commented Jul 16, 2022 •

edited

Loading

alice-i-cecile commented Jul 16, 2022

alice-i-cecile commented Jul 16, 2022 •

edited

Loading

TimJentzsch left a comment •

edited

Loading

Weibye left a comment

IceSentry left a comment

tim-blackbird commented Jul 20, 2022

alice-i-cecile left a comment

alice-i-cecile commented Jul 20, 2022

IceSentry commented Jul 20, 2022

bors bot commented Jul 20, 2022

[Merged by Bors] - Improve ergonomics and reduce boilerplate around creating text elements. #5343

[Merged by Bors] - Improve ergonomics and reduce boilerplate around creating text elements. #5343

Conversation

tim-blackbird commented Jul 16, 2022 • edited Loading

Objective

Changelog

Note for reviewers.

Migration Guide

alice-i-cecile commented Jul 16, 2022 • edited Loading

alice-i-cecile commented Jul 16, 2022

alice-i-cecile commented Jul 16, 2022 • edited Loading

TimJentzsch left a comment • edited Loading

Choose a reason for hiding this comment

Weibye left a comment

Choose a reason for hiding this comment

IceSentry left a comment

Choose a reason for hiding this comment

tim-blackbird commented Jul 20, 2022

alice-i-cecile left a comment

Choose a reason for hiding this comment

alice-i-cecile commented Jul 20, 2022

IceSentry commented Jul 20, 2022

bors bot commented Jul 20, 2022

tim-blackbird commented Jul 16, 2022 •

edited

Loading

alice-i-cecile commented Jul 16, 2022 •

edited

Loading

alice-i-cecile commented Jul 16, 2022 •

edited

Loading

TimJentzsch left a comment •

edited

Loading