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

FlexPanel enhancements (grow, shrink, basis, fixes) #45

Merged
merged 5 commits into from
Nov 21, 2023

Conversation

Athari
Copy link
Contributor

@Athari Athari commented Nov 13, 2023

Several enhancements to FlexPanel:

  1. Fixed implementation of justify-content and align-content to follow the correct spacing calculation like in HTML. The original implementation was off in case of non-zero row and column gaps. Frankly, all variations looks kinda messy, but one is at least the standardized mess.

  2. Fixed implementation of align-content in case of items overflowing out of available panel size. In HTML, it falls back to three basic cases.

  3. Implemented flex-shrink and flex-grow. It's a very basic implementation which performs all calculations in the arrange stage. It covers the basic case of stretching items in wrap and no-wrap modes, as well as shrinking in no-wrap mode.

Proper flexbox implementation with fully functional flex-grow and flex-shrink would require support for flex-basis, much more sophisticated measure stage to handle min/max width/height constraints, resizing across main axis affecting cross axis and stuff like this. I don't understand HTML's and Avalonia's layout systems enough to properly match everything, so only basic implementation is included.

Maybe I'll implement flex-basis later. Using it as percent/absolute main axis constraint at the measure stage sounds potentially useful (?).

If anybody is curious, here's the official specification of the flex layout algorithm.

P.S. Is there any reason to use RowSpacing and ColumnSpacing property names instead of RowGap and ColumnGap? This seems to be the only mismatch with HTML flexbox naming. [EDIT] I see, there's Spacing in StackPanel.

@Athari
Copy link
Contributor Author

Athari commented Nov 13, 2023

@dotnet-policy-service agree

@Athari
Copy link
Contributor Author

Athari commented Nov 14, 2023

Added support for flex-basis and some other features:

  1. FlexBasis="Auto" is treated similar to flex-basis: auto / flex-basis: content (size to content, default behavior).

  2. FlexBasis="256" is treated similar to flex-basis: 256px (similar to width but can be affected by flex-shrink and flex-grow).

  3. FlexBasis="42%" is treated similar to flex-basis: 42% (percent of container's main axis size).

  4. When the sum of flex-grow of items in flex line is less than 1.0, distribute only a portion of free main axis space.

  5. HorizontalAlignment and VerticalAlignment are treated similarly to corresponding margin-x: auto (free main axis space remaining after applying grow and shrink factors is distributed evenly among all "auto-margins" along main axis).

  6. Grow and shrink factors can affect cross size now.

Known issues:

  1. MinWidth, MaxWidth, MinHeight, MaxHeight constraints don't function like main and cross axis constraints in the Flexbox standard and don't affect arrangement of other items in a flex line. With the way layout system in Avalonia works, I don't see a straightforward way to hook into measuring to see what constraints were applied.

    The standard algorithm of resolving flexible lengths heavily relies on intricate knowledge of constraints and it's unclear how to untangle it. One way is to declare separate attached properties for flex constraints, but it sounds messy. Or maybe I'm just missing something.

  2. flex-basis supports the values min-content, max-content, fit-content in the standard which aren't supported (yet?). Would min-content and max-content correspond to Measure(Size.Empty) and Measure(Size.Infinity)? Would it make any sense?

  3. There seem to be rounding errors when there're lots of arranged items with grow or shrink factors.

  4. There may be errors in cases not covered by the test app. For example, currently arranging heavily relies on measuring so if parent arranges in a completely unrelated size, or measures without constraints, or something like this, it'll probably fail. I hven't really tested any cases like this.

@Athari Athari changed the title FlexPanel enhancements (grow, shrink, fixes) FlexPanel enhancements (grow, shrink, basis, fixes) Nov 20, 2023
@maxkatz6
Copy link
Member

flex-basis supports the values min-content, max-content, fit-content in the standard which aren't supported (yet?). Would min-content and max-content correspond to Measure(Size.Empty) and Measure(Size.Infinity)? Would it make any sense?

It looks like min-content is similar to Left/Right/Center horizontal alignment, when fit-content is a Stretch alignment in XAML. max-content looks like Measure(Size.Infinity) for sure. Not sure if min-content should be Measure(Size.Empty) though, needs to be tested.

@maxkatz6
Copy link
Member

MinWidth, MaxWidth, MinHeight, MaxHeight constraints don't function like main and cross axis constraints in the Flexbox standard and don't affect arrangement of other items in a flex line

Yes, Min/Max constraints are used on the visual element directly, and parent Panel can't really redefine this behavior. It only can read this property and add side effects.

Copy link
Member

@maxkatz6 maxkatz6 left a comment

Choose a reason for hiding this comment

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

Thank you!
I feel like this PR is already good enough, and other problems with flex-basis and min/max can be addressed later. Or at least flex-basis.
In general, I feel like unit tests would be a great addition to this PR as well, as flex panel lacks them, but let's merge it as it is.

@maxkatz6 maxkatz6 merged commit c4e1144 into AvaloniaUI:main Nov 21, 2023
3 checks passed
@Athari
Copy link
Contributor Author

Athari commented Nov 21, 2023

In general, I feel like unit tests would be a great addition to this PR as well, as flex panel lacks them, but let's merge it as it is.

Browsers contain tons of flexbox tests. This seems to be the source (of some of them?):
https://test.csswg.org/suites/css-flexbox-1_dev/nightly-unstable/ (BSD)
https://test.csswg.org/suites/css-flexbox-1_dev/nightly-unstable/xhtml1/reftest-toc.xht
The main problem is that tests are in the form of "HTML with flexbox in CSS" and "HTML without flexbox in CSS" as a reference, with assumptions on fonts and various metrics, if I understand correctly.

I think it should be possible to convert the relevant subset of these tests to C#...

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.

None yet

2 participants