Skip to content

Normalized C and Java definition of isDimDefined.#59

Merged
vjeux merged 1 commit into
react:masterfrom
freakboy3742:normalize_isDimDefined
Mar 22, 2015
Merged

Normalized C and Java definition of isDimDefined.#59
vjeux merged 1 commit into
react:masterfrom
freakboy3742:normalize_isDimDefined

Conversation

@freakboy3742

Copy link
Copy Markdown
Contributor

The JavaScript implementation of isDimDefined contains a check to ensure
that the dimension value is positive; the C and Java versions did not
have this check. As a result, a negative style value for 'width' (such
as that used by the "should layout node with negative width" test) would
have different layout under the C/Java implementation to the JavaScript
implementation.

This was hidden because the C/Java transpilers filtered out any negative
instantiation values from the test suite. In effect, the negative value
tests weren't running on the C/Java implementation.

This patch removes the negative value filter from the transpiler, and
makes the isDimDefined definition consistent between the three
implementations.

The JavaScript implementation of isDimDefined contains a check to ensure
that the dimension value is positive; the C and Java versions did not
have this check. As a result, a negative style value for 'width' (such
as that used by the "should layout node with negative width" test) would
have different layout under the C/Java implementation to the JavaScript
implementation.

This was hidden because the C/Java transpilers filtered out any negative
instantiation values from the test suite. In effect, the negative value
tests weren't running on the C/Java implementation.

This patch removes the negative value filter from the transpiler, and
makes the isDimDefined definition consistent between the three
implementations.
vjeux added a commit that referenced this pull request Mar 22, 2015
Normalized C and Java definition of isDimDefined.
@vjeux vjeux merged commit a7a8d1d into react:master Mar 22, 2015
@vjeux

vjeux commented Mar 22, 2015

Copy link
Copy Markdown
Contributor

Good catch! Thanks for sending this pull request :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants