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

Android: Make lineHeight accept decimal values #13843

Closed
wants to merge 2 commits into from

Conversation

jonaslu
Copy link
Contributor

@jonaslu jonaslu commented May 8, 2017

Make android-version accept a decimal
number as lineHeight.

Credits where due, solution was given in this
issue: #10607

Motivation (required)

According to the w3 spec the property
line-height should accept decimal values
(and it does on iOS) but the android
version has the wrong data-type for the
shadowed method, resulting in a stacktrace
saying:
com.facebook.react.bridge.UnexpectedNativeTypeException: TypeError:
expected dynamic type int64', but had type double'

Setting it to a float makes it accept
decmial values as it should.

Test Plan (required)

  • Create an app without this commit and create the same app with this commit:
    In both apps:
  • Leave line-height undefined. Behavior is unaffected by this commit.
  • Set lineHeight to a integer number. Behavior is unaffected by this commit.
  • Set lineHeight to a decimal number. Line height is now rendered with decimals in the app with this fix.
  • Run android integration tests to see nothing
    else broke.

Next Steps

Sign the CLA, if you haven't already.

Small pull requests are much easier to review and more likely to get merged. Make sure the PR does only one thing, otherwise please split it.

Make sure all tests pass on both Travis and Circle CI. PRs that break tests are unlikely to be merged.

For more info, see the "Pull Requests" section of our "Contributing" guidelines.

Make android-version accept a decimal
number as lineHeight.

According to the spec the property
line-height should accept decimal values
(and it does on iOS) but the android
version has the wrong data-type for the
shadowed method, resulting in a stacktrace
saying:
com.facebook.react.bridge.UnexpectedNativeTypeException: TypeError:
expected dynamic type `int64', but had type `double'

Setting it to a float makes it accept
decmial values as it should.

Credits where due, solution was given in this
issue: facebook#10607

Verification:
* Create an app without this fix:
* Create the same app with this fix:
- Leave line-height undefined. Behaviour
    is same as before.
- Set lineHeight to a integer number:
    Behaviour is same as before.
- Set lineHeight to a decimal number.
    Line height is now rendered with decimals.
* Run android integration tests to see nothing
  else broke.
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. GH Review: review-needed labels May 8, 2017
@facebook-github-bot facebook-github-bot added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels May 31, 2017
@facebook-github-bot
Copy link
Contributor

@shergin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Import Started This pull request has been imported. This does not imply the PR has been approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants