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

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

Closed
wants to merge 6 commits into from
Closed

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e514f70
Fix StyleSheet 'textAlign' for AndroidTextInput
Nov 26, 2015
4281d91
remove duplicate examples of textAlign prop
Dec 5, 2015
ad16ca2
Merge pull request #5239 from BerndWessels/master
satya164 Jan 10, 2016
a9068cd
Fix StyleSheet 'textAlign' for AndroidTextInput
Nov 26, 2015
7e26860
remove duplicate examples of textAlign prop
Dec 5, 2015
847747a
Merge branch 'textAlign' of github.com:trave7er/react-native into tex…
Jan 10, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions Examples/UIExplorer/TextInputExample.android.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,46 @@ exports.examples = [
);
}
},
{
title: 'Text input, Alignment (Horizontal Demo Only)',
render: function() {
return (
<View>
<TextInput
placeholder="StyleSheet - textAlign: 'auto'"
style={[styles.singleLine, {textAlign: 'auto'}]}
/>
<TextInput
placeholder="StyleSheet - textAlign: 'left'"
style={[styles.singleLine, {textAlign: 'left'}]}
/>
<TextInput
placeholder="StyleSheet - textAlign: 'center'"
style={[styles.singleLine, {textAlign: 'center'}]}
/>
<TextInput
placeholder="StyleSheet - textAlign: 'right'"
style={[styles.singleLine, {textAlign: 'right'}]}
/>
<TextInput
placeholder="Native - textAlign='start'"
textAlign="start"
style={[styles.singleLine]}
/>
<TextInput
placeholder="Native - textAlign='center'"
textAlign="center"
style={[styles.singleLine]}
/>
<TextInput
placeholder="Native - textAlign='end'"
textAlign="end"
style={[styles.singleLine]}
/>
</View>
);
}
},
{
title: 'Passwords',
render: function() {
Expand Down
8 changes: 2 additions & 6 deletions Libraries/Components/TextInput/TextInput.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,10 +497,6 @@ var TextInput = React.createClass({

_renderAndroid: function() {
var autoCapitalize = UIManager.UIText.AutocapitalizationType[this.props.autoCapitalize];
var textAlign =
UIManager.AndroidTextInput.Constants.TextAlign[this.props.textAlign];
var textAlignVertical =
UIManager.AndroidTextInput.Constants.TextAlignVertical[this.props.textAlignVertical];
var children = this.props.children;
var childCount = 0;
ReactChildren.forEach(children, () => ++childCount);
Expand All @@ -517,8 +513,8 @@ var TextInput = React.createClass({
style={[this.props.style]}
autoCapitalize={autoCapitalize}
autoCorrect={this.props.autoCorrect}
textAlign={textAlign}
textAlignVertical={textAlignVertical}
textAlign={this.props.textAlign}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I'd still prefer this to be moved into styles as opposed to being a view prop. Just for the sake of consistency with the Text element.

Note that currently style property of TextInput element use the same propType as Text (see here), which allows textAlign (see here). This means that using textAlign for TextInput in style won't result in a warning.

I think we should add textAlignVertical to the style definition (with an annotation it's android-only) and remove it from propTypes definition for TextInput

textAlignVertical={this.props.textAlignVertical}
keyboardType={this.props.keyboardType}
mostRecentEventCount={this.state.mostRecentEventCount}
multiline={this.props.multiline}
Expand Down
47 changes: 28 additions & 19 deletions ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@

import javax.annotation.Nullable;

import java.util.Map;
import java.util.Map;
import java.util.HashMap;

import android.graphics.PorterDuff;
import android.os.SystemClock;
Expand All @@ -29,6 +30,7 @@

import com.facebook.infer.annotation.Assertions;
import com.facebook.react.bridge.JSApplicationCausedNativeException;
import com.facebook.react.bridge.JSApplicationIllegalArgumentException;
import com.facebook.react.bridge.ReactContext;
import com.facebook.react.bridge.ReadableArray;
import com.facebook.react.common.MapBuilder;
Expand Down Expand Up @@ -59,6 +61,24 @@ public class ReactTextInputManager extends
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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

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

TEXT_ALIGN_CONSTANTS = MapBuilder.of(
ViewProps.TEXT_ALIGN,
MapBuilder.of(
"left", Gravity.LEFT,
"right", Gravity.RIGHT,
"auto", Gravity.NO_GRAVITY,
"start", Gravity.START,
"center", Gravity.CENTER_HORIZONTAL,
"end", Gravity.END),
"textAlignVertical",
MapBuilder.of(
"top", Gravity.TOP,
"center", Gravity.CENTER_VERTICAL,
"bottom", Gravity.BOTTOM));
}

@Override
public String getName() {
return REACT_CLASS;
Expand Down Expand Up @@ -189,13 +209,17 @@ public void setUnderlineColor(ReactEditText view, @Nullable Integer underlineCol
}
}

@ReactProp(name = "textAlign")
public void setTextAlign(ReactEditText view, int gravity) {
@ReactProp(name = ViewProps.TEXT_ALIGN)
public void setTextAlign(ReactEditText view, @Nullable String textAlign) {
Map<String, Integer> constants = TEXT_ALIGN_CONSTANTS.get(ViewProps.TEXT_ALIGN);
int gravity = constants.get(textAlign);
view.setGravityHorizontal(gravity);
}

@ReactProp(name = "textAlignVertical")
public void setTextAlignVertical(ReactEditText view, int gravity) {
public void setTextAlignVertical(ReactEditText view, @Nullable String textAlignVertical) {
Map<String, Integer> constants = TEXT_ALIGN_CONSTANTS.get("textAlignVertical");
Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't use hash maps for this in java. They tend not to be very efficient (compering to alternative approaches) when you have a small number of keys. I'd recommend to at least have a separate maps for TEXT_ALIGN_CONSTANTS and TEXT_ALIGN_VERTICAL_CONSTANTS or even resign from the secondary level map here and just having a bunch of if blocks similarily to how it's handled here: ReactTextViewManager.java

int gravity = constants.get(textAlignVertical);
view.setGravityVertical(gravity);
}

Expand Down Expand Up @@ -417,19 +441,4 @@ public boolean onEditorAction(TextView v, int actionId, KeyEvent keyEvent) {
}
});
}

@Override
public @Nullable Map getExportedViewConstants() {
return MapBuilder.of(
"TextAlign",
MapBuilder.of(
"start", Gravity.START,
"center", Gravity.CENTER_HORIZONTAL,
"end", Gravity.END),
"TextAlignVertical",
MapBuilder.of(
"top", Gravity.TOP,
"center", Gravity.CENTER_VERTICAL,
"bottom", Gravity.BOTTOM));
}
}