Skip to content

Commit

Permalink
Fix padding with Text on Android
Browse files Browse the repository at this point in the history
Summary: Text was not correctly respecting padding.  We would account for it when measuring, but then not actually apply the padding to the text.  This adds support for proper padding

Reviewed By: andreicoman11

Differential Revision: D3516692

fbshipit-source-id: 9a0991d89e9194c0e87af0af56c6631a6b95700a
  • Loading branch information
Dave Miller authored and Facebook Github Bot 9 committed Jul 6, 2016
1 parent 0fde81c commit c3f2bba
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ public void onCollectExtraUpdates(UIViewOperationQueue uiViewOperationQueue) {
super.onCollectExtraUpdates(uiViewOperationQueue);
if (mPreparedSpannableText != null) {
ReactTextUpdate reactTextUpdate =
new ReactTextUpdate(mPreparedSpannableText, UNSET, mContainsImages);
new ReactTextUpdate(mPreparedSpannableText, UNSET, mContainsImages, getPadding());
uiViewOperationQueue.enqueueUpdateExtraData(getReactTag(), reactTextUpdate);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

import android.text.Spannable;

import com.facebook.csslayout.Spacing;

/**
* Class that contains the data needed for a text update.
* Used by both <Text/> and <TextInput/>
Expand All @@ -21,11 +23,17 @@ public class ReactTextUpdate {
private final Spannable mText;
private final int mJsEventCounter;
private final boolean mContainsImages;
private final Spacing mPadding;

public ReactTextUpdate(Spannable text, int jsEventCounter, boolean containsImages) {
public ReactTextUpdate(
Spannable text,
int jsEventCounter,
boolean containsImages,
Spacing padding) {
mText = text;
mJsEventCounter = jsEventCounter;
mContainsImages = containsImages;
mPadding = padding;
}

public Spannable getText() {
Expand All @@ -39,4 +47,8 @@ public int getJsEventCounter() {
public boolean containsImages() {
return mContainsImages;
}

public Spacing getPadding() {
return mPadding;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import android.view.ViewGroup;
import android.widget.TextView;

import com.facebook.csslayout.Spacing;
import com.facebook.react.uimanager.ReactCompoundView;

public class ReactTextView extends TextView implements ReactCompoundView {
Expand Down Expand Up @@ -45,6 +46,12 @@ public void setText(ReactTextUpdate update) {
setLayoutParams(EMPTY_LAYOUT_PARAMS);
}
setText(update.getText());
Spacing padding = update.getPadding();
setPadding(
(int) Math.ceil(padding.get(Spacing.LEFT)),
(int) Math.ceil(padding.get(Spacing.TOP)),
(int) Math.ceil(padding.get(Spacing.RIGHT)),
(int) Math.ceil(padding.get(Spacing.BOTTOM)));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public void onCollectExtraUpdates(UIViewOperationQueue uiViewOperationQueue) {
if (mJsEventCount != UNSET) {
Spannable preparedSpannableText = fromTextCSSNode(this);
ReactTextUpdate reactTextUpdate =
new ReactTextUpdate(preparedSpannableText, mJsEventCount, mContainsImages);
new ReactTextUpdate(preparedSpannableText, mJsEventCount, mContainsImages, getPadding());
uiViewOperationQueue.enqueueUpdateExtraData(getReactTag(), reactTextUpdate);
}
}
Expand Down

5 comments on commit c3f2bba

@tomauty
Copy link
Contributor

Choose a reason for hiding this comment

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

:)

@silentcloud
Copy link

Choose a reason for hiding this comment

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

👍

@antoinerousseau
Copy link
Contributor

@antoinerousseau antoinerousseau commented on c3f2bba Aug 5, 2016

Choose a reason for hiding this comment

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

woohoo that fixes my bug 👍

EDIT: it doesn't :/

@pein892
Copy link

Choose a reason for hiding this comment

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

good job!

@noneven
Copy link

Choose a reason for hiding this comment

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

good

Please sign in to comment.