Get full box width of element

[JuanCaicedo]

I want to have a test case where I want to assert that an element takes up the full width of the parent minus the width of its sibling.

it('takes up full width minus the sibling', function() {
  first.assert({
    width: body.width.minus(second.width)
  });
});

However, this breaks when the second element has a margin, because margins are not included as part of the width.

.second {
  width: 114px;
  margin: 3px;
}

console.log(second.getRawPosition().width) // 114px

My solution involves manually factoring in the width of the padding, but it would be nice to have something along the lines of

it('takes up full width minus the sibling', function() {
  first.assert({
    width: body.width.minus(second.fullWidth)
  });
});

console.log(second.getRawPosition().fullWidth) // 120px

[JuanCaicedo]

There might be a better way to get this, right now I wrote myself a helper function

function getFullWidth(el) {
  var rightMargin = el.getRawStyle('margin-right');
  rightMargin = rightMargin.replace('px', '');
  rightMargin = parseInt(rightMargin);

  var leftMargin = el.getRawStyle('margin-right');
  leftMargin = leftMargin.replace('px', '');
  leftMargin = parseInt(leftMargin);

  return el.width.plus(rightMargin).plus(leftMargin);
}

This would also need to handle em instead of px to be more robust.

[jamesshore]

Looks good. This is definitely something worth adding. I'm not sure about the 'fullWidth' descriptor, though. Would body.width.minus(second).minus(second.marginWidth) be okay, or is that too wordy?

Also, I believe getRawStyle() converts everything to px, so em should already be handled for you. (It's using getComputedStyle() under the covers, which does the conversion for us.)

[jamesshore]

@JuanCaicedo BTW, there's a bug in your helper function: var leftMargin is getting margin-right. Not sure if that's in your real code or not, but thought you'd want to know. Otherwise, it looks correct.

[JuanCaicedo]

Copy paste error 😅

[JuanCaicedo]

@jamesshore marginWidth sounds good to me. I could see borderWidth and paddingWidth all being useful as well. What do you think of the equivalent height properties?

I can take a stab at implementing this and add any questions I have 😄

[jamesshore]

Agreed on padding. I'm not sure about borders as they have special cases, but it might be worth doing. I'd prefer to take each one as a separate pull request in any case.

Let's think this through a bit more first... eventually, when we do borders, we will want to be able to say things like border.color, border.style, border.cornerRadius, etc.

So, for consistency, perhaps what we need here is:

• element.margin.width
• element.margin.height
• element.margin.width.left, element.margin.width.right (or should it be margin.left.width?)
• element.margin.height.top, element.margin.height.bottom

What do you think? Am I missing anything?

[jamesshore]

Some more ideas for margin--would it be useful to talk about margin's positions? E.g., margin.left.left corresponds to the left edge of the left margin and margin.left.right corresponds to the right edge of the left margin?

If so, then we should probably use margin.left.width rather than margin.width.left.

[JuanCaicedo]

I think I lean towards being able to split border.right and border.left. I can imagine a case where your border is different on both sides and you want to test them separately. Then padding and margin would be the same for consistency.

I would then imagine that border.width is the sum of border.left.width and border.right.width

If we pursue the above, I'm not sure how to handle border.top.width or border.right.height

[jamesshore]

I'd say border.top.width and border.right.height are just undefined.

[jamesshore]

Although I'm not sure about border.width being the sum of border.left.width and border.right.width. If your CSS defines border-width: 3px then it applies equally to all sides. In that case border.left.width is 3px and border.right.width is 3px. Having border.width equal 6px would be confusing.

Perhaps it should be called border.totalWidth.

[JuanCaicedo]

That makes sense, I'm good with border.totalWidth and border.top.width == undefined

[JuanCaicedo]

My guess is that these, along with the logic for getting them, would need to be added in https://github.com/jamesshore/quixote/blob/master/src/q_element.js#L32?

[jamesshore]

The proper way to implement this is to create a new descriptor. This guide explains how.

[jamesshore]

Also, your descriptor should extend SizeDescriptor. The best example to follow is ElementSize.

[jamesshore]

@JuanCaicedo I've been thinking about margins. I think there might be a better option.

The problem with margins is that they're an implementation detail. The idea of Quixote is that it allows you to talk about the visual appearance of your page. From a visual perspective, margins don't exist, at least not in the same way that they do in CSS. (Borders and padding, in contrast, do have a direct correspondence in the page.)

Instead of margins, the viewer sees white space. Margins are a way of achieving white space, and because they collapse and combine there's no direct visual connection between the margin property in CSS and the white space seen on the page.

As a result, I think we might be better off providing ways to talk about white space rather than margins. Specifically, the distance between elements.

I'm proposing something like this: first.right.to(second.left). That would be the amount of space between the first and second elements.

Then, to talk about how two columns fill up your body element, you might say this:

// first and second fill up the entire body
body.assert({
  left: first.left,
  right: second.right,
  width: first.left.to(second.right)
});

// There's a ten-pixel gutter between first and second
// (This uses a new assertion API that I've been planning)
first.right.to(second.left).should.equal(10);

We might even want to provide a convenience API for this:

body.should.be.filled(first, 10, second);  // element, gutter, element

What do you think?

[JuanCaicedo]

I think I'm on board. I can picture this being useful in addition to the border stuff we're discussing above. For example, when I implemented the styles for the categories section of this page, it would have been nice to be able to make an assertion about how much space there was from the right edge of the text to the left edge of the border.

It seems like what would need to be implemented is a new function to on any positional descriptors (top, right, bottom, left). The result of this would eventually be a number, so it could be asserted on width and height.

[JuanCaicedo]

Are you writing keeping track of your thoughts for that new assertion API in an issue? I might like to drop some thoughts in as well 😃

[jamesshore]

Good idea. See #47.

[jamesshore]

Regarding the to() function, I'm planning to implement it on PositionDescriptor, just like plus() and minus() are. That should automatically make it available on all positional descriptors. It will return a SizeDescriptor (which I might rename), which is the same class width and height descriptors use.

[jamesshore]

@JuanCaicedo I've updated the Descriptor tutorial. The changes are mostly minor but you might find it useful.

[JuanCaicedo]

@jamesshore I've been thinking about this some more, and I'm not sure it will exactly work for the case I'm trying to test. If you think about a case like the one I show in this codepen, I think an assertion like the following would fail:

parent.assert({ 
  width: first.left.to(second.right) 
})

I think this would actually also fail if you manually factor in the width of the margin (since it takes up width on both the left and right.

parent.assert({ 
  width: first.left.to(second.right).plus(second.margin.width) 
})

To fully account for this, you would need to factor in both margins

parent.assert({ 
  width: first.left.to(second.right).plus(second.margin.left.width).plus(second.margin.right.width) 
})

This makes me think that to might not be as good of a solution as we were hoping. I agree with coming up with a way of testing the end result instead of the implementation details, but maybe we need to still think of what that looks like.

What are your thoughts?

[jamesshore]

@JuanCaicedo I'm not following. In plain English, what about that codepen are you trying to check? Is it the gap between the green box and the red box, or something else?

[JuanCaicedo]

The same as the example above, that the whole width of the parent is taken up by the two children

[jamesshore]

Except it's not, is it? There's a five-pixel gap between second and the right hand side of the parent. Unless I've misunderstood things?

|--------------------- parent -------------------|
|------- first -------| 5px |--- second ---| 5px |

first.left.should.equal(parent.left);
first.right.to.second.left.should.equal(5);
second.right.to.parent.right.should.equal(5);

// or maybe...
parent.should.be.filled(first, 5, second, 5)

[JuanCaicedo]

Yeah, I think we have the same understanding. I just don't think that this type of assertion should require knowledge about the margin, that seems like testing implementation details

[jamesshore]

I'm still confused--what knowledge of the margin is involved in the example I gave? There's knowledge that there's a gap, but that's a visual feature that presumably you'd want to test.

If you just want to test that the two elements fill the entire parent, with a five pixel gap on the right, you could do it this way:

parent.assert({
  width: first.left.to(second.right).plus(5)
});

Ultimately, the child elements don't fill the parent--there's that gap on the right--so any test that says they do should fail.

first.left.to(second.right).should.equal(parent.width);

// Outputs:
// Distance from left edge of 'first' to right edge of 'second' should be 5px larger.
// Expected: 200px (width of 'parent')
//  But was: 195px

[JuanCaicedo]

Okay, now it makes sense 😃 I was thinking that you'd want to avoid needing to specify 5 so that you're testing at a higher level, but your reasoning makes sense. You should have to specify that gap, because it's what the user is experiencing

[jamesshore]

PositionDescriptor.to() has been added and will be included in the 0.13 release.

[jamesshore]

Given that PositionDescriptor.to() was added a few years ago, the new assertion API described in issue #47 is coming soon with the v1.0 release, and issue #58 covers additional improvements to assertions, I'm going to consider this one closed.