Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[iOS] Fix hard crash when horizontal GridViewLayout is refreshed #7612

Merged
merged 4 commits into from
Oct 4, 2019

Conversation

adrianknight89
Copy link
Contributor

@adrianknight89 adrianknight89 commented Sep 21, 2019

Description of Change

Not sure if this is the right fix. When horizontal GridViewLayout is refreshed via a pull gesture, the application crashes. Previously, the code was trying to calculate missing cell count based on layout span. If you run the attached test, you'll see that base.LayoutAttributesForElementsInRect(rect) returns 9 items even though there are 8 visible items on the screen. The code calculated missing cell count to be 2 even though it should be 1. Consequently, we ended up with an index out of bounds error.

The fix allocates UICollectionViewLayoutAttributes[] based on the number of items in section 0, copies existing attributes from base, and appends new ones.

@hartez Do we need to change the Wikipedia image URLs?

@hartez @jfversluis @samhouts @jsuarezruiz Also, when the loading spinner is shown, the CollectionView seems to be resized, which forces it to show 3 rows of items instead of 4 before the next batch of items is added to the items source. Can you confirm if this is a bug or expected behavior? (Tested on XS)

Issues Resolved

Testing Procedure

  • Run the test
  • Pull to refresh 4 times
  • Verify that the application does not crash

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

Copy link
Contributor

@hartez hartez left a comment

Choose a reason for hiding this comment

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

Looks fine as long as it passes the UI test for 6077.

@samhouts samhouts added the in-progress This issue has an associated pull request that may resolve it! label Oct 2, 2019
@jsuarezruiz
Copy link
Contributor

LGTM.

7642

@samhouts samhouts added approved Has two approvals, no pending reviews, and no changes requested and removed in-progress This issue has an associated pull request that may resolve it! labels Oct 2, 2019
@rmarinho rmarinho merged commit 32cb21a into xamarin:4.3.0 Oct 4, 2019
felipebaltazar pushed a commit to felipebaltazar/Xamarin.Forms that referenced this pull request Oct 16, 2019
…arin#7612) fixes xamarin#7593

* fix refreshview issue

* fix path

* remove extra line after rebase
felipebaltazar pushed a commit to felipebaltazar/Xamarin.Forms that referenced this pull request Oct 16, 2019
…arin#7612) fixes xamarin#7593

* fix refreshview issue

* fix path

* remove extra line after rebase
@samhouts samhouts added this to the 4.3.0 milestone Oct 17, 2019
@samhouts samhouts added i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often and removed i/critical labels Feb 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/collectionview approved Has two approvals, no pending reviews, and no changes requested i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often p/iOS 🍎 t/bug 🐛
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants