DaddyFrosty/yoga
1
0
Fork 0
Code Issues 96 Pull Requests 31 Actions Packages Projects Releases 33 Wiki Activity

Baseline support #317

Closed
woehrl01 wants to merge 33 commits from baseline-support into master
pull from: baseline-support
merge into: DaddyFrosty:master
Conversation 39 Commits 33 Files Changed 20 +1612 -6
16 changed files with 1612 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 3e5e01f6e1 - Show all commits

6
yoga/Yoga.c
View File

@@ -951,7 +951,7 @@ static inline YGDirection YGNodeResolveDirection(const YGNodeRef node,
}
static float YGBaselineOfFirstLine(const YGNodeRef node,
const YGFlexDirection mainAxis,
const YGFlexDirection crossAxis,
const float parentWidth) {
emilsjolander commented 2017-01-05 11:06:20 -08:00 (Migrated from github.com)
Review
Copy Link

This will always be height as baseline is undefined for column flex direction. right? in that case I think it is better to be clear and use YGDimensionHeight instead of dim[crossAxis]

This will always be height as baseline is undefined for column flex direction. right? in that case I think it is better to be clear and use `YGDimensionHeight` instead of `dim[crossAxis]`
emilsjolander commented 2017-01-05 11:07:40 -08:00 (Migrated from github.com)
Review
Copy Link

Is there a good reason to allow undefined return value for baseline? I would prefer asserting and crashing as it would probably indicate a bug. What do you think?

Is there a good reason to allow undefined return value for baseline? I would prefer asserting and crashing as it would probably indicate a bug. What do you think?
emilsjolander commented 2017-01-05 11:11:39 -08:00 (Migrated from github.com)
Review
Copy Link

Why child->lineIndex > 0? I understand that it skips multiline children but i'm not sure why.

Why `child->lineIndex > 0`? I understand that it skips multiline children but i'm not sure why.
emilsjolander commented 2017-01-05 11:12:21 -08:00 (Migrated from github.com)
Review
Copy Link

same as above regarding dim[crossAxis]

same as above regarding `dim[crossAxis]`
emilsjolander commented 2017-01-05 11:14:02 -08:00 (Migrated from github.com)
Review
Copy Link

The baseline of a container is defined by its first baseline aligned child, but when there is no baseline aligned child then the container's baseline is defined by it's last child. This seems odd. Are you sure we have test cases covering edge cases here to make sure this is how it works on the web?

The baseline of a container is defined by its first baseline aligned child, but when there is no baseline aligned child then the container's baseline is defined by it's last child. This seems odd. Are you sure we have test cases covering edge cases here to make sure this is how it works on the web?
emilsjolander commented 2017-01-05 11:14:19 -08:00 (Migrated from github.com)
Review
Copy Link

same as above regarding pos[crossAxis]

same as above regarding `pos[crossAxis]`
woehrl01 commented 2017-01-05 11:25:14 -08:00 (Migrated from github.com)
Review
Copy Link

I'm not sure my self about this. I thought this could be a kind of "feature", so if you return undefined, you simply use the nodes height. But crashing would be fine too for me. What I'm not sure about is if we explicitly add the padding-top here or if the implementation of the custom function needs to consider this.

I'm not sure my self about this. I thought this could be a kind of "feature", so if you return undefined, you simply use the nodes height. But crashing would be fine too for me. What I'm not sure about is if we explicitly add the padding-top here or if the implementation of the custom function needs to consider this.
woehrl01 commented 2017-01-05 11:26:59 -08:00 (Migrated from github.com)
Review
Copy Link

we use only the first line of the children for base layout alignment. At least this is how chrome handles it.

we use only the first line of the children for base layout alignment. At least this is how chrome handles it.
woehrl01 commented 2017-01-05 11:27:56 -08:00 (Migrated from github.com)
Review
Copy Link

no it's defined by its first child on the first line or the first baseline aligned child if there is one (one the first line). I'll add a test for this.

no it's defined by its first child on the first line or the first baseline aligned child if there is one (one the first line). I'll add a test for this.
emilsjolander commented 2017-01-05 11:40:10 -08:00 (Migrated from github.com)
Review
Copy Link

The custom function should not take padding into account. We don't expect this for the measure function so I would like to preserve that here if possible.

Let's crash for now. If we find a valid reason to have this feature we can implement it later.

The custom function should not take padding into account. We don't expect this for the measure function so I would like to preserve that here if possible. Let's crash for now. If we find a valid reason to have this feature we can implement it later.
emilsjolander commented 2017-01-05 11:41:06 -08:00 (Migrated from github.com)
Review
Copy Link

yes, but this looks like we are skipping over children with multiple lines instead of just looking at their first line?

yes, but this looks like we are skipping over children with multiple lines instead of just looking at their first line?
emilsjolander commented 2017-01-05 11:43:16 -08:00 (Migrated from github.com)
Review
Copy Link

So the current code is wrong i think. Right? As it will pick the last child in the case no child is baseline aligned?

So the current code is wrong i think. Right? As it will pick the last child in the case no child is baseline aligned?
woehrl01 commented 2017-01-05 11:46:36 -08:00 (Migrated from github.com)
Review
Copy Link

oh, yep. We should break instead of continue here!

oh, yep. We should break instead of continue here!
woehrl01 commented 2017-01-05 11:47:30 -08:00 (Migrated from github.com)
Review
Copy Link

no as we only use the child if baselineChild is still NULL. Which is false as seen as we find the first one. We still need to iterate to take any baseline aligned child into account.

no as we only use the child if ```baselineChild``` is still ```NULL```. Which is false as seen as we find the first one. We still need to iterate to take any baseline aligned child into account.
if (node->baseline != NULL) {
emilsjolander commented 2017-01-05 02:26:21 -08:00 (Migrated from github.com)
Review
Copy Link

uint32_t

uint32_t
emilsjolander commented 2017-01-05 02:26:32 -08:00 (Migrated from github.com)
Review
Copy Link

i++

i++
return node->baseline(node);
@@ -982,8 +982,8 @@ static float YGBaselineOfFirstLine(const YGNodeRef node,
node->style.flexDirection,
node->layout.measuredDimensions[YGDimensionWidth]);
if (YGFloatIsUndefined(baseline)) {
baseline = YGNodeLeadingPaddingAndBorder(baselineChild, mainAxis, parentWidth) +
baselineChild->layout.measuredDimensions[dim[mainAxis]];
baseline = YGNodeLeadingPaddingAndBorder(baselineChild, node->style.flexDirection, node->layout.measuredDimensions[YGDimensionWidth]) +
baselineChild->layout.measuredDimensions[dim[node->style.flexDirection]];
}
return baseline + baselineChild->layout.position[YGEdgeTop];