Skip to content

Fix WrapPanel stretching behavior for last item #704

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

AndrewKeepCoding
Copy link
Contributor

@AndrewKeepCoding AndrewKeepCoding commented Jul 17, 2025

Added a conditional check to prevent the last item in the WrapPanel from stretching when the parent measure's width is infinite.

Fixes

Fixes #703 and #297

PR Type

What kind of change does this PR introduce?

  • Bugfix

What is the current behavior?

THIS WORKS:

<Grid ColumnDefinitions="*">
    <controls:WrapPanel StretchChild="Last">
        <Button HorizontalAlignment="Stretch" Content="Item 1" />
        <Button HorizontalAlignment="Stretch" Content="Item 2" />
    </controls:WrapPanel>
</Grid>
image

THIS THROWS:

This code tries to stretch its last item to an infinity width causing System.Runtime.InteropServices.COMException.

<Grid ColumnDefinitions="Auto">
    <controls:WrapPanel StretchChild="Last">
        <Button HorizontalAlignment="Stretch" Content="Item 1" />
        <Button HorizontalAlignment="Stretch" Content="Item 2" />
    </controls:WrapPanel>
</Grid>

What is the new behavior?

Doesn't stretch its last item when given an infinity width.

image

PR Checklist

Please check if your PR fulfills the following requirements:

  • Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
  • Based off latest main branch of toolkit
  • Tested code with current supported SDKs
  • New component
    • Documentation has been added
    • Sample in sample app has been added
    • Analyzers are passing for documentation and samples
    • Icon has been created (if new sample) following the Thumbnail Style Guide and templates
  • Tests for the changes have been added (if applicable)
  • Header has been added to all new source files
  • Contains NO breaking changes

Other information

Added a conditional check to prevent the last item in the WrapPanel from stretching when the parent measure's width is infinite.
@michael-hawker
Copy link
Member

Thanks Andrew, I think a screenshot would have helped me understand the context too.

If I understand correctly, in the case where this is happening, all the items are going to be laid out on one row, as it'll never wrap due to the infinite available width, right? Therefore, yes it makes sense that the last item should just be whatever size it wants vs. trying to fill that void.

Is there a case where that wouldn't be true?

Even if in the measure pass it's infinite and doesn't stretch, if the arrange pass is fixed, then it should self-correct and still stretch when laying out, right?

It could be good to add a test here too just to catch this case so we can ensure we don't regress if we modify this logic in the future.

Thanks for taking a look into this! 🦙❤️

@michael-hawker michael-hawker requested a review from Copilot July 17, 2025 19:23
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a WrapPanel stretching behavior issue where the last item would attempt to stretch to infinite width when the parent container provides infinite measure constraints, causing a System.Runtime.InteropServices.COMException.

  • Added a conditional check to prevent stretching when parent measure width is infinite
  • Resolves crashes in scenarios where WrapPanel is placed in auto-sized containers

@@ -219,7 +219,8 @@ void Arrange(UIElement child, bool isLast = false)
}

// Stretch the last item to fill the available space
if (isLast)
// if the parent measure is not infinite
if (isLast && double.IsInfinity(parentMeasure.U) is false)
Copy link
Preview

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

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

[nitpick] Using 'is false' pattern is less readable than the negation operator. Consider using '!double.IsInfinity(parentMeasure.U)' for better readability and consistency with common C# practices.

Suggested change
if (isLast && double.IsInfinity(parentMeasure.U) is false)
if (isLast && !double.IsInfinity(parentMeasure.U))

Copilot uses AI. Check for mistakes.

Copy link
Member

Choose a reason for hiding this comment

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

Oops, I accidentally hit this button... interesting.

@AndrewKeepCoding
Copy link
Contributor Author

AndrewKeepCoding commented Jul 18, 2025

Hi @michael-hawker. Thanks for the feedback. 🙂

I think a screenshot would have helped me understand the context too.
Sorry about this. I just added a couple of screenshots. 😅

If I understand correctly, in the case where this is happening, all the items are going to be laid out on one row, as it'll never wrap due to the infinite available width, right? Therefore, yes it makes sense that the last item should just be whatever size it wants vs. trying to fill that void.
Is there a case where that wouldn't be true?

This PR just skips the last item stretching if the given space, width or height depending on the WrapPanel's `Orientation, is infinity. It'll still wrap if there's not enough space.

Even if in the measure pass it's infinite and doesn't stretch, if the arrange pass is fixed, then it should self-correct and still stretch when laying out, right?

Yes. The UpdateRows() method is also called on arrange.

It could be good to add a test here too just to catch this case so we can ensure we don't regress if we modify this logic in the future.

I'm working on this.

@AndrewKeepCoding
Copy link
Contributor Author

@michael-hawker I added a simple test case, and it passes successfully with the fix. Without the fix, the test doesn't technically "fail". It breaks the test host itself. This is the error I'm getting:

The active test run was aborted. Reason: Unable to communicate with test host process.

I already tried handling this at App.xaml but didn't help.

public App()
{
    this.InitializeComponent();
    UnhandledException += (sender, e) =>
    {
        // Confimed that the exception (System.Runtime.InteropServices.COMException)
        // is handled.
        e.Handled = true; 
    };
}

I don't think that this behavior is related to the WrapPanel.

Copy link

@Jay-o-Way Jay-o-Way left a comment

Choose a reason for hiding this comment

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

Hey @AndrewKeepCoding, I see you use the term "infinite width", but * does not mean "infinite". It means "stretch, using equal space" and that can be seen by something like <Grid ColumnDefinitions="1*, 3*">. This means: the second column must be three times the size of the first. In other words: two columns, 25% and 75% width of the total. When you have only one column, and one child element, that means the Grid is stretched horizontally, so the child is actually correctly stretching its last item.

Notes: Auto means the minimum size needed (determined by the children) and notice the pretty rounded corners on the right hand side? Nothing is being clipped.

@AndrewKeepCoding
Copy link
Contributor Author

Hi @Jay-o-Way! Thanks for the clarification 🙂 By “infinite width,” I’m specifically referring to the availableSize.Width argument passed to the WrapPanel’s MeasureOverride() method. In some layouts, such as when the panel is placed inside a Grid with Auto column width, this value can be double.Infinity. When that happens, the last item tries to stretch across an infinite space, which leads to a System.Runtime.InteropServices.COMException. This PR introduces a guard to skip that stretching behavior when infinite space is detected. Sorry for the confusion.

@Jay-o-Way
Copy link

@AndrewKeepCoding yep, you know a lot more about this than I do.
However, I do want to say that - from my perspective - I'm not sure if this is the best solution. I mean, given these two properties:

<Grid ColumnDefinitions="*">
    <controls:WrapPanel StretchChild="Last">

combined with your first screenshot, the result IS exactly what I would expect it to be: the grid is stretched, and the last item stretches pretty inside the available space. The result you introduce with this PR is that the last element does not stretch, and that would seem like a new bug (for future developers who don't know about this history).

Obviously, the runtime exception is something to solve, but I don't think this is the best result.

@AndrewKeepCoding
Copy link
Contributor Author

The result you introduce with this PR is that the last element does not stretch, and that would seem like a new bug (for future developers who don't know about this history).

I agree. But I couldn't find a good way to stretch the last item on this case. Interestingly, even the Slider control (which stretches by default) only takes its minimal space when placed in an Auto column.

STRETCHES:

<Grid ColumnDefinitions="*">
    <Slider />
</Grid>
image

DOES NOT STRETCH:

<Grid ColumnDefinitions="Auto">
    <Slider />
</Grid>
image

@Jay-o-Way
Copy link

even the Slider control (which stretches by default) only takes its minimal space when placed in an Auto column

As long a any control does not have a minimum size, that is to be expected.

@AndrewKeepCoding
Copy link
Contributor Author

As long a any control does not have a minimum size, that is to be expected.

Since both WrapPanel and TokenizedTextBox, like the Slider, default to MinWidth = 0 this behavior is not that weird, right? 😅

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.

WrapPanel with infinity width crashes trying to stretch its last item
3 participants