Fix StyleSheet 'textAlign' for AndroidTextInput. Closes #2702 #4481

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)

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

ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java

@@ -59,6 +61,24 @@
 private static final String KEYBOARD_TYPE_NUMERIC = "numeric";
 private static final InputFilter[] EMPTY_FILTERS = new InputFilter[0];

+ private static final Map<String, Map<String, Integer>> TEXT_ALIGN_CONSTANTS;
+ static {


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

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 #2702 Dec 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 #5239 from 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 f453e14 Jan 21, 2016

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix StyleSheet 'textAlign' for AndroidTextInput. Closes #2702 #4481

Fix StyleSheet 'textAlign' for AndroidTextInput. Closes #2702 #4481

hyugit commented Dec 1, 2015

facebook-github-bot commented Dec 1, 2015

kmagiera Dec 2, 2015

mkonicek commented Dec 2, 2015

facebook-github-bot commented Dec 5, 2015

hyugit commented Dec 7, 2015

kmagiera commented Dec 8, 2015

mkonicek commented Dec 9, 2015

hyugit commented Dec 11, 2015

hyugit commented Dec 17, 2015

satya164 commented Dec 27, 2015

GantMan commented Jan 6, 2016

christopherdro commented Jan 10, 2016

facebook-github-bot commented Jan 10, 2016

hyugit commented Jan 10, 2016

gr4yscale commented Jan 12, 2016

jsierles commented Jan 12, 2016

mkonicek commented Jan 12, 2016

facebook-github-bot commented Jan 12, 2016

mkonicek commented Jan 12, 2016

gr4yscale commented Jan 21, 2016

mkonicek commented Jan 21, 2016

mkonicek commented Jan 21, 2016

mkonicek commented Jan 22, 2016

Fix StyleSheet 'textAlign' for AndroidTextInput. Closes #2702 #4481

Fix StyleSheet 'textAlign' for AndroidTextInput. Closes #2702 #4481

Conversation

hyugit commented Dec 1, 2015

facebook-github-bot commented Dec 1, 2015

kmagiera Dec 2, 2015

Choose a reason for hiding this comment

mkonicek commented Dec 2, 2015

facebook-github-bot commented Dec 5, 2015

hyugit commented Dec 7, 2015

kmagiera commented Dec 8, 2015

mkonicek commented Dec 9, 2015

hyugit commented Dec 11, 2015

hyugit commented Dec 17, 2015

satya164 commented Dec 27, 2015

GantMan commented Jan 6, 2016

christopherdro commented Jan 10, 2016

facebook-github-bot commented Jan 10, 2016

hyugit commented Jan 10, 2016

gr4yscale commented Jan 12, 2016

jsierles commented Jan 12, 2016

mkonicek commented Jan 12, 2016

facebook-github-bot commented Jan 12, 2016

mkonicek commented Jan 12, 2016

gr4yscale commented Jan 21, 2016

mkonicek commented Jan 21, 2016

mkonicek commented Jan 21, 2016

mkonicek commented Jan 22, 2016