Aspect ratio box

[arthmis]

So there are some things to fix.

• First, I don't understand what the instrument macro is about so would like advice on how to add that to the widget impl.
• I left some comments in the code because I wasn't too sure what I should do for certain things.
• The last test I wrote I have a comment on an edge case I'm not too sure what the preferred default is.
• I still have to add the change to the changelog.
• Also I think it would nice to have a ConstrainedBox to pass in arbitrary constraints to children for testing.
• This has only been tested on Windows 10.

I think I will need to make some changes after reading about Flutter's constraints model.

[cmyr]

Thanks!

Some preliminary notes, but before I dig in much more, have you looked at the BoxConstraints::constrain_aspect_ratio method? I'm not sure if it will do exactly what you need, but if not it is a very good example of how to write this sort of "16 different possible combinations of inputs" code in a reasonably clear way. :)

[cmyr]

I think instead of a new example I might try to add this to examples/layout.rs?

[arthmis]

I don't see how layout example is related? I feel like there should be an example that shows off the different types of box widgets, like it would have SizedBox, AspectRatio, ConstrainedBox, and any future box like widgets like UnconstrainedBox and so on. I feel like the layout example would let AspectRatio disappear in the noise of the other widgets. I do plan to add the other box widgets soon, like the FractionallySizedBox.

[SecondFlight]

After looking at both examples, I'm not sure this fits well in --example layout without reworking that example. I think it does make sense to have a comprehensive layout example, but I'm worried that the current example will become convoluted without a thoughtful rework, which I think is outside the scope of this PR.

[arthmis]

Some preliminary notes, but before I dig in much more, have you looked at the BoxConstraints::constrain_aspect_ratio method? I'm not sure if it will do exactly what you need, but if not it is a very good example of how to write this sort of "16 different possible combinations of inputs" code in a reasonably clear way. :)

Ok so I think I understand the issue. I assumed the aspect ratio in constrain_aspect_ratio was width / height. So when I used it I got wildly incorrect results. Is there a reason it is using height / width instead of width / height?

Also it seemed very complex? I found the implementation that Flutter is using and that doesn't handle nearly as many cases. There implementation is similar to mine, but simpler. I think when I simplify, with the comments you made, my code it will be similar to the flutter version.

[cmyr]

I don't know much about the other impl, but let's see if dodj has any suggestions?

[arthmis]

Ok so, I have two questions now. Should I create a new example that demonstrates the different box type widgets? The other is am I handling correctly infinite constraints and tight constraints?

[arthmis]

So I asked @derekdreery about his implementation. They should achieve the same things but his also allows "finding the closest possible aspect ratio that is possible". I think for this widget it isn't necessary to have that behavior. If it can't choose the appropriate size to fit the aspect ratio then it will choose the max constraints as its size.

[richard-uk1]

So I asked @derekdreery about his implementation. They should achieve the same things but his also allows "finding the closest possible aspect ratio that is possible". I think for this widget it isn't necessary to have that behavior. If it can't choose the appropriate size to fit the aspect ratio then it will choose the max constraints as its size.

Yes there are a lot of subtly different choices the user could make. Its good to give multiple options I think.

[cmyr]

Sorry, I've been distracted the past little while. What's the status of this? What can I do to help get it finished?

[arthmis]

Well I think you can review the layout function again to make sure I'm doing what's expected for box constraints. I think you said that child's box constraints should be respected and the parent would clip if necessary? Otherwise this works correctly with respect to the Flutter version. The implementation is basically a copy.

Also I had posed the question of whether there should be an example that shows all the layouts and put the AspectRatioBox in there. Otherwise I will add it to the layout example.

[SecondFlight]

I looked through the layout function. Looks correct as far as I can tell 👍

[SecondFlight]

Looks good as long as CI passes. I didn't look at the unit tests but I trust your judgement there.

[arthmis]

For the warnings, I don't think I will add any more for now. I guess if it proves to be a problem then more checks can be added in the future. So if that's fine, then I think this should be ready.