UIWrappedText: Fix single-character strings not being rendered #142
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
By default, UIWrappedText will set its width to the width of the text string. When it is rendering the text, it'll check if the width of the component is less than or equal to the width of a character (in this case 'x'), this is flawed for a few reasons: 1. Different characters have different widths 2. The comment "if we are smaller than a character" doesn't match up with the behavior (less than or equal to) 3. `UGraphics.getCharWidth` (used to get the width of 'x') does not take the `fontProvider` into account, meaning that the width that it calculates for that character could be wrong. To solve this, the check has been removed, as it is flawed and doesn't seem to have an explicit error, contrary to what the comment in the code said. The only problem found with this was that `getStringSplitToWidth` would push an extra line onto the list if the container wasn't long enough to render one character. This has been fixed by adding a `isNotEmpty` check before pushing the line (that's okay to do in this case, as it's not a newline check). The only "breaking change" about this, is that if your container is less than "a character" wide, they will still render. If you for some reason want to avoid this, put a `ScissorEffect` onto your `UIWrappedText`, but this shouldn't really be necessary.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
By default, UIWrappedText will set its width to the width of the text string. When it is rendering the text, it'll check if the width of the component is less than or equal to the width of a character (in this case 'x'), this is flawed for a few reasons:
UGraphics.getCharWidth(used to get the width of 'x') does not take thefontProviderinto account, meaning that the width that it calculates for that character could be wrong.To solve this, the check has been removed, as it is flawed and doesn't seem to have an explicit error, contrary to what the comment in the code said.
The only problem found with this was that
getStringSplitToWidthwould push an extra line onto the list if the container wasn't long enough to render one character. This has been fixed by adding aisNotEmptycheck before pushing the line (that's okay to do in this case, as it's not a newline check).The only "breaking change" about this, is that if your container is less than "a character" wide, they will still render. If you for some reason want to avoid this, put a
ScissorEffectonto yourUIWrappedText, but this shouldn't really be necessary.