Content box

[arcanis]

Now that percent unit support has been added to the library, I was wondering if you've considered exposing the content box on top of the element box. What do you think? It currently seems hard to know the exact size of the padding inside the box.

[emilsjolander]

I would prefer to only support border-box sizing. Can you give an example of what is hard the achieve with the current model?

[arcanis]

Oh, sure, I completely agree with using border-box. My point was that because units can now be percents, some values need to be resolved one way or the other to be added or substracted to others. For example, let's say I want to get the content width of an element like this:

<div style="width: 500px; padding-left: 5%; padding-right: 10%;" />

I currently need to apply the resolve logic myself:

let contentWidth = myDiv.getComputedWidth();
contentWidth -= myDiv.getPadding(EDGE_LEFT).value / myDiv.getParent().getComputedWidth() * 100;
contentWidth -= myDiv.getPadding(EDGE_RIGHT).value / myDiv.getParent().getComputedWidth() * 100;

Which means that I have to embed some layout logic inside my code. Thankfully, most of those relative properties (margin & padding) all depends on the parent width, otherwise it would have required extra steps to detect whether we should use the parent width or height.

To give you a better idea of what I mean, here is a function I've been using when experimenting with Yoga. I directly added it into the Javascript port, but it could also be implemented inside the library itself (note that it also automatically resolve EDGE_HORIZONTAL into EDGE_LEFT + EDGE_RIGHT, and EDGE_VERTICAL into EDGE_TOP + EDGE_BOTTOM):

for (let fnName of [ `Margin`, `Border`, `Padding` ]) {

	let method = lib.Node.prototype[`get${fnName}`];

	if (method == null)
		throw new Error(`Assertion failed; get${fnName} seems meesing`);

	patch(lib.Node.prototype, `getComputed${fnName}`, function getComputed(_, edge) {

		if (edge === constants.EDGE_HORIZONTAL)
			return getComputed.call(this, _, constants.EDGE_LEFT) + getComputed.call(this, _, constants.EDGE_RIGHT);

		if (edge === constants.EDGE_VERTICAL)
			return getComputed.call(this, _, constants.EDGE_TOP) + getComputed.call(this, _, constants.EDGE_BOTTOM);

		let value = method.call(this, edge);

		if (typeof value === `number`)
			return value;

		switch (value.unit) {

			case constants.UNIT_UNDEFINED: {
				return constants.UNDEFINED;
			} break;

			case constants.UNIT_PIXEL: {
				return value.value;
			} break;

			case constants.UNIT_PERCENT: {
				let relativeSize = this.getParent() ? this.getParent().getWidth() : 0;
				return value.value * relativeSize / 100;
			} break;

		}

	});

}

[emilsjolander]

Ah ok this has recently been added to the C version. Seems like the javascript wrapper is missing just 👍 https://github.com/facebook/yoga/blob/master/yoga/Yoga.h#L200 gets the computed layout padding after calculation.

[emilsjolander]

@arcanis does this look like what you need?

[arcanis]

Sounds like it! 👍 How would you feel about adding border & margin counterparts, and the EDGE_HORIZONTAL & EDGE_VERTICAL support? Can I open a PR with these changes?

[emilsjolander]

I would be fine with border and margin support. I'm not sure about EDGE_HORIZONTAL & EDGE_VERTICAL support as i'm not certain what that would mean? If you get the horizontal padding of a node with 5 in left padding but 10 in right padding then what should it return?

[arcanis]

It would return 15, the sum of left + right. So computing the content width would work like this:

let contentWidth = node.getComputedWidth();
contentWidth -= node.getComputedBorder(EDGE_HORIZONTAL);
contentWidth -= node.getComputedPadding(EDGE_HORIZONTAL);

Instead of:

let contentWidth = node.getComputedWidth();
contentWidth -= node.getComputedBorder(EDGE_LEFT);
contentWidth -= node.getComputedBorder(EDGE_RIGHT);
contentWidth -= node.getComputedPadding(EDGE_LEFT);
contentWidth -= node.getComputedPadding(EDGE_RIGHT);

[emilsjolander]

I don't think we should add that as it works differently than setting horizontal padding (setMargin(EDGE_HORIZONTAL, 10) sets 10 on both sides) and it is easy enough to get left and right and add them together 👍 .

[arcanis]

Ok :) How do you feel about implementing YGNodeGetBorder if we don't support additional edge values? Because the borders are unitless, it would just be an alias to YGStyleGetBorder. I think that this alias should be implemented, because even if those functions have the same return value they won't have the same semantic, but it might be controversial. What do you think?

[emilsjolander]

@arcanis I agree. Please split it up in two PRs though.