-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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 iOS ListView cell gone hidden (#11287) #13669
Fix iOS ListView cell gone hidden (#11287) #13669
Conversation
Workaround iOS ListView cell disappear when user scroll back and DequeueReusableCell return a hidden cell
Hey there @ooikengsiang! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
@dotnet-policy-service agree |
@dotnet-policy-service agree |
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nit, we are trying to move away from '== null' and '!= null'. Thanks for the fix!
src/Controls/src/Core/Compatibility/Handlers/ListView/iOS/CellTableViewCell.cs
Outdated
Show resolved
Hide resolved
…TableViewCell.cs Co-authored-by: Manuel de la Pena <mandel@microsoft.com>
{ | ||
// set hidden to false doesn't work | ||
// force recreate of the whole cell instead | ||
reusableCell = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
won't this break all cell reuse?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, because those cell return that are not hidden are perfectly fine to reuse. Only those hidden are not. Also, I point out that setting hidden to false doesn't fix it. This may not be a perfect fix, but currently this fix at least make the ListView work correctly.
Azure Pipelines successfully started running 2 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ooikengsiang Not sure if I'm missing something but I think the wrong reproduction was uploaded to the original issue? https://github.com/ooikengsiang/ListViewProblem ?
Can you point me to a reproduction of the issue so that I can test your fix?
@PureWeen I have a repro here: #13633 which might be the same issue, it was also linked with the original issue. The only difference is that in my issue cells become hidden when app goes into background and come back while in the original issue its when scrolling through the list. But the result is the same, cells become hidden. |
Fixed, sorry for the mix up. |
👀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the main problem here is that the viewhandler on the ViewCellRenderer isn't being completely disconnected.
I've checked in an alternate approach here
https://github.com/dotnet/maui/tree/13669_testing
can you let me know how that works for you? if that seems to address the problem?
Let me check and will get back to you. |
FYI, I meant to say isn't being disconnected.
|
Test and it work! Awesome, never know that disconnect is required there. |
Thank you for testing @ooikengsiang and narrowing down to where this was being quirky! Closing this PR in favor of #15036 |
@ooikengsiang if you have time can you test my updates here? I think I was also breaking recycling with my changes :-) I tried your repro and a few of our samples and they seemed to work fine. |
Description of Change
Workaround iOS / MacOS ListView cell disappear when user scroll back and DequeueReusableCell return a hidden cell.
Force any hidden cell return by DequeueReusableCell which are hidden to create a new cell instead (set hidden to false doesn't work).
Issues Fixed
Fixes #11287