Flexbox container isn't shrinking to be smaller than its content size + margins

iamralpht commented 1 month ago

`taffy` version

Platform

Rust

What you did

We use taffy for UI layout, and updating from a 0.3 release to 0.5.2 caused a regression in our render tests. It looks like some flex items that used to shrink to be smaller than the size of their content + margins (with flex-basis: 0), no longer do.

What went wrong

In the attached test case, I'm expecting that the box id "#outer" have its height controlled by grandchild "#inner" (so whatever "#inner"'s height is, plus margins). Instead, the box id "#outer" gets the height from box id "#large", which should be clipped instead.

Chrome performs the layout I'm expecting, and what we were seeing with taffy 0.3.

<html>
<style>
    div {
            box-sizing: border-box;
            border: 1px solid red;
    }
    body {
            margin: 20px;
    }
</style>

<body>
    <div
        style="display: flex; overflow-x: hidden; overflow-y: hidden; position: relative; top: 0px; left: 0px; bottom: 0px; right: 0px; width: 913px; height: 1228px; min-width: auto; min-height: auto; max-width: auto; max-height: auto; margin-top: 0px; margin-left: 0px; margin-bottom: 0px; margin-right: 0px; padding-top: 0px; padding-left: 0px; padding-bottom: 0px; padding-right: 0px; align-items: flex-start; align-self: normal; align-content: flex-start; justify-content: flex-start; gap: 0px 0px; flex-direction: row; flex-wrap: nowrap; flex-basis: auto; flex-grow: 0; flex-shrink: 0">
        <div id="outer"
            style="display: flex; overflow-x: hidden; overflow-y: hidden; position: absolute; top: 0%; left: 0%; bottom: auto; right: auto; width: auto; height: auto; min-width: auto; min-height: auto; max-width: auto; max-height: auto; margin-top: 30px; margin-left: 10px; margin-bottom: 0px; margin-right: 0px; padding-top: 8px; padding-left: 8px; padding-bottom: 8px; padding-right: 8px; align-items: flex-end; align-self: normal; align-content: flex-start; justify-content: center; gap: 48px 48px; flex-direction: column; flex-wrap: nowrap; flex-basis: auto; flex-grow: 0; flex-shrink: 0">
            <div
                style="display: flex; overflow-x: hidden; overflow-y: hidden; position: relative; top: 0px; left: 0px; bottom: 0px; right: 0px; width: auto; height: auto; min-width: auto; min-height: auto; max-width: auto; max-height: auto; margin-top: 0px; margin-left: 0px; margin-bottom: 0px; margin-right: 0px; padding-top: 0px; padding-left: 0px; padding-bottom: 0px; padding-right: 0px; align-items: flex-start; align-self: normal; align-content: flex-start; justify-content: flex-start; gap: 0px 0px; flex-direction: row; flex-wrap: nowrap; flex-basis: auto; flex-grow: 0; flex-shrink: 0">
                <div id="inner"
                    style="display: flex; overflow-x: hidden; overflow-y: hidden; position: relative; top: 0px; left: 0px; bottom: 0px; right: 0px; width: 544px; height: 251px; min-width: auto; min-height: auto; max-width: auto; max-height: auto; margin-top: 0px; margin-left: 0px; margin-bottom: 0px; margin-right: 0px; padding-top: 32px; padding-left: 32px; padding-bottom: 32px; padding-right: 32px; align-items: flex-start; align-self: normal; align-content: flex-start; justify-content: flex-start; gap: 0px 0px; flex-direction: column; flex-wrap: nowrap; flex-basis: auto; flex-grow: 0; flex-shrink: 0">
                </div>
                <div
                    style="display: flex; overflow-x: hidden; overflow-y: hidden; position: relative; top: 0px; left: 0px; bottom: 0px; right: 0px; width: 312px; height: auto; min-width: auto; min-height: 251px; max-width: auto; max-height: auto; margin-top: 0px; margin-left: 0px; margin-bottom: 0px; margin-right: 0px; padding-top: 0px; padding-left: 0px; padding-bottom: 0px; padding-right: 0px; align-items: flex-start; align-self: stretch; align-content: flex-start; justify-content: flex-start; gap: 0px 0px; flex-direction: column; flex-wrap: nowrap; flex-basis: auto; flex-grow: 0; flex-shrink: 0">
                    <div
                        style="display: flex; overflow-x: hidden; overflow-y: hidden; position: relative; top: 0px; left: 0px; bottom: 0px; right: 0px; width: auto; height: auto; min-width: 312px; min-height: 251px; max-width: auto; max-height: auto; margin-top: 0px; margin-left: 0px; margin-bottom: 0px; margin-right: 0px; padding-top: 32px; padding-left: 32px; padding-bottom: 32px; padding-right: 32px; align-items: flex-start; align-self: stretch; align-content: flex-start; justify-content: flex-start; gap: 16px 16px; flex-direction: column; flex-wrap: nowrap; flex-basis: 0px; flex-grow: 1; flex-shrink: 0">
                        <div id="large"
                            style="display: flex; overflow-x: hidden; overflow-y: hidden; position: relative; top: 0px; left: 0px; bottom: 0px; right: 0px; width: 170px; height: 394px; min-width: auto; min-height: auto; max-width: auto; max-height: auto; margin-top: 0px; margin-left: 0px; margin-bottom: 0px; margin-right: 0px; padding-top: 0px; padding-left: 0px; padding-bottom: 0px; padding-right: 0px; align-items: flex-start; align-self: normal; align-content: flex-start; justify-content: flex-start; gap: 0px 0px; flex-direction: row; flex-wrap: nowrap; flex-basis: auto; flex-grow: 0; flex-shrink: 0">
                        </div>
                    </div>
                </div>
            </div>
        </div>
    </div>
</body>

</html>

nicoburns commented 1 month ago

I've had an initial look into this. It seems that it's the min-height: 251px on the parent of #large that makes this work in Chrome (and, I believe, according to the spec). Of note (not to this particular tree, but in general): without a specified min-height (or min-width - whichever is in the main axis) style, the automatic minimum size would take affect and cause the stretching behaviour you don't want.

Quite why this isn't working in Taffy with the min-height style specified (which it is here) I'm not sure and will require further investigation.

nicoburns commented 1 month ago

Actually, EITHER a min-height or overflow: hidden makes this work properly in Chrome (which makes sense: overflow: hidden disables the automatic minimum main size for flexbox items.

iamralpht commented 1 month ago

Oh nice - thanks for investigating this! It took a little while to reduce this, but I can look at creating a gentest if it's helpful.

nicoburns commented 1 month ago

Oh nice - thanks for investigating this! It took a little while to reduce this, but I can look at creating a gentest if it's helpful.

Don't worry - I'm on that! I actually have one, but working on reducing it further.

nicoburns commented 1 month ago

I think it reduces down to:

<!DOCTYPE html>
<html lang="en">
<head>
  <script src="../../scripts/gentest/test_helper.js"></script>
  <link rel="stylesheet" type="text/css" href="../../scripts/gentest/test_base_style.css">
  <title>
    Test description
  </title>
</head>
<body>
    <div id="test-root" style="display: flex; flex-direction: column; width: 200px; ">
        <div style="display: flex; flex-direction: column; overflow: hidden; min-height: 100px; padding: 20px; flex-basis: 0px;">
            <div style="display: flex; height: 200px; flex-shrink: 0;"></div>
        </div>
    </div>
</body>

</html>

Which we might want to split into two tests (one for overflow: hidden and one for min-height: 100px as they both ought to work but neither seem to be working).

iamralpht commented 1 month ago

That makes sense - I think that overflow: scroll should behave as overflow: hidden, too (but I'm guessing they don't need separate tests).

DioxusLabs / taffy