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 UniformGridLayout trying to scroll to first item #2969

Merged

Conversation

marcelwgn
Copy link
Contributor

@marcelwgn marcelwgn commented Jul 22, 2020

Description

The issue is that we didn't managed to properly recycle the first item, even though the UniformGridLayout was the owner. That resulted in the ElementManager trying to move ownership to realize that it isn't the actual owner, which resulted in the crash.

The behavior changed to only cache the item's desired size (which is the only information we needed), and let the ElementManager take care of managing.

Motivation and Context

Closes #535 and closes #2834

How Has This Been Tested?

Add API test for #2834, tested #535 manually (code not included in this PR)

Screenshots (if appropriate):

@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label Jul 22, 2020
@marcelwgn
Copy link
Contributor Author

Also, does #2812 have the green light? During this PR, I wish there was some form of documentation on how those areas work together as the stacktrace can be quite long and go through different areas of the ItemsRepeater control.

if (const auto firstElement = context.GetOrCreateElementAt(0, winrt::ElementRealizationOptions::ForceCreate))
{
firstElement.Measure(availableSize);
hasCachedSize = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

hasCachedSize [](start = 20, length = 13)

If size change after the first measure, we will not update the size of all the items. This is a change in behavior with this change because of the caching here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed the code now to always realize if FlowLayout did not do that. Is there anything else we need to consider here?

@StephenLPeters StephenLPeters added area-ItemsRepeater team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Jul 22, 2020
Copy link
Contributor

@StephenLPeters StephenLPeters left a comment

Choose a reason for hiding this comment

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

:shipit:

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@KanniyappanP
Copy link

KanniyappanP commented Jan 12, 2021

I used ItemsRepeater with UniformGridLayout as layout and I faced the same issue as mentioned below in Winui 3 preview 3.

  1. Item 0 disappeared when scroll up and scroll down the items.
  2. When using column spacing and row spacing in uniform grid layout, below crash occurred

"Itemsrepeater's child not found in its children collection."

Is this fix reflected in the Winui 3 preview 3 release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-ItemsRepeater team-Controls Issue for the Controls team
Projects
None yet
5 participants