Fix StyleSheet 'textAlign' for AndroidTextInput. Closes #2702 by hyugit · Pull Request #4481 · react/react-native

hyugit · 2015-12-01T22:36:56Z

change setTextAlign and setTextAlignVertical to receive argument of type String (the same as in StyleSheet), so that native props and stylesheet props are calling the same @ReactMethod

add demo (may not be necessary)

@ReactMethod

change `setTextAlign` and `setTextAlignVertical` to receive argument of type `String`, so that native props and stylesheet props are calling the same @ReactMethod - add demo (may not be necessary)

facebook-github-bot · 2015-12-01T22:38:27Z

By analyzing the blame information on this pull request, we identified @mkonicek, @foghina and @kmagiera to be potential reviewers.

kmagiera · 2015-12-02T07:50:09Z

you don't need a static block for this, you can just continue with this assignment on the line above

mkonicek · 2015-12-02T22:59:28Z

Thanks for the review @kmagiera!

remove textAlign and textAlignVertical from Android native props add textAlignVertical as Text Style Prop (Android only) add TEXT_ALIGN_VERTICAL as a view prop constant

facebook-github-bot · 2015-12-05T18:37:17Z

@trave7er updated the pull request.

hyugit · 2015-12-07T21:56:23Z

@kmagiera , I updated the diff based on your comments.

Some highlights of updates:

removed the use of HashMap
removed Android native props for textAlign & textAlignVertical
added textAlignVertical to TextStyleProps
added TEXT_ALIGN_VERTICAL constant to ViewProps

Let me know what you think. Thanks,

kmagiera · 2015-12-08T06:10:17Z

This looks very good. Thanks @trave7er !

I believe this should be marked as a breaking change and someone should update callsites in Fb internal apps. What needs to be done is just to move textAlign and textVerticalAlign out of view props and to the styles object cc @mkonicek

mkonicek · 2015-12-09T20:34:10Z

Adding to my TODO-list of PRs.

hyugit · 2015-12-11T02:32:41Z

was going to also update the docs, and found it's already updated. Thanks!

Let me know if you need me to update anything,

hyugit · 2015-12-17T21:13:32Z

@mkonicek @kmagiera a quick note, I named the prop "textAlignVertical" instead of "textVerticalAlign", do you want me to change it?

satya164 · 2015-12-27T09:05:12Z

@trave7er I think textAlignVertical is better.

GantMan · 2016-01-06T17:26:08Z

👍 on this one. I was able to put off the "Android Version" of an app to 2016, but I'd love soon.

christopherdro · 2016-01-10T19:31:31Z

@trave7er Can you resolve the merge conflicts so we can try and get this merged in?

Added Hyper-V Android Emulator Setup Instructions. Closes#3234

@ReactMethod

change `setTextAlign` and `setTextAlignVertical` to receive argument of type `String`, so that native props and stylesheet props are calling the same @ReactMethod - add demo (may not be necessary)

remove textAlign and textAlignVertical from Android native props add textAlignVertical as Text Style Prop (Android only) add TEXT_ALIGN_VERTICAL as a view prop constant

…tAlign

facebook-github-bot · 2016-01-10T22:58:20Z

@trave7er updated the pull request.

hyugit · 2016-01-10T23:00:19Z

@christopherdro updated

gr4yscale · 2016-01-12T10:07:24Z

just ran into this issue myself - hoping this gets merged today!

jsierles · 2016-01-12T10:11:30Z

@trave7er Could you also squash commits?

mkonicek · 2016-01-12T14:49:44Z

Actually I think shipit squashes the commits, let's try it:

@facebook-github-bot shipit

facebook-github-bot · 2016-01-12T14:50:32Z

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

mkonicek · 2016-01-12T14:51:38Z

Yup, looks squashed, landing now :)

gr4yscale · 2016-01-21T11:36:35Z

Any news on this?

mkonicek · 2016-01-21T11:54:17Z

I need to fix an internal failing test, will cherry-pick this to 0.19.

mkonicek · 2016-01-21T17:53:47Z

Merging again now. Please don't update the PR :)

mkonicek · 2016-01-22T16:58:26Z

🚀

Summary: change `setTextAlign` and `setTextAlignVertical` to receive argument of type `String` (the same as in `StyleSheet`), so that native props and stylesheet props are calling the same ReactMethod - add demo (may not be necessary) Closes#4481 Reviewed By: svcscm Differential Revision: D2823456 Pulled By: mkonicek fb-gh-sync-id: 349d17549f419b5bdc001d70b583423ade06bfe8

Fix StyleSheet 'textAlign' for AndroidTextInput
e514f70
change `setTextAlign` and `setTextAlignVertical` to receive argument of type `String`, so that native props and stylesheet props are calling the same @ReactMethod - add demo (may not be necessary)

facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Dec 1, 2015

hyugit mentioned this pull request Dec 1, 2015
Fix StyleSheet 'textAlign' for AndroidTextInput,rename Android native props #4364
Closed

kmagiera reviewed Dec 2, 2015
View reviewed changes

kmagiera added GH Review: needs-revision and removed GH Review: review-needed labels Dec 2, 2015

remove duplicate examples of textAlign prop
4281d91
remove textAlign and textAlignVertical from Android native props add textAlignVertical as Text Style Prop (Android only) add TEXT_ALIGN_VERTICAL as a view prop constant

facebook-github-bot added GH Review: review-needed and removed GH Review: needs-revision labels Dec 5, 2015

kmagiera mentioned this pull request Dec 9, 2015
[Android] Style with alignText property "right" error #2702
Closed

mkonicek added the GH Review: accepted label Dec 9, 2015

skevy added Android and removed GH Review: review-needed labels Dec 12, 2015

satya164 changed the title ~~Fix StyleSheet 'textAlign' for AndroidTextInput~~Fix StyleSheet 'textAlign' for AndroidTextInput. Closes #2702Dec 24, 2015

hyugit mentioned this pull request Jan 6, 2016
[Android] [TextInput] Error while updating property 'textAlign' of a view managed by: AndroidTextInput #3684
Closed

Merge pull request#5239from BerndWessels/master
ad16ca2
Added Hyper-V Android Emulator Setup Instructions. Closes#3234

Huang Yu added 3 commits January 10, 2016 14:49

Fix StyleSheet 'textAlign' for AndroidTextInput
a9068cd
change `setTextAlign` and `setTextAlignVertical` to receive argument of type `String`, so that native props and stylesheet props are calling the same @ReactMethod - add demo (may not be necessary)

remove duplicate examples of textAlign prop
7e26860
remove textAlign and textAlignVertical from Android native props add textAlignVertical as Text Style Prop (Android only) add TEXT_ALIGN_VERTICAL as a view prop constant

Merge branch 'textAlign' of github.com:trave7er/react-native into tex…
847747a
…tAlign

jsierles mentioned this pull request Jan 12, 2016
[0.18] Things we should cherry-pick to 0.18 #5201
Closed
11 tasks

mkonicek added the Import Failed label Jan 15, 2016

bestander mentioned this pull request Jan 21, 2016
[release-0.19] Commits to cherry pick #5462
Closed
10 tasks

ghost closed this in f453e14Jan 21, 2016

This pull request was closed.

Uh oh!

Conversation

hyugit commented Dec 1, 2015

Uh oh!

facebook-github-bot commented Dec 1, 2015

Uh oh!

kmagieraDec 2, 2015

Choose a reason for hiding this comment

Uh oh!

mkonicek commented Dec 2, 2015

Uh oh!

facebook-github-bot commented Dec 5, 2015

Uh oh!

hyugit commented Dec 7, 2015

Uh oh!

kmagiera commented Dec 8, 2015

Uh oh!

mkonicek commented Dec 9, 2015

Uh oh!

hyugit commented Dec 11, 2015

Uh oh!

hyugit commented Dec 17, 2015

Uh oh!

satya164 commented Dec 27, 2015

Uh oh!

GantMan commented Jan 6, 2016

Uh oh!

christopherdro commented Jan 10, 2016

Uh oh!

facebook-github-bot commented Jan 10, 2016

Uh oh!

hyugit commented Jan 10, 2016

Uh oh!

gr4yscale commented Jan 12, 2016

Uh oh!

jsierles commented Jan 12, 2016

Uh oh!

mkonicek commented Jan 12, 2016

Uh oh!

facebook-github-bot commented Jan 12, 2016

Uh oh!

mkonicek commented Jan 12, 2016

Uh oh!

gr4yscale commented Jan 21, 2016

Uh oh!

mkonicek commented Jan 21, 2016

Uh oh!

mkonicek commented Jan 21, 2016

Uh oh!

mkonicek commented Jan 22, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants