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.

Sign up for GitHub

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

Jump to bottom

Fix View translations in Android #3954

Closed
wants to merge 1 commit into from
Closed

Fix View translations in Android #3954

wants to merge 1 commit into from

Conversation

dgladkov
Copy link
Contributor

@dgladkov dgladkov commented Nov 6, 2015

Fixes regression in bf59864

Closes #3773

@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @kmagiera to be a potential reviewer.

@facebook-github-bot
Copy link
Contributor

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 [email protected]. Thanks!

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 6, 2015
@dgladkov dgladkov changed the title Fix translations in Android Fix View translations in Android Nov 6, 2015
@dgladkov
Copy link
Contributor Author

dgladkov commented Nov 6, 2015

Also closes #3669

@ide
Copy link
Contributor

ide commented Nov 6, 2015

Nice catch!

@ide ide added the Android label Nov 6, 2015
kmagiera
kmagiera reviewed Nov 9, 2015
View reviewed changes
ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java
@@ -115,13 +115,13 @@ public void setScaleY(T view, float scaleY) {
@Deprecated
@ReactProp(name = PROP_TRANSLATE_X, defaultFloat = 1f)
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this was copy-paste kind of an issue from setScale. Also defaultFloat should be 0f here instead of 1f. Do you mind updating defaultFloat here (and in translateY) too to completely fix translate property related regression introduced by refactoring? Once it's done I'll be happy to merge your patch

@kmagiera
Copy link
Contributor

kmagiera commented Nov 9, 2015

ok, nevermind. It'd be better to merge it asap. I'll update default on my own. Thanks for your patch

@facebook-github-bot shipit

@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/1734939810071328/int_phab to review.

@kmagiera kmagiera mentioned this pull request Nov 9, 2015
Closed
@ghost ghost closed this in b12117a Nov 9, 2015
@PhilippKrone
Copy link
Contributor

@kmagiera Is this connected to #3557 as well? I was trying to apply this PR to my local repo, but without luck until now.

@kmagiera
Copy link
Contributor

kmagiera commented Nov 9, 2015

@PhilippKrone no, I'm 99% sure this won't fix #3557

MattFoley pushed a commit to skillz/react-native that referenced this pull request Nov 9, 2015
@dgladkov
Fix View translations in Android
98098ae 
Summary: Fixes regression in bf59864

Closes facebook#3773
Closes facebook#3954

Reviewed By: svcscm

Differential Revision: D2631180

Pulled By: kmagiera

fb-gh-sync-id: 09a1a2e48fd6fff37d1491c120a28221cdc1b163
ide pushed a commit to expo/react-native that referenced this pull request Nov 13, 2015
@dgladkov @ide
Fix View translations in Android
f4a37f1 
Summary: Fixes regression in bf59864

Closes facebook#3773
Closes facebook#3954

Reviewed By: svcscm

Differential Revision: D2631180

Pulled By: kmagiera

fb-gh-sync-id: 09a1a2e48fd6fff37d1491c120a28221cdc1b163
@ramilushev
Copy link

@kmagiera Is there a reason this was not included in 0.15.0? The transitions for Android are still broken.

@fender
Copy link

fender commented Nov 25, 2015

@6i6arka I was wondering the same thing.

@kmagiera
Copy link
Contributor

@fender we did a branch cut for 0.15 on Nov 6 (f7af7d2) and this was merged on Nov 9. It made it to 0.16-rc though so should be out next week I believe (cc @mkonicek).

You can find some information about release process for RN here: https://facebook.github.io/react/blog/2015/05/22/react-native-release-process.html
Also If you believe that some of the fixes you see as PRs or commits should be cherry-picked to the RC branch just let us know.

@ldn0x7dc ldn0x7dc mentioned this pull request Dec 1, 2015
Closed
@fender
Copy link

fender commented Dec 2, 2015

@kmagiera Ok, great. Should we then be able to close this issue? #4132

@kmagiera
Copy link
Contributor

kmagiera commented Dec 2, 2015

I believe #4132 should be fixed as of b12117a, would like to have the issue author to verify that first before closing

@kmagiera kmagiera mentioned this pull request Dec 2, 2015
Closed
@brentvatne brentvatne mentioned this pull request Dec 5, 2015
Closed
Crash-- pushed a commit to Crash--/react-native that referenced this pull request Dec 24, 2015
@dgladkov @Crash--
Fix View translations in Android
e299994 
Summary: Fixes regression in bf59864

Closes facebook#3773
Closes facebook#3954

Reviewed By: svcscm

Differential Revision: D2631180

Pulled By: kmagiera

fb-gh-sync-id: 09a1a2e48fd6fff37d1491c120a28221cdc1b163
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@dgladkov @facebook-github-bot @ide @kmagiera @PhilippKrone @ramilushev @fender