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

Fix crash with unbounded sizing for WrapLayout. #3330

Merged
merged 7 commits into from
Jun 6, 2020

Conversation

marcpems
Copy link
Contributor

@marcpems marcpems commented Jun 5, 2020

Also add some new stuff to the sample for easier manipulation.

Fixes #3327

#3327

PR Type

What kind of change does this PR introduce?

  • Bugfix

What is the current behavior?

WrapLayout throws when contained in a control with an infinite size in the axis opposite to the WrapLayout.

What is the new behavior?

It doesnt crash now. And there are some neat controls on the sample to help swap axis.

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tested code with current supported SDKs
  • Pull Request has been submitted to the documentation repository instructions. Link:
  • Sample in sample app has been added / updated (for bug fixes / features)
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Header has been added to all new source files (run build/UpdateHeaders.bat)
  • Contains NO breaking changes

Other information

Also add some new stuff to the sample for easier manipulation.
@ghost
Copy link

ghost commented Jun 5, 2020

Thanks marcpems for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

@ghost ghost assigned michael-hawker Jun 5, 2020
@michael-hawker michael-hawker added this to the 6.1 milestone Jun 5, 2020
@ghost ghost added the bug 🐛 An unexpected issue that highlights incorrect behavior label Jun 5, 2020
@michael-hawker
Copy link
Member

Not sure what the build failure is about...

@azchohfi
Copy link
Contributor

azchohfi commented Jun 5, 2020

    21>WrapLayout\WrapLayout.cs(258,132): error SA1028: Code should not contain trailing whitespace [D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.Layout\Microsoft.Toolkit.Uwp.UI.Controls.Layout.csproj]
    21>WrapLayout\WrapLayout.cs(268,145): error SA1106: Code should not contain empty statements [D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.Layout\Microsoft.Toolkit.Uwp.UI.Controls.Layout.csproj]
    21>WrapLayout\WrapLayout.cs(268,145): error SA1107: Code should not contain multiple statements on one line [D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls.Layout\Microsoft.Toolkit.Uwp.UI.Controls.Layout.csproj]

@marcpems
Copy link
Contributor Author

marcpems commented Jun 5, 2020

Fixed the Release compile issue


_wrapScrollParent = control.FindChildByName("WrapScrollParent") as ScrollViewer;
var wrapLayoutParent = _wrapScrollParent?.FindDescendant<ItemsRepeater>();
_wrapLayout = wrapLayoutParent.Layout as WrapLayout;
Copy link
Member

Choose a reason for hiding this comment

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

I hit an exception here, we should already have a reference to the repeater above, testing something out quick.

Copy link
Contributor

@Kyaa-dost Kyaa-dost left a comment

Choose a reason for hiding this comment

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

It's crashing for me as too. I think @michael-hawker is taking a look. Let's see what seems to be the problem.

Copy link
Contributor

@Kyaa-dost Kyaa-dost left a comment

Choose a reason for hiding this comment

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

🚀🚀🚀🚀

@marcpems marcpems merged commit 70ed20e into master Jun 6, 2020
@delete-merged-branch delete-merged-branch bot deleted the marcpe/wraplayout_infinite_container_crash branch June 6, 2020 07:46
grokys added a commit to AvaloniaUI/Avalonia that referenced this pull request Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modifying WrapLayout Orientation in Sample App Crashes
5 participants