From c74938e72e51b172e94abe10fab226c9fd9099d5 Mon Sep 17 00:00:00 2001 From: Emil Sjolander Date: Wed, 10 Aug 2016 05:08:13 -0700 Subject: [PATCH] Use spacing for position Differential Revision: D3690235 fbshipit-source-id: 4c04952e6ded32fd5fbfdccf63736cf025ae470e --- .../java/com/facebook/csslayout/CSSNode.java | 88 +---------- .../com/facebook/csslayout/CSSNodeAPI.java | 16 +- .../com/facebook/csslayout/CSSNodeJNI.java | 142 ++++++----------- .../react/uimanager/LayoutShadowNode.java | 5 +- .../LayoutPropertyApplicatorTest.java | 148 +++++++++++------- 5 files changed, 148 insertions(+), 251 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/csslayout/CSSNode.java b/ReactAndroid/src/main/java/com/facebook/csslayout/CSSNode.java index 1d3a8b28d..21582f3cc 100644 --- a/ReactAndroid/src/main/java/com/facebook/csslayout/CSSNode.java +++ b/ReactAndroid/src/main/java/com/facebook/csslayout/CSSNode.java @@ -19,12 +19,6 @@ import static com.facebook.csslayout.CSSLayout.DIMENSION_HEIGHT; import static com.facebook.csslayout.CSSLayout.DIMENSION_WIDTH; import static com.facebook.csslayout.CSSLayout.POSITION_LEFT; import static com.facebook.csslayout.CSSLayout.POSITION_TOP; -import static com.facebook.csslayout.Spacing.BOTTOM; -import static com.facebook.csslayout.Spacing.END; -import static com.facebook.csslayout.Spacing.LEFT; -import static com.facebook.csslayout.Spacing.RIGHT; -import static com.facebook.csslayout.Spacing.START; -import static com.facebook.csslayout.Spacing.TOP; /** * A CSS Node. It has a style object you can manipulate at {@link #style}. After calling @@ -414,95 +408,17 @@ public class CSSNode implements CSSNodeAPI { * Get this node's position, as defined by style. */ @Override - public Spacing getPositionValue() { + public Spacing getPosition() { return style.position; } @Override - public void setPositionValue(int spacingType, float position) { + public void setPosition(int spacingType, float position) { if (style.position.set(spacingType, position)) { dirty(); } } - /** - * Get this node's position top, as defined by style. - */ - @Override - public float getPositionTop() { - return style.position.get(TOP); - } - - @Override - public void setPositionTop(float positionTop) { - setPositionValue(TOP, positionTop); - } - - /** - * Get this node's position bottom, as defined by style. - */ - @Override - public float getPositionBottom() { - return style.position.get(BOTTOM); - } - - @Override - public void setPositionBottom(float positionBottom) { - setPositionValue(BOTTOM, positionBottom); - } - - /** - * Get this node's position left, as defined by style. - */ - @Override - public float getPositionLeft() { - return style.position.get(LEFT); - } - - @Override - public void setPositionLeft(float positionLeft) { - setPositionValue(LEFT, positionLeft); - } - - /** - * Get this node's position right, as defined by style. - */ - @Override - public float getPositionRight() { - return style.position.get(RIGHT); - } - - @Override - public void setPositionRight(float positionRight) { - setPositionValue(RIGHT, positionRight); - } - - /** - * Get this node's position start, as defined by style. - */ - @Override - public float getPositionStart() { - return style.position.get(START); - } - - @Override - public void setPositionStart(float positionStart) { - setPositionValue(START, positionStart); - } - - /** - * Get this node's position end, as defined by style. - */ - @Override - public float getPositionEnd() { - return style.position.get(END); - } - - @Override - public void setPositionEnd(float positionEnd) { - setPositionValue(END, positionEnd); - } - /** * Get this node's width, as defined in the style. */ diff --git a/ReactAndroid/src/main/java/com/facebook/csslayout/CSSNodeAPI.java b/ReactAndroid/src/main/java/com/facebook/csslayout/CSSNodeAPI.java index ba2d82e80..7cd418c96 100644 --- a/ReactAndroid/src/main/java/com/facebook/csslayout/CSSNodeAPI.java +++ b/ReactAndroid/src/main/java/com/facebook/csslayout/CSSNodeAPI.java @@ -58,20 +58,8 @@ public interface CSSNodeAPI { void setPadding(int spacingType, float padding); Spacing getBorder(); void setBorder(int spacingType, float border); - Spacing getPositionValue(); - void setPositionValue(int spacingType, float position); - float getPositionTop(); - void setPositionTop(float positionTop); - float getPositionBottom(); - void setPositionBottom(float positionBottom); - float getPositionLeft(); - void setPositionLeft(float positionLeft); - float getPositionRight(); - void setPositionRight(float positionRight); - float getPositionStart(); - void setPositionStart(float positionStart); - float getPositionEnd(); - void setPositionEnd(float positionEnd); + Spacing getPosition(); + void setPosition(int spacingType, float position); float getStyleWidth(); void setStyleWidth(float width); float getStyleHeight(); diff --git a/ReactAndroid/src/main/java/com/facebook/csslayout/CSSNodeJNI.java b/ReactAndroid/src/main/java/com/facebook/csslayout/CSSNodeJNI.java index 4fd97521f..676729a92 100644 --- a/ReactAndroid/src/main/java/com/facebook/csslayout/CSSNodeJNI.java +++ b/ReactAndroid/src/main/java/com/facebook/csslayout/CSSNodeJNI.java @@ -488,118 +488,72 @@ public class CSSNodeJNI implements CSSNodeAPI { } } + private native float jni_CSSNodeStyleGetPositionLeft(int nativePointer); + private native float jni_CSSNodeStyleGetPositionTop(int nativePointer); + private native float jni_CSSNodeStyleGetPositionRight(int nativePointer); + private native float jni_CSSNodeStyleGetPositionBottom(int nativePointer); + private native float jni_CSSNodeStyleGetPositionStart(int nativePointer); + private native float jni_CSSNodeStyleGetPositionEnd(int nativePointer); @Override - public Spacing getPositionValue() { + public Spacing getPosition() { Spacing position = new Spacing(); - position.set(Spacing.LEFT, getPositionLeft()); - position.set(Spacing.TOP, getPositionTop()); - position.set(Spacing.RIGHT, getPositionRight()); - position.set(Spacing.BOTTOM, getPositionBottom()); + position.set(Spacing.LEFT, jni_CSSNodeStyleGetPositionLeft(mNativePointer)); + position.set(Spacing.TOP, jni_CSSNodeStyleGetPositionTop(mNativePointer)); + position.set(Spacing.RIGHT, jni_CSSNodeStyleGetPositionRight(mNativePointer)); + position.set(Spacing.BOTTOM, jni_CSSNodeStyleGetPositionBottom(mNativePointer)); + position.set(Spacing.START, jni_CSSNodeStyleGetPositionStart(mNativePointer)); + position.set(Spacing.END, jni_CSSNodeStyleGetPositionEnd(mNativePointer)); return position; } + private native void jni_CSSNodeStyleSetPositionLeft(int nativePointer, float position); + private native void jni_CSSNodeStyleSetPositionTop(int nativePointer, float position); + private native void jni_CSSNodeStyleSetPositionRight(int nativePointer, float position); + private native void jni_CSSNodeStyleSetPositionBottom(int nativePointer, float position); + private native void jni_CSSNodeStyleSetPositionStart(int nativePointer, float position); + private native void jni_CSSNodeStyleSetPositionEnd(int nativePointer, float position); @Override - public void setPositionValue(int spacingType, float position) { + public void setPosition(int spacingType, float position) { switch (spacingType) { case Spacing.LEFT: - setPositionLeft(position); + jni_CSSNodeStyleSetPositionLeft(mNativePointer, position); break; case Spacing.TOP: - setPositionTop(position); + jni_CSSNodeStyleSetPositionTop(mNativePointer, position); break; case Spacing.RIGHT: - setPositionRight(position); + jni_CSSNodeStyleSetPositionRight(mNativePointer, position); break; case Spacing.BOTTOM: - setPositionBottom(position); + jni_CSSNodeStyleSetPositionBottom(mNativePointer, position); + break; + case Spacing.START: + jni_CSSNodeStyleSetPositionStart(mNativePointer, position); + break; + case Spacing.END: + jni_CSSNodeStyleSetPositionEnd(mNativePointer, position); + break; + case Spacing.HORIZONTAL: + jni_CSSNodeStyleSetPositionLeft(mNativePointer, position); + jni_CSSNodeStyleSetPositionRight(mNativePointer, position); + jni_CSSNodeStyleSetPositionStart(mNativePointer, position); + jni_CSSNodeStyleSetPositionEnd(mNativePointer, position); + break; + case Spacing.VERTICAL: + jni_CSSNodeStyleSetPositionTop(mNativePointer, position); + jni_CSSNodeStyleSetPositionBottom(mNativePointer, position); + break; + case Spacing.ALL: + jni_CSSNodeStyleSetPositionLeft(mNativePointer, position); + jni_CSSNodeStyleSetPositionRight(mNativePointer, position); + jni_CSSNodeStyleSetPositionStart(mNativePointer, position); + jni_CSSNodeStyleSetPositionEnd(mNativePointer, position); + jni_CSSNodeStyleSetPositionTop(mNativePointer, position); + jni_CSSNodeStyleSetPositionBottom(mNativePointer, position); break; } } - private native float jni_CSSNodeStyleGetPositionTop(int nativePointer); - @Override - public float getPositionTop() { - assertNativeInstance(); - return jni_CSSNodeStyleGetPositionTop(mNativePointer); - } - - private native void jni_CSSNodeStyleSetPositionTop(int nativePointer, float positionTop); - @Override - public void setPositionTop(float positionTop) { - assertNativeInstance(); - jni_CSSNodeStyleSetPositionTop(mNativePointer, positionTop); - } - - private native float jni_CSSNodeStyleGetPositionBottom(int nativePointer); - @Override - public float getPositionBottom() { - assertNativeInstance(); - return jni_CSSNodeStyleGetPositionBottom(mNativePointer); - } - - private native void jni_CSSNodeStyleSetPositionBottom(int nativePointer, float positionBottom); - @Override - public void setPositionBottom(float positionBottom) { - assertNativeInstance(); - jni_CSSNodeStyleSetPositionBottom(mNativePointer, positionBottom); - } - - private native float jni_CSSNodeStyleGetPositionLeft(int nativePointer); - @Override - public float getPositionLeft() { - assertNativeInstance(); - return jni_CSSNodeStyleGetPositionLeft(mNativePointer); - } - - private native void jni_CSSNodeStyleSetPositionLeft(int nativePointer, float positionLeft); - @Override - public void setPositionLeft(float positionLeft) { - assertNativeInstance(); - jni_CSSNodeStyleSetPositionLeft(mNativePointer, positionLeft); - } - - private native float jni_CSSNodeStyleGetPositionRight(int nativePointer); - @Override - public float getPositionRight() { - assertNativeInstance(); - return jni_CSSNodeStyleGetPositionRight(mNativePointer); - } - - private native void jni_CSSNodeStyleSetPositionRight(int nativePointer, float positionRight); - @Override - public void setPositionRight(float positionRight) { - assertNativeInstance(); - jni_CSSNodeStyleSetPositionRight(mNativePointer, positionRight); - } - - private native float jni_CSSNodeStyleGetPositionStart(int nativePointer); - @Override - public float getPositionStart() { - assertNativeInstance(); - return jni_CSSNodeStyleGetPositionStart(mNativePointer); - } - - private native void jni_CSSNodeStyleSetPositionStart(int nativePointer, float positionStart); - @Override - public void setPositionStart(float positionStart) { - assertNativeInstance(); - jni_CSSNodeStyleSetPositionStart(mNativePointer, positionStart); - } - - private native float jni_CSSNodeStyleGetPositionEnd(int nativePointer); - @Override - public float getPositionEnd() { - assertNativeInstance(); - return jni_CSSNodeStyleGetPositionEnd(mNativePointer); - } - - private native void jni_CSSNodeStyleSetPositionEnd(int nativePointer, float positionEnd); - @Override - public void setPositionEnd(float positionEnd) { - assertNativeInstance(); - jni_CSSNodeStyleSetPositionEnd(mNativePointer, positionEnd); - } - private native float jni_CSSNodeStyleGetWidth(int nativePointer); @Override public float getStyleWidth() { diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/LayoutShadowNode.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/LayoutShadowNode.java index 6b48667a2..b7e8620d3 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/LayoutShadowNode.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/LayoutShadowNode.java @@ -133,10 +133,9 @@ public class LayoutShadowNode extends ReactShadowNode { ViewProps.BOTTOM, }, defaultFloat = CSSConstants.UNDEFINED) public void setPositionValues(int index, float position) { - setPositionValue( + setPosition( ViewProps.POSITION_SPACING_TYPES[index], - CSSConstants.isUndefined(position) ? position : PixelUtil.toPixelFromDIP(position) - ); + CSSConstants.isUndefined(position) ? position : PixelUtil.toPixelFromDIP(position)); } @ReactProp(name = ViewProps.POSITION) diff --git a/ReactAndroid/src/test/java/com/facebook/react/uimanager/LayoutPropertyApplicatorTest.java b/ReactAndroid/src/test/java/com/facebook/react/uimanager/LayoutPropertyApplicatorTest.java index 453e0943c..5fb385c39 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/uimanager/LayoutPropertyApplicatorTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/uimanager/LayoutPropertyApplicatorTest.java @@ -36,7 +36,6 @@ import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyFloat; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.spy; @@ -73,19 +72,23 @@ public class LayoutPropertyApplicatorTest { LayoutShadowNode reactShadowNode = spy(new LayoutShadowNode()); ReactStylesDiffMap map = spy( buildStyles( - "width", 10.0, - "height", 10.0, - "left", 10.0, - "top", 10.0)); + "width", + 10.0, + "height", + 10.0, + "left", + 10.0, + "top", + 10.0)); reactShadowNode.updateProperties(map); verify(reactShadowNode).setStyleWidth(anyFloat()); verify(map).getFloat(eq("width"), anyFloat()); verify(reactShadowNode).setStyleHeight(anyFloat()); verify(map).getFloat(eq("height"), anyFloat()); - verify(reactShadowNode).setPositionValue(eq(Spacing.START), anyFloat()); + verify(reactShadowNode).setPosition(eq(Spacing.START), anyFloat()); verify(map).getFloat(eq("left"), anyFloat()); - verify(reactShadowNode).setPositionValue(eq(Spacing.TOP), anyFloat()); + verify(reactShadowNode).setPosition(eq(Spacing.TOP), anyFloat()); verify(map).getFloat(eq("top"), anyFloat()); reactShadowNode = spy(new LayoutShadowNode()); @@ -96,9 +99,9 @@ public class LayoutPropertyApplicatorTest { verify(map, never()).getFloat(eq("width"), anyFloat()); verify(reactShadowNode, never()).setStyleHeight(anyFloat()); verify(map, never()).getFloat(eq("height"), anyFloat()); - verify(reactShadowNode, never()).setPositionValue(eq(Spacing.START), anyFloat()); + verify(reactShadowNode, never()).setPosition(eq(Spacing.START), anyFloat()); verify(map, never()).getFloat(eq("left"), anyFloat()); - verify(reactShadowNode, never()).setPositionValue(eq(Spacing.TOP), anyFloat()); + verify(reactShadowNode, never()).setPosition(eq(Spacing.TOP), anyFloat()); verify(map, never()).getFloat(eq("top"), anyFloat()); } @@ -123,13 +126,16 @@ public class LayoutPropertyApplicatorTest { public void testPosition() { LayoutShadowNode reactShadowNode = spy(new LayoutShadowNode()); ReactStylesDiffMap map = spy(buildStyles( - "position", "absolute", - "bottom", 10.0, - "right", 5.0)); + "position", + "absolute", + "bottom", + 10.0, + "right", + 5.0)); reactShadowNode.updateProperties(map); - verify(reactShadowNode).setPositionValue(eq(Spacing.BOTTOM), anyFloat()); - verify(reactShadowNode).setPositionValue(eq(Spacing.END), anyFloat()); + verify(reactShadowNode).setPosition(eq(Spacing.BOTTOM), anyFloat()); + verify(reactShadowNode).setPosition(eq(Spacing.END), anyFloat()); verify(reactShadowNode).setPositionType(any(CSSPositionType.class)); verify(map).getFloat("bottom", Float.NaN); verify(map).getFloat("right", Float.NaN); @@ -138,8 +144,8 @@ public class LayoutPropertyApplicatorTest { map = spy(buildStyles()); reactShadowNode.updateProperties(map); - verify(reactShadowNode, never()).setPositionValue(eq(Spacing.BOTTOM), anyFloat()); - verify(reactShadowNode, never()).setPositionValue(eq(Spacing.END), anyFloat()); + verify(reactShadowNode, never()).setPosition(eq(Spacing.BOTTOM), anyFloat()); + verify(reactShadowNode, never()).setPosition(eq(Spacing.END), anyFloat()); verify(reactShadowNode, never()).setPositionType(any(CSSPositionType.class)); verify(map, never()).getFloat("bottom", Float.NaN); verify(map, never()).getFloat("right", Float.NaN); @@ -283,11 +289,16 @@ public class LayoutPropertyApplicatorTest { public void testEnumerations() { LayoutShadowNode reactShadowNode = spy(new LayoutShadowNode()); ReactStylesDiffMap map = buildStyles( - "flexDirection", "column", - "alignSelf", "stretch", - "alignItems", "center", - "justifyContent", "space_between", - "position", "relative"); + "flexDirection", + "column", + "alignSelf", + "stretch", + "alignItems", + "center", + "justifyContent", + "space_between", + "position", + "relative"); reactShadowNode.updateProperties(map); verify(reactShadowNode).setFlexDirection(CSSFlexDirection.COLUMN); @@ -315,25 +326,38 @@ public class LayoutPropertyApplicatorTest { LayoutShadowNode reactShadowNode = spy(new LayoutShadowNode()); ReactStylesDiffMap map = buildStyles( - "width", 10.0, - "height", 10.0, - "left", 10.0, - "top", 10.0, - "flex", 1.0, - "padding", 10.0, - "marginLeft", 10.0, - "borderTopWidth", 10.0, - "flexDirection", "row", - "alignSelf", "stretch", - "alignItems", "center", - "justifyContent", "space_between", - "position", "absolute"); + "width", + 10.0, + "height", + 10.0, + "left", + 10.0, + "top", + 10.0, + "flex", + 1.0, + "padding", + 10.0, + "marginLeft", + 10.0, + "borderTopWidth", + 10.0, + "flexDirection", + "row", + "alignSelf", + "stretch", + "alignItems", + "center", + "justifyContent", + "space_between", + "position", + "absolute"); reactShadowNode.updateProperties(map); verify(reactShadowNode).setStyleWidth(10.f); verify(reactShadowNode).setStyleHeight(10.f); - verify(reactShadowNode).setPositionValue(Spacing.START, 10.f); - verify(reactShadowNode).setPositionValue(Spacing.TOP, 10.f); + verify(reactShadowNode).setPosition(Spacing.START, 10.f); + verify(reactShadowNode).setPosition(Spacing.TOP, 10.f); verify(reactShadowNode).setFlex(1.0f); verify(reactShadowNode).setPadding(Spacing.ALL, 10.f); verify(reactShadowNode).setMargin(Spacing.START, 10.f); @@ -345,26 +369,39 @@ public class LayoutPropertyApplicatorTest { verify(reactShadowNode).setPositionType(CSSPositionType.ABSOLUTE); map = buildStyles( - "width", null, - "height", null, - "left", null, - "top", null, - "flex", null, - "padding", null, - "marginLeft", null, - "borderTopWidth", null, - "flexDirection", null, - "alignSelf", null, - "alignItems", null, - "justifyContent", null, - "position", null); + "width", + null, + "height", + null, + "left", + null, + "top", + null, + "flex", + null, + "padding", + null, + "marginLeft", + null, + "borderTopWidth", + null, + "flexDirection", + null, + "alignSelf", + null, + "alignItems", + null, + "justifyContent", + null, + "position", + null); reset(reactShadowNode); reactShadowNode.updateProperties(map); verify(reactShadowNode).setStyleWidth(CSSConstants.UNDEFINED); verify(reactShadowNode).setStyleHeight(CSSConstants.UNDEFINED); - verify(reactShadowNode).setPositionValue(Spacing.START, CSSConstants.UNDEFINED); - verify(reactShadowNode).setPositionValue(Spacing.TOP, CSSConstants.UNDEFINED); + verify(reactShadowNode).setPosition(Spacing.START, CSSConstants.UNDEFINED); + verify(reactShadowNode).setPosition(Spacing.TOP, CSSConstants.UNDEFINED); verify(reactShadowNode).setFlex(0.f); verify(reactShadowNode).setPadding(Spacing.ALL, CSSConstants.UNDEFINED); verify(reactShadowNode).setMargin(Spacing.START, CSSConstants.UNDEFINED); @@ -404,9 +441,12 @@ public class LayoutPropertyApplicatorTest { mapNodes[3] = buildStyles("paddingBottom", 10.0, "paddingHorizontal", 5.0); mapNodes[4] = buildStyles("padding", null, "paddingTop", 5.0); mapNodes[5] = buildStyles( - "paddingRight", 10.0, - "paddingHorizontal", null, - "paddingVertical", 7.0); + "paddingRight", + 10.0, + "paddingHorizontal", + null, + "paddingVertical", + 7.0); mapNodes[6] = buildStyles("margin", 5.0); for (int idx = 0; idx < nodes.length; idx++) {