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.

Sign up for GitHub

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

Jump to bottom

[iOS] Changing model property scrolls CollectionView back to top #7128

Closed
adrianknight89 opened this issue Aug 12, 2019 · 3 comments
Closed

[iOS] Changing model property scrolls CollectionView back to top #7128

adrianknight89 opened this issue Aug 12, 2019 · 3 comments
Assignees
@hartez
Labels
a/collectionview blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. p/iOS 🍎 t/bug 🐛
Projects
v4.3.0 Sprint 158

Comments

@adrianknight89
Copy link
Contributor

Description

There are actually two bugs in this repro.

Steps to Reproduce

Hard crash

  1. Run the repro
  2. Scroll up and down continuously until the app crashes
  3. If unable to reproduce, you should tap one of the labels and then scroll up and down

Scrolling

  1. Run the repro
  2. Scroll to the bottom of the screen
  3. Tap a label to change its text
  4. Observe that the CollectionView is scrolled back to top

Basic Information

  • Version with issue: 4.3.0.1032-nightly
  • IDE: VS2019
  • Affected Devices: XS

Reproduction Link

App144.zip
@adrianknight89 adrianknight89 added s/unverified New report that has yet to be verified t/bug 🐛 labels Aug 12, 2019
@pauldipietro pauldipietro added this to New in Triage Aug 12, 2019
@PureWeen
Copy link
Contributor

PureWeen commented Aug 14, 2019 •
edited

After it scrolls to the top it seems like it doesn't want to let you scroll all the way down anymore. If you use inertia to try and scroll all the way it'll just bounce and not scroll all the way to the bottom. If you force it to scroll all the way down then it'll latch on to the bottom row again

Here's the sample against the source
https://github.com/PureWeen/Xamarin.Forms.Sandbox/tree/fix_7128

@PureWeen PureWeen added this to To do (blockers) in CollectionView via automation Aug 14, 2019
@PureWeen PureWeen added the blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. label Aug 14, 2019
@PureWeen PureWeen removed this from New in Triage Aug 14, 2019
@PureWeen PureWeen added this to the 4.3.0 milestone Aug 14, 2019
@PureWeen PureWeen removed the s/unverified New report that has yet to be verified label Aug 14, 2019
@adrianknight89
Copy link
Contributor Author

adrianknight89 commented Aug 16, 2019 •
edited

@PureWeen I commented out the following line

Xamarin.Forms/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs

Line 233 in 502916d

Layout?.InvalidateLayout();

which seems to fix the hard crash and the scroll issues both, but I suppose layout invalidation is needed in other cases such as when content size changes (#5905)?

@adrianknight89
Copy link
Contributor Author

adrianknight89 commented Aug 16, 2019 •
edited

Also, this seems to make #7152 better though scroll performance still needs improvement. We need to make sure GetCell does minimal work and return as soon as possible.

@PureWeen PureWeen mentioned this issue Aug 17, 2019
Open
@adrianknight89 adrianknight89 mentioned this issue Aug 17, 2019
Open
hartez added a commit that referenced this issue Aug 26, 2019
@hartez
More intelligent cell resizing; fix infinite layout loop and crash;
ed2b0de 
Reuse template views instead of re-creating them every time;
Fixes #7128
@hartez hartez mentioned this issue Aug 27, 2019
Merged
4 tasks
@hartez hartez self-assigned this Aug 27, 2019
@hartez hartez moved this from To do (blockers) to In Progress in CollectionView Aug 27, 2019
@samhouts samhouts added this to In Progress in v4.3.0 Aug 27, 2019
@samhouts samhouts added this to To do in Sprint 158 via automation Aug 27, 2019
@samhouts samhouts moved this from To do to Ready for Review (Issues) in Sprint 158 Aug 27, 2019
@samhouts samhouts removed this from the 4.3.0 milestone Aug 29, 2019
hartez added a commit that referenced this issue Sep 2, 2019
@hartez
More intelligent cell resizing; fix infinite layout loop and crash;
68d9d3a 
Reuse template views instead of re-creating them every time;
Fixes #7128
@samhouts samhouts moved this from In Progress to Done in v4.3.0 Sep 9, 2019
@samhouts samhouts moved this from Ready for Review (Issues) to Done in Sprint 158 Sep 9, 2019
CollectionView automation moved this from In Progress to Done Sep 12, 2019
felipebaltazar pushed a commit to felipebaltazar/Xamarin.Forms that referenced this issue Oct 16, 2019
@hartez @felipebaltazar
[iOS] More intelligent cell resizing (xamarin#7285)
3f18107 
* More intelligent cell resizing; fix infinite layout loop and crash;
Reuse template views instead of re-creating them every time;
Fixes xamarin#7128

* Fix attribute ambiguity

* Drop trailing 'and'
felipebaltazar pushed a commit to felipebaltazar/Xamarin.Forms that referenced this issue Oct 16, 2019
@hartez @felipebaltazar
[iOS] More intelligent cell resizing (xamarin#7285)
a90e7eb 
* More intelligent cell resizing; fix infinite layout loop and crash;
Reuse template views instead of re-creating them every time;
Fixes xamarin#7128

* Fix attribute ambiguity

* Drop trailing 'and'
@samhouts samhouts removed this from Done in CollectionView May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hartez hartez

Labels
a/collectionview blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. p/iOS 🍎 t/bug 🐛
Projects
No open projects
Sprint 158
  
Done
v4.3.0
  
Done
Milestone
No milestone
Development

No branches or pull requests
5 participants
@hartez @PureWeen @jsuarezruiz @samhouts @adrianknight89