Skip to content
This repository has been archived by the owner on Feb 2, 2023. It is now read-only.

[ASEditableTextNode] Round up editable text node to the next point #1761

Merged

Conversation

vadims
Copy link
Contributor

@vadims vadims commented Jun 16, 2016

Rounding up to the next device pixel was calculating a height smaller
than UITextView's contentSize. This was causing the baseline to move as
the user was typing.

Rounding up to the next device pixel was calculating a height smaller
than  UITextView's contentSize. This was causing the baseline to move as
the user was typing.
@ghost
Copy link

ghost commented Jun 16, 2016

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@appleguy appleguy changed the title Round up editable text node to the next point [ASEditableTextNode] Round up editable text node to the next point Jun 17, 2016
@appleguy
Copy link
Contributor

Solution to #1760

@appleguy
Copy link
Contributor

@vadims thanks for fixing this! I'm pretty annoyed by the name of ceilSizeValue - it is not at all clear that it does screen pixel rounding rather than points (e.g. numerical rounding).

@vadims / @maicki it would be nice to include a short comment here clarifying that it is important to use points / UITextView apparently does sizing on a whole-point basis. We should also rename the other method to more clearly describe its scale-dependent behavior.

@appleguy appleguy merged commit e550373 into facebookarchive:master Jun 17, 2016
@appleguy appleguy added this to the 1.9.9 milestone Jun 17, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants