Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(fillability): Don't allow shrinking of non-fill items in fillable containers #391

Merged
merged 2 commits into from
Aug 1, 2023

Conversation

gadenbuie
Copy link
Member

@gadenbuie gadenbuie commented Aug 1, 2023

Fixes #370

As currently implemented, both fill items and non-fill items will compete for vertical space in the fillable container's flex context. In the screenshot below (derived from the example in #370), the fillable container has a height of 300px and it contains two children with a set height of 250px (so together they would otherwise overflow the container were it not fillable [flex]). The first child is a fill item and the second child is not. In the current implementation, they equally share the vertical space in the fillable container, thanks to browser defaults of flex-shrink: 1 allowing overflowing items to shrink in display: flex.

image

This PR changes non-fill item children of fillable containers to have flex-shrink: 0, meaning that they'll use their initial height. The non-fill item in this scenario now has a height of 250px and the fill item shrinks as expected to fill the remaining space.

In general, users should set min-height and max-height to constrain fill items. This PR doesn't interfere with that advice, but it does mean that setting height is now the preferred method for setting the height of a non-fill item. Here's another example, similar to the one above, where the fillable container contains a fill item with min-height of 35px, a non-fill item with height: 250px and a fill item without an intrinsic height. Currently, all three share equal space, but in this PR the first fill item uses 35px, the non-fill item uses 250px and the unbounded fill item uses the remaining 15 pixels.

image

Another weird issue this PR avoids is that currently, because the non-fill item is allowed to shrink, its final height is proportional to the overall height of all of the flex children before shrinking. In other words, if we request a height of 100px for the non-fill item, it currently will have a final height of 50px (100 / (250 + 100 + 250) * 300). After this PR, the non-fill item has the requested 100px height and the remaining 200 pixels are shared between the fill items.

image

@gadenbuie gadenbuie merged commit 8189d98 into main Aug 1, 2023
@gadenbuie gadenbuie deleted the feat/html-non-fill-item branch August 1, 2023 20:55
@gadenbuie gadenbuie requested a review from jcheng5 August 2, 2023 17:18
Copy link
Member

@jcheng5 jcheng5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Sorry, I misunderstood and thought this was going to be more complicated than it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Non-fill tags shrink inside a fill container context
3 participants