Fix divider strech

[bruno-rino]

Fixes #1529

Only dividers inside boxes with predefined size (like the the top-level box in a window) would stretch their child Dividers along their direction. In the other cases, a single 1x1 pixel (on macOS) would be drawn.

While I can't claim to fully grasp the pack layout code, a debug statement in _layout_node mentions that if node.width and node.intrinsic.width are falsy, the widget should "USE ALL AVAILABLE WIDTH" (

toga/src/core/toga/style/pack.py

Line 153 in b32e8bd

# self._debug("USE ALL AVAILABLE WIDTH", available_width)

).

But for this to be true in boxes without a predefined size, some code was missing. Those are my edits to the layout algorithm.
It also requires changing the widget implementation, to set intrinsic.width and intrinsic.height to 0 instead of at_least(...)

I only have a macOS, hence I only edited the cocoa widget. I did update the example code, so the other platforms implementation should be updated as well before merging these changes.

PR Checklist:

• All new features have been tested
• All new features have been documented
• I have read the CONTRIBUTING.md file
• I will abide by the code of conduct

[freakboy3742]

Can't argue that this has the desired effect for the specific case you've provided, but I'm not comfortable with it as a general solution. It may look like an innocuous change, but it's fairly radical - you're adding a specific interpretation of "intrinsic height = 0" (and similarly for width). Furthermore, you're using this interpretation after the pass computing the height of the widget has (theoretically) been completed; the logic you're altering should only be setting the position of the child inside the parent, not changing the height of widgets.

I think your comments about using all available width aren't entirely accurate - in this case, the calculation of available width should be identical, because the "at_least" clause will result in available width being computed as the full available width (due to the max on L147).

I'm also unclear why we need to make a special case of "0" as a height (which is at the very least, misleading); why can't we use at_least() as a marker? At least at the declaration level, it would seem to make more sense.

I'm not fundamentally opposed to making this change if it is necessary, but we need to be careful about making such changes. If we do make this change, it's something that will also need a change in the algorithmic description in the docs, as well as a new collection of tests exercising this specific layout situation.

[codecov]

Codecov Report

Merging #1540 (e258c3e) into main (5f3eae4) will decrease coverage by 0.02%.
The diff coverage is 75.00%.

❗ Current head e258c3e differs from pull request most recent head efb50ef. Consider uploading reports for the commit efb50ef to get more accurate results

Impacted Files	Coverage Δ
src/core/toga/style/pack.py	79.26% <75.00%> (-0.32%)	⬇️

[bruno-rino]

The special case of "0" was derived from my interpretation of the code. I was actually hoping for "oh yeah, you're right, I had meant to do that".

But it was also partly because of my misunderstanding of at_least(). And with further digging, I noticed the same misbehaviour with a TextInput not stretching even though flex=1 was set on it.
This code show it off: https://gist.github.com/bruno-rino/b5902f8e65839813fbf154572da7c2dc
The button on the left side won't stretch even though I set flex=1. The button on the right does, even though I didn't set flex=1, but its container has a predetermined size (200px)

So here is my updated solution. Gone is the special case of "0". Instead the check is:
child.style.flex or type(child.intrinsic.width) is at_least

But the location of the code has to be there. The error happens when the widgets container does not have a predetermined width or height. That is calculated in step 3. Only then can the children be stretched.

[freakboy3742]

I'm not convinced the example you've provided is a bug.

You're correct that the first box doesn't set width or flex (L17). That means it defaults to flex=0.

The third box has a fixed width; it's children expand to fill the available space in the parent.

The second box is the only flexible element in the row, so it absorbs all available space in the row. The first box has flex=0, so it gets a flex allocation of 0, and is thus assigned the minimum intrinsic width - the width of the label.

If you add flex=1 to the first box, it will have a non-zero flex allocation, and so the width of the first box will the maximum of the minimum intrinsic width and the flex allocation, so the TextInput will expand to the available space in the box.

Regarding the updated check - isinstance(obj, at_least) is a better way to express what you're saying there; but even then, you'll notice there isn't a single isinstance check in the entire Pack implementation. Instead, we look for a .value attribute, and catch the AttributeError if that attribute doesn't exist. There's also a need to check for the case when at_least specifies a value greater than the space that is about to be allocated.