Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix measure inner dimensions #1114

Closed

Conversation

kinarobin
Copy link
Contributor

@kinarobin kinarobin commented Dec 17, 2021

YGFloatIsUndefined(collectedFlexItemsValues.totalFlexGrowFactors) && collectedFlexItemsValues.totalFlexGrowFactors == 0 is not reachable.

yoga/Yoga.cpp Outdated
Comment on lines 2956 to 2958
collectedFlexItemsValues.totalFlexGrowFactors) ||
collectedFlexItemsValues.totalFlexGrowFactors == 0) ||
(YGFloatIsUndefined(node->resolveFlexGrow()) &&
(YGFloatIsUndefined(node->resolveFlexGrow()) ||
Copy link
Contributor

Choose a reason for hiding this comment

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

Guess those extra parentheses are redundant, now that it's a list of OR clauses... granted, the expression is quite hard to unpack visually, and extra parentheses don't help here, quite an opposite

Copy link
Contributor

@rshest rshest Oct 7, 2022

Choose a reason for hiding this comment

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

I actually believe that the original intent there may have been to have it like this instead:

if (!node->getConfig()->useLegacyStretchBehaviour &&
            ((!YGFloatIsUndefined(
                  collectedFlexItemsValues.totalFlexGrowFactors) &&
              collectedFlexItemsValues.totalFlexGrowFactors == 0) ||
             (!YGFloatIsUndefined(node->resolveFlexGrow()) &&
              node->resolveFlexGrow() == 0))) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, you're right. I had update this code.

Copy link
Contributor

@rshest rshest left a comment

Choose a reason for hiding this comment

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

Please check my comment, the code was wrong there, but I believe the intent was to check if not undefined and then compare with zero for both values.

@kinarobin kinarobin force-pushed the fix-flex-inner-main-dimension-measure branch from 6be2fbf to 9f85924 Compare October 8, 2022 03:37
@facebook-github-bot
Copy link
Contributor

@rshest has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot pushed a commit to facebook/litho that referenced this pull request Oct 11, 2022
Summary:
`YGFloatIsUndefined(collectedFlexItemsValues.totalFlexGrowFactors) &&            collectedFlexItemsValues.totalFlexGrowFactors == 0`  is not reachable.

X-link: facebook/yoga#1114

Reviewed By: NickGerleman

Differential Revision: D40203817

Pulled By: NickGerleman

fbshipit-source-id: 46a8c19dc0e097f4c28e7f48135f0382a557764a
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Oct 11, 2022
Summary:
`YGFloatIsUndefined(collectedFlexItemsValues.totalFlexGrowFactors) &&            collectedFlexItemsValues.totalFlexGrowFactors == 0`  is not reachable.

X-link: facebook/yoga#1114

Reviewed By: NickGerleman

Differential Revision: D40203817

Pulled By: NickGerleman

fbshipit-source-id: 46a8c19dc0e097f4c28e7f48135f0382a557764a
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
Summary:
`YGFloatIsUndefined(collectedFlexItemsValues.totalFlexGrowFactors) &&            collectedFlexItemsValues.totalFlexGrowFactors == 0`  is not reachable.

X-link: facebook/yoga#1114

Reviewed By: NickGerleman

Differential Revision: D40203817

Pulled By: NickGerleman

fbshipit-source-id: 46a8c19dc0e097f4c28e7f48135f0382a557764a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants