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

Add fieldHeight prop for FlxText to control textfield height #2789

Merged
merged 9 commits into from
Jun 5, 2023

Conversation

T1mL3arn
Copy link
Contributor

@T1mL3arn T1mL3arn commented May 7, 2023

closes #2783

A fix with backward compatibility. Though I would add fieldHeight to the constructor. Also I made sort of a demo with tests, if you want it I will clean up and post here.

The deleted assignments actually happen in deep down of `makeGraphic()`. And it is not documented, so it should be fixed too.
- Fix hitbox and field size difference for fixed-size text.
- Remove magic const `1.2`: rendered field height does not depend on it,
and new field height has no effect since the height is calculated from `textHeight` and not from `height`
@Geokureli
Copy link
Member

Sorry for taking a while to get to this, hopefully soon

@Geokureli
Copy link
Member

@T1mL3arn you said you had a demo, can you share it?

@T1mL3arn
Copy link
Contributor Author

T1mL3arn commented May 31, 2023

https://github.com/T1mL3arn/Flixel-text-height-demo

update:

Note there are small inconsistencies between flixel and openfl (for neko and hl targets), but I believe they came from openfl itself.

Copy link
Member

@Geokureli Geokureli left a comment

Choose a reason for hiding this comment

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

mostly needs doc and organizational changes, otherwise seems good

flixel/text/FlxText.hx Outdated Show resolved Hide resolved
flixel/text/FlxText.hx Outdated Show resolved Hide resolved
flixel/text/FlxText.hx Outdated Show resolved Hide resolved
flixel/text/FlxText.hx Outdated Show resolved Hide resolved
@Geokureli Geokureli added this to the 5.4.0 milestone Jun 1, 2023
@T1mL3arn
Copy link
Contributor Author

T1mL3arn commented Jun 4, 2023

What do you think about adding fieldHeight to the constructor?

@Geokureli
Copy link
Member

What do you think about adding fieldHeight to the constructor?

That would be a breaking change, unfortunately, also it's not very commonly used, tbh

@Geokureli Geokureli merged commit d37742b into HaxeFlixel:dev Jun 5, 2023
15 checks passed
@Geokureli
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FlxText always shrinks its "graphic object" height
2 participants