Update text plane size following text mutation (fixes #2764). #5357
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
The plane width/height was being calculated when the text value was first set, but would not update when the text changed. Documentation has also been updated to advise using `0` rather than `auto` for autoscaling behavior. This is based on the the comment [here](aframevr#2837 (comment)) suggesting that `auto` is not a valid value for geometry (NaN is falsy, so it happened to still work).
|
Thanks for taking the time. Have been a while I touched this code. Your knowledge is more fresh. At first look I would expect the update method to be called when updating the text and this logic to change the plane size to execute: Why is not working as expected? |
|
The old code checks whether the geometry width/height is falsy, and if so, sets the geometry width/height. On the next run through update the width/height will no longer be falsy (it was set on the prior update), and so the width/height doesn't get adjusted when the text changes. |
|
Thanks for the tests! |
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.
The plane width/height was being calculated when the text value was first set, but would not update when the text changed.
Documentation has also been updated to advise using
0rather thanautofor autoscaling behavior. This is based on the the comment here suggesting thatautois not a valid value for geometry (NaN is falsy, so it happened to still work).