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

Added `get_size` to `Text` and renamed the old `size` to `scale` #41

Merged
merged 2 commits into from Dec 17, 2018

Conversation

@VictorKoenders
Copy link
Contributor

VictorKoenders commented Dec 17, 2018

I wanted to add a method get_size to Text, and figured that the old set_size actually sets the scale, so I made a breaking change and renamed that. That might make this a better PR for the 0.2 branch.

I've also moved the clear the internal quads logic to the invalidate method.

This could be made faster if we make the assumptions that new_quads[i].x2 > new_quads[i].x1 but I wasn't sure if that was the case.

@17cupsofcoffee

This comment has been minimized.

Copy link
Owner

17cupsofcoffee commented Dec 17, 2018

This seems handy, definitely would be happy to have this functionality added :)

I'm not sure if 'size' is the right name for this, though - when I hear size in the context of text, my mind jumps to font size (possibly because that's the terminology I've seen in other libraries I've used). What do you think about get_bounds? Apologies for the bikeshedding :)

@VictorKoenders

This comment has been minimized.

Copy link
Contributor Author

VictorKoenders commented Dec 17, 2018

Yeah get_bounds or get_outer_bounds would probably be more descriptive

@VictorKoenders

This comment has been minimized.

Copy link
Contributor Author

VictorKoenders commented Dec 17, 2018

Do you want me to rename the set_scale back to set_size?

@17cupsofcoffee

This comment has been minimized.

Copy link
Owner

17cupsofcoffee commented Dec 17, 2018

Do you want me to rename the set_scale back to set_size?

Yes please :)

@17cupsofcoffee 17cupsofcoffee merged commit 7d8cad7 into 17cupsofcoffee:master Dec 17, 2018
@17cupsofcoffee

This comment has been minimized.

Copy link
Owner

17cupsofcoffee commented Dec 17, 2018

Merged! Thank you again for the contribution (and the speedy responses to my nitpicking 😄)!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.