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

Updates default Text font size to 24px #13603

Merged
merged 5 commits into from
May 31, 2024

Conversation

mnmaita
Copy link
Member

@mnmaita mnmaita commented May 31, 2024

Objective

Solution

  • Updated the default font size value in TextStyle from 12px to 24px.
  • Resorted to Text defaults in examples to use the default font size in most of them.

Testing

  • WIP

Migration Guide

  • The default font size has been increased to 24px from 12px. Make sure you set the font to the appropriate values in places you were using Default text style.

@alice-i-cecile alice-i-cecile added A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels May 31, 2024
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.

Much nicer for the examples! I've updated the PR description a bit to make it clearer and improve the migration guide.

@mnmaita mnmaita force-pushed the mnmaita/default-font-size branch from 5c45cf4 to 24d8a70 Compare May 31, 2024 15:30
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.

What a lovely diff.

(Non-blocking) There may be a few examples where FiraMono-Medium is manually specified and doesn't need to be.

There are a few that do need it because the help text uses glyphs beyond the limited set the default font contains. But I think a few may not.

In one case, I was able to use the default after swapping out unicode em-dashes for regular dashes. But some examples need "box drawing" glyphs.

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels May 31, 2024
@mnmaita
Copy link
Member Author

mnmaita commented May 31, 2024

What a lovely diff.

(Non-blocking) There may be a few examples where FiraMono-Medium is manually specified and doesn't need to be.

There are a few that do need it because the help text uses glyphs beyond the limited set the default font contains. But I think a few may not.

In one case, I was able to use the default after swapping out unicode em-dashes for regular dashes. But some examples need "box drawing" glyphs.

I could split this up into several PRs but figured out that it was going to be too much work and then we'd be testing every single example on a different PR (or something like that). ETOOMANYEXAMPLES 😅

I'll check those examples you mentioned and reduce code a bit more where possible, I like the idea.

@alice-i-cecile
Copy link
Member

Ping me when you're done cleaning up and I'll validate it quick before merging :)

@mnmaita
Copy link
Member Author

mnmaita commented May 31, 2024

@alice-i-cecile finished with getting rid of the default font loading where appropriate. I did some extra cleanup in places where the default text color was used too.

@mnmaita mnmaita marked this pull request as ready for review May 31, 2024 16:29
@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 31, 2024
@alice-i-cecile alice-i-cecile removed the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label May 31, 2024
Merged via the queue into bevyengine:main with commit f237cf2 May 31, 2024
27 checks passed
@mnmaita mnmaita deleted the mnmaita/default-font-size branch May 31, 2024 23:24
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-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide 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.

Increase default font size
3 participants