Skip to content

Commit

Permalink
Fix CSSLayout to Support RTL
Browse files Browse the repository at this point in the history 
Summary:
The current CSSLayout can't support RTL because wrong calculation for absolute position.
This change is mainly to fix the issue: facebook/yoga#197
Three main problems I fixed:
1. Calculate the position in the same way as margin, boarder, and padding. So that to fix the absolute problem.
2. Fix one wrong calculation for leading value when we only know the trailing value. It was hard code for the LTR situation. Now I changed it to depends on the main Axis.
3. Expose getter and setter function for RN to read layout direction and start/end position value.

Reviewed By: fkgozali

Differential Revision: D3616949

fbshipit-source-id: ae7a47cc0a5d02b42b95f87232be51ab144056d9
  • Loading branch information
@MengjueW
MengjueW authored and Facebook Github Bot 9 committed Jul 28, 2016
1 parent b5abb87 commit 873c6ff
Show file tree
Hide file tree
Showing 6 changed files with 201 additions and 135 deletions.
2 changes: 1 addition & 1 deletion React/CSSLayout/CSSLayout-internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ typedef struct CSSStyle {
CSSOverflow overflow;
float flex;
float margin[6];
float position[4];
float position[6];
/**
* You should skip all the rules that contain negative values for the
* following attributes. For example:
Expand Down
147 changes: 84 additions & 63 deletions React/CSSLayout/CSSLayout.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ void CSSNodeInit(CSSNodeRef node) {
node->style.position[CSSPositionTop] = CSSUndefined;
node->style.position[CSSPositionRight] = CSSUndefined;
node->style.position[CSSPositionBottom] = CSSUndefined;
node->style.position[CSSPositionStart] = CSSUndefined;
node->style.position[CSSPositionEnd] = CSSUndefined;

node->style.margin[CSSPositionStart] = CSSUndefined;
node->style.margin[CSSPositionEnd] = CSSUndefined;
Expand Down Expand Up @@ -173,6 +175,8 @@ CSS_NODE_STYLE_PROPERTY_IMPL(float, PositionLeft, positionLeft, position[CSSPosi
CSS_NODE_STYLE_PROPERTY_IMPL(float, PositionTop, positionTop, position[CSSPositionTop]);
CSS_NODE_STYLE_PROPERTY_IMPL(float, PositionRight, positionRight, position[CSSPositionRight]);
CSS_NODE_STYLE_PROPERTY_IMPL(float, PositionBottom, positionBottom, position[CSSPositionBottom]);
CSS_NODE_STYLE_PROPERTY_IMPL(float, PositionStart, positionStart, position[CSSPositionStart]);
CSS_NODE_STYLE_PROPERTY_IMPL(float, PositionEnd, positionEnd, position[CSSPositionEnd]);

CSS_NODE_STYLE_PROPERTY_IMPL(float, MarginLeft, marginLeft, margin[CSSPositionLeft]);
CSS_NODE_STYLE_PROPERTY_IMPL(float, MarginTop, marginTop, margin[CSSPositionTop]);
Expand Down Expand Up @@ -208,6 +212,7 @@ CSS_NODE_LAYOUT_PROPERTY_IMPL(float, Right, position[CSSPositionRight]);
CSS_NODE_LAYOUT_PROPERTY_IMPL(float, Bottom, position[CSSPositionBottom]);
CSS_NODE_LAYOUT_PROPERTY_IMPL(float, Width, dimensions[CSSDimensionWidth]);
CSS_NODE_LAYOUT_PROPERTY_IMPL(float, Height, dimensions[CSSDimensionHeight]);
CSS_NODE_LAYOUT_PROPERTY_IMPL(CSSDirection, Direction, direction);

int gCurrentGenerationCount = 0;

Expand Down Expand Up @@ -623,18 +628,36 @@ static bool isLayoutDimDefined(CSSNode* node, CSSFlexDirection axis) {
return !isUndefined(value) && value >= 0.0;
}

static bool isPosDefined(CSSNode* node, CSSPosition position) {
return !isUndefined(node->style.position[position]);
static bool isLeadingPosDefined(CSSNode* node, CSSFlexDirection axis) {
return (isRowDirection(axis) && !isUndefined(node->style.position[CSSPositionStart]))
|| !isUndefined(node->style.position[leading[axis]]);
}

static bool isTrailingPosDefined(CSSNode* node, CSSFlexDirection axis) {
return (isRowDirection(axis) && !isUndefined(node->style.position[CSSPositionEnd]))
|| !isUndefined(node->style.position[trailing[axis]]);
}

static bool isMeasureDefined(CSSNode* node) {
return node->measure;
}

static float getPosition(CSSNode* node, CSSPosition position) {
float result = node->style.position[position];
if (!isUndefined(result)) {
return result;
static float getLeadingPosition(CSSNode* node, CSSFlexDirection axis) {
if (isRowDirection(axis) && !isUndefined(node->style.position[CSSPositionStart])) {
return node->style.position[CSSPositionStart];
}
if (!isUndefined(node->style.position[leading[axis]])) {
return node->style.position[leading[axis]];
}
return 0;
}

static float getTrailingPosition(CSSNode* node, CSSFlexDirection axis) {
if (isRowDirection(axis) && !isUndefined(node->style.position[CSSPositionEnd])) {
return node->style.position[CSSPositionEnd];
}
if (!isUndefined(node->style.position[trailing[axis]])) {
return node->style.position[trailing[axis]];
}
return 0;
}
Expand Down Expand Up @@ -670,20 +693,18 @@ static float boundAxis(CSSNode* node, CSSFlexDirection axis, float value) {
}

static void setTrailingPosition(CSSNode* node, CSSNode* child, CSSFlexDirection axis) {
float size = child->style.positionType == CSSPositionTypeAbsolute ?
0 :
child->layout.measuredDimensions[dim[axis]];
float size = child->layout.measuredDimensions[dim[axis]];
child->layout.position[trailing[axis]] = node->layout.measuredDimensions[dim[axis]] - size - child->layout.position[pos[axis]];
}

// If both left and right are defined, then use left. Otherwise return
// +left or -right depending on which is defined.
static float getRelativePosition(CSSNode* node, CSSFlexDirection axis) {
float lead = node->style.position[leading[axis]];
float lead = getLeadingPosition(node, axis);
if (!isUndefined(lead)) {
return lead;
}
return -getPosition(node, trailing[axis]);
return -getTrailingPosition(node, axis);
}

static void setPosition(CSSNode* node, CSSDirection direction) {
Expand Down Expand Up @@ -1294,12 +1315,12 @@ static void layoutNodeImpl(CSSNode* node, float availableWidth, float availableH
child = CSSNodeListGet(node->children, i);

if (child->style.positionType == CSSPositionTypeAbsolute &&
isPosDefined(child, leading[mainAxis])) {
isLeadingPosDefined(child, mainAxis)) {
if (performLayout) {
// In case the child is position absolute and has left/top being
// defined, we override the position to whatever the user said
// (and margin/border).
child->layout.position[pos[mainAxis]] = getPosition(child, leading[mainAxis]) +
child->layout.position[pos[mainAxis]] = getLeadingPosition(child, mainAxis) +
getLeadingBorder(node, mainAxis) +
getLeadingMargin(child, mainAxis);
}
Expand Down Expand Up @@ -1361,8 +1382,8 @@ static void layoutNodeImpl(CSSNode* node, float availableWidth, float availableH
if (child->style.positionType == CSSPositionTypeAbsolute) {
// If the child is absolutely positioned and has a top/left/bottom/right
// set, override all the previously computed positions to set it correctly.
if (isPosDefined(child, leading[crossAxis])) {
child->layout.position[pos[crossAxis]] = getPosition(child, leading[crossAxis]) +
if (isLeadingPosDefined(child, crossAxis)) {
child->layout.position[pos[crossAxis]] = getLeadingPosition(child, crossAxis) +
getLeadingBorder(node, crossAxis) +
getLeadingMargin(child, crossAxis);
} else {
Expand Down Expand Up @@ -1518,38 +1539,7 @@ static void layoutNodeImpl(CSSNode* node, float availableWidth, float availableH
paddingAndBorderAxisCross);
}

// STEP 10: SETTING TRAILING POSITIONS FOR CHILDREN
if (performLayout) {
bool needsMainTrailingPos = false;
bool needsCrossTrailingPos = false;

if (mainAxis == CSSFlexDirectionRowReverse ||
mainAxis == CSSFlexDirectionColumnReverse) {
needsMainTrailingPos = true;
}

if (crossAxis == CSSFlexDirectionRowReverse ||
crossAxis == CSSFlexDirectionColumnReverse) {
needsCrossTrailingPos = true;
}

// Set trailing position if necessary.
if (needsMainTrailingPos || needsCrossTrailingPos) {
for (i = 0; i < childCount; ++i) {
child = CSSNodeListGet(node->children, i);

if (needsMainTrailingPos) {
setTrailingPosition(node, child, mainAxis);
}

if (needsCrossTrailingPos) {
setTrailingPosition(node, child, crossAxis);
}
}
}
}

// STEP 11: SIZING AND POSITIONING ABSOLUTE CHILDREN
// STEP 10: SIZING AND POSITIONING ABSOLUTE CHILDREN
currentAbsoluteChild = firstAbsoluteChild;
while (currentAbsoluteChild != NULL) {
// Now that we know the bounds of the container, perform layout again on the
Expand All @@ -1563,10 +1553,10 @@ static void layoutNodeImpl(CSSNode* node, float availableWidth, float availableH
childWidth = currentAbsoluteChild->style.dimensions[CSSDimensionWidth] + getMarginAxis(currentAbsoluteChild, CSSFlexDirectionRow);
} else {
// If the child doesn't have a specified width, compute the width based on the left/right offsets if they're defined.
if (isPosDefined(currentAbsoluteChild, CSSPositionLeft) && isPosDefined(currentAbsoluteChild, CSSPositionRight)) {
if (isLeadingPosDefined(currentAbsoluteChild, CSSFlexDirectionRow) && isTrailingPosDefined(currentAbsoluteChild, CSSFlexDirectionRow)) {
childWidth = node->layout.measuredDimensions[CSSDimensionWidth] -
(getLeadingBorder(node, CSSFlexDirectionRow) + getTrailingBorder(node, CSSFlexDirectionRow)) -
(currentAbsoluteChild->style.position[CSSPositionLeft] + currentAbsoluteChild->style.position[CSSPositionRight]);
(getLeadingPosition(currentAbsoluteChild, CSSFlexDirectionRow) + getTrailingPosition(currentAbsoluteChild, CSSFlexDirectionRow));
childWidth = boundAxis(currentAbsoluteChild, CSSFlexDirectionRow, childWidth);
}
}
Expand All @@ -1575,10 +1565,10 @@ static void layoutNodeImpl(CSSNode* node, float availableWidth, float availableH
childHeight = currentAbsoluteChild->style.dimensions[CSSDimensionHeight] + getMarginAxis(currentAbsoluteChild, CSSFlexDirectionColumn);
} else {
// If the child doesn't have a specified height, compute the height based on the top/bottom offsets if they're defined.
if (isPosDefined(currentAbsoluteChild, CSSPositionTop) && isPosDefined(currentAbsoluteChild, CSSPositionBottom)) {
if (isLeadingPosDefined(currentAbsoluteChild, CSSFlexDirectionColumn) && isTrailingPosDefined(currentAbsoluteChild, CSSFlexDirectionColumn)) {
childHeight = node->layout.measuredDimensions[CSSDimensionHeight] -
(getLeadingBorder(node, CSSFlexDirectionColumn) + getTrailingBorder(node, CSSFlexDirectionColumn)) -
(currentAbsoluteChild->style.position[CSSPositionTop] + currentAbsoluteChild->style.position[CSSPositionBottom]);
(getLeadingPosition(currentAbsoluteChild, CSSFlexDirectionColumn) + getTrailingPosition(currentAbsoluteChild, CSSFlexDirectionColumn));
childHeight = boundAxis(currentAbsoluteChild, CSSFlexDirectionColumn, childHeight);
}
}
Expand Down Expand Up @@ -1613,25 +1603,56 @@ static void layoutNodeImpl(CSSNode* node, float availableWidth, float availableH

layoutNodeInternal(currentAbsoluteChild, childWidth, childHeight, direction, CSSMeasureModeExactly, CSSMeasureModeExactly, true, "abs-layout");

if (isPosDefined(currentAbsoluteChild, trailing[CSSFlexDirectionRow]) &&
!isPosDefined(currentAbsoluteChild, leading[CSSFlexDirectionRow])) {
currentAbsoluteChild->layout.position[leading[CSSFlexDirectionRow]] =
node->layout.measuredDimensions[dim[CSSFlexDirectionRow]] -
currentAbsoluteChild->layout.measuredDimensions[dim[CSSFlexDirectionRow]] -
getPosition(currentAbsoluteChild, trailing[CSSFlexDirectionRow]);
if (isTrailingPosDefined(currentAbsoluteChild, mainAxis) &&
!isLeadingPosDefined(currentAbsoluteChild, mainAxis)) {
currentAbsoluteChild->layout.position[leading[mainAxis]] =
node->layout.measuredDimensions[dim[mainAxis]] -
currentAbsoluteChild->layout.measuredDimensions[dim[mainAxis]] -
getTrailingPosition(currentAbsoluteChild, mainAxis);
}

if (isPosDefined(currentAbsoluteChild, trailing[CSSFlexDirectionColumn]) &&
!isPosDefined(currentAbsoluteChild, leading[CSSFlexDirectionColumn])) {
currentAbsoluteChild->layout.position[leading[CSSFlexDirectionColumn]] =
node->layout.measuredDimensions[dim[CSSFlexDirectionColumn]] -
currentAbsoluteChild->layout.measuredDimensions[dim[CSSFlexDirectionColumn]] -
getPosition(currentAbsoluteChild, trailing[CSSFlexDirectionColumn]);
if (isTrailingPosDefined(currentAbsoluteChild, crossAxis) &&
!isLeadingPosDefined(currentAbsoluteChild, crossAxis)) {
currentAbsoluteChild->layout.position[leading[crossAxis]] =
node->layout.measuredDimensions[dim[crossAxis]] -
currentAbsoluteChild->layout.measuredDimensions[dim[crossAxis]] -
getTrailingPosition(currentAbsoluteChild, crossAxis);
}
}

currentAbsoluteChild = currentAbsoluteChild->nextChild;
}

// STEP 11: SETTING TRAILING POSITIONS FOR CHILDREN
if (performLayout) {
bool needsMainTrailingPos = false;
bool needsCrossTrailingPos = false;

if (mainAxis == CSSFlexDirectionRowReverse ||
mainAxis == CSSFlexDirectionColumnReverse) {
needsMainTrailingPos = true;
}

if (crossAxis == CSSFlexDirectionRowReverse ||
crossAxis == CSSFlexDirectionColumnReverse) {
needsCrossTrailingPos = true;
}

// Set trailing position if necessary.
if (needsMainTrailingPos || needsCrossTrailingPos) {
for (i = 0; i < childCount; ++i) {
child = CSSNodeListGet(node->children, i);

if (needsMainTrailingPos) {
setTrailingPosition(node, child, mainAxis);
}

if (needsCrossTrailingPos) {
setTrailingPosition(node, child, crossAxis);
}
}
}
}
}

int gDepth = 0;
Expand Down
3 changes: 3 additions & 0 deletions React/CSSLayout/CSSLayout.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ CSS_NODE_STYLE_PROPERTY(float, PositionLeft, positionLeft);
CSS_NODE_STYLE_PROPERTY(float, PositionTop, positionTop);
CSS_NODE_STYLE_PROPERTY(float, PositionRight, positionRight);
CSS_NODE_STYLE_PROPERTY(float, PositionBottom, positionBottom);
CSS_NODE_STYLE_PROPERTY(float, PositionStart, positionStart);
CSS_NODE_STYLE_PROPERTY(float, PositionEnd, positionEnd);

CSS_NODE_STYLE_PROPERTY(float, MarginLeft, marginLeft);
CSS_NODE_STYLE_PROPERTY(float, MarginTop, marginTop);
Expand Down Expand Up @@ -204,6 +206,7 @@ CSS_NODE_LAYOUT_PROPERTY(float, Right);
CSS_NODE_LAYOUT_PROPERTY(float, Bottom);
CSS_NODE_LAYOUT_PROPERTY(float, Width);
CSS_NODE_LAYOUT_PROPERTY(float, Height);
CSS_NODE_LAYOUT_PROPERTY(CSSDirection, Direction);

CSS_EXTERN_C_END

Expand Down
39 changes: 25 additions & 14 deletions ReactAndroid/src/main/java/com/facebook/csslayout/CSSNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@

import static com.facebook.csslayout.CSSLayout.DIMENSION_HEIGHT;
import static com.facebook.csslayout.CSSLayout.DIMENSION_WIDTH;
import static com.facebook.csslayout.CSSLayout.POSITION_BOTTOM;
import static com.facebook.csslayout.CSSLayout.POSITION_LEFT;
import static com.facebook.csslayout.CSSLayout.POSITION_RIGHT;
import static com.facebook.csslayout.CSSLayout.POSITION_TOP;
import static com.facebook.csslayout.Spacing.BOTTOM;
import static com.facebook.csslayout.Spacing.LEFT;
import static com.facebook.csslayout.Spacing.RIGHT;
import static com.facebook.csslayout.Spacing.TOP;

/**
* A CSS Node. It has a style object you can manipulate at {@link #style}. After calling
Expand Down Expand Up @@ -373,16 +375,28 @@ public void setBorder(int spacingType, float border) {
}
}

/**
* Get this node's position, as defined by style.
*/
public Spacing getPosition() {
return style.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.
*/
public float getPositionTop() {
return style.position[POSITION_TOP];
return style.position.get(TOP);
}

public void setPositionTop(float positionTop) {
if (!valuesEqual(style.position[POSITION_TOP], positionTop)) {
style.position[POSITION_TOP] = positionTop;
if (style.position.set(TOP, positionTop)) {
dirty();
}
}
Expand All @@ -391,12 +405,11 @@ public void setPositionTop(float positionTop) {
* Get this node's position bottom, as defined by style.
*/
public float getPositionBottom() {
return style.position[POSITION_BOTTOM];
return style.position.get(BOTTOM);
}

public void setPositionBottom(float positionBottom) {
if (!valuesEqual(style.position[POSITION_BOTTOM], positionBottom)) {
style.position[POSITION_BOTTOM] = positionBottom;
if (style.position.set(BOTTOM, positionBottom)) {
dirty();
}
}
Expand All @@ -405,12 +418,11 @@ public void setPositionBottom(float positionBottom) {
* Get this node's position left, as defined by style.
*/
public float getPositionLeft() {
return style.position[POSITION_LEFT];
return style.position.get(LEFT);
}

public void setPositionLeft(float positionLeft) {
if (!valuesEqual(style.position[POSITION_LEFT], positionLeft)) {
style.position[POSITION_LEFT] = positionLeft;
if (style.position.set(LEFT, positionLeft)) {
dirty();
}
}
Expand All @@ -419,12 +431,11 @@ public void setPositionLeft(float positionLeft) {
* Get this node's position right, as defined by style.
*/
public float getPositionRight() {
return style.position[POSITION_RIGHT];
return style.position.get(RIGHT);
}

public void setPositionRight(float positionRight) {
if (!valuesEqual(style.position[POSITION_RIGHT], positionRight)) {
style.position[POSITION_RIGHT] = positionRight;
if (style.position.set(RIGHT, positionRight)) {
dirty();
}
}
Expand Down
Loading

0 comments on commit 873c6ff

Please sign in to comment.