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

Uses the sum of border and padding if it's greater than the container definite size #651

Closed
wants to merge 1 commit into from

Conversation

julia-script
Copy link
Contributor

@julia-script julia-script commented May 7, 2024

Objective

Match the flex container behavior with padding and border values larger than the container

Context

I couldn't find it in the flexbox spec, but I noticed that all major browsers implement this behavior

When the sum of padding and border is larger than the definite size of the flex container, it uses that as it's definite size

https://codepen.io/juliascript/pen/WNBeoev

Yoga behaves the same in the case of padding, but it ignores border

Right now Taffy behavior seems to be that in a ComputeSize run mode pass, it doesn't consider padding and border at all when styled_based_known_dimensions can be derived, while in a PerformLayout run it does account for that in the cross axis but not in the main axis

Feedback wanted

I added a simple test case, I'm not sure that's the right place

Edit:
Apparently the block layout implements this behavior so I imagine it comes from the block layout model

intrinsic_width.maybe_clamp(min_size.width, max_size.width).maybe_max(Some(padding_border_size.width))

@alice-i-cecile
Copy link
Collaborator

Adopted in #655 <3 If you'd like, a review there would be helpful :) Thank you very much for tackling this.

@nicoburns
Copy link
Collaborator

@julia-script I've gone ahead and merged #655 because I want to put out a release. But I wanted to echo Alice's thanks to you for raising this issue and submitting a fix. It is really helpful, and I've made sure that you are credited as an author of the new PR which was directly based on this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants