Skip to content

Commit

Permalink
When paddingStart is 0, it should override paddingHorizontal (#816)
Browse files Browse the repository at this point in the history
Summary:
Fixes #815

Imagine a node with this style: `{ paddingHorizontal: 10, paddingStart: 0 }`.

After running layout on this node, we expect its computed `paddingStart` to be `0`. However, it is actually `10`.

Consider the expression `paddingEdgeStart.getValue() > 0.0f` in [`getLeadingPadding`](https://github.com/facebook/yoga/blob/328ec7dc4d104b42b836d5ccebff04033d045133/yoga/YGNode.cpp#L461). Why is `0` handled like a negative number rather than a positive number? I suspect this should be `>=` so `0` is handled like the positive numbers (this is how `getTrailingPadding` works).

It looks like facebook/yoga@3a82d2b?diff=unified&w=1#diff-07b4949bf42749fde386e769ff08a124 changed the operator from `>=` to `>` in `getLeadingPadding`. I suspect it was a mistake. `getTrailingPadding` still uses `>=`.

I manually verified this using the code in #815 and added some unit tests to catch this bug and other similar issues.

Adam Comella
Microsoft Corp.
Pull Request resolved: facebook/yoga#816

Reviewed By: priteshrnandgaonkar

Differential Revision: D10282617

Pulled By: shergin

fbshipit-source-id: 2ab2874ae39d9454308a020a960ace85573fe777
  • Loading branch information
Adam Comella authored and facebook-github-bot committed Oct 12, 2018
1 parent 10fc548 commit f90c9b5
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions ReactCommon/yoga/yoga/YGNode.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) Facebook, Inc. and its affiliates.
* Copyright (c) 2018-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the LICENSE
* file in the root directory of this source tree.
Expand Down Expand Up @@ -458,7 +458,7 @@ YGFloatOptional YGNode::getLeadingPadding(
YGResolveValue(style_.padding[YGEdgeStart], widthSize);
if (YGFlexDirectionIsRow(axis) &&
style_.padding[YGEdgeStart].unit != YGUnitUndefined &&
!paddingEdgeStart.isUndefined() && paddingEdgeStart.getValue() > 0.0f) {
!paddingEdgeStart.isUndefined() && paddingEdgeStart.getValue() >= 0.0f) {
return paddingEdgeStart;
}

Expand Down

0 comments on commit f90c9b5

Please sign in to comment.