Bring back some aspect of ConvertView on TableView and avoid AT_MOST Measure #20130
Conversation
PureWeen
requested review from
jfversluis and
rmarinho
and removed request for
a team
PureWeen
changed the title
|
iOS Failures are unrelated |
| @@ -107,7 +137,10 @@ public override SizeRequest GetDesiredSize(double widthConstraint, double height | |||
| continue; | |||
| } | |||
|
|
|||
| AView listItem = _adapter.GetView(i, null, Control); | |||
| var platformView = cell.Handler?.PlatformView as AView; | |||
There was a problem hiding this comment.
Should this be ToPlatform()? If we don't want to create, will it break if it is using a wrapper/container view? Or is that not possible with these views
There was a problem hiding this comment.
yea, not really possible with cells. Also, toPlatform doesn't really work here because we only want to return a PlatformView if it's already been created. This is just to fake the ConvertView behavior if the platformview already exists. Also, the cells only implement IElementHandler so they don't have a container view
src/Controls/src/Core/Compatibility/Handlers/TableView/Android/TableViewRenderer.cs
Outdated
Show resolved
Hide resolved
| var platformView = cell.Handler?.PlatformView as AView; | ||
| var convertView = (platformView?.Parent as AView) ?? platformView; | ||
|
|
||
| AView listItem = _adapter.GetView(i, convertView, Control); | ||
| int widthSpec; | ||
|
|
There was a problem hiding this comment.
A few lines down, I see an atmost... Is that correct?
There was a problem hiding this comment.
I'm not sure, but, it's been like that since net6 so I'm hesitant to change it without a use case at this point
src/Controls/src/Core/Compatibility/Handlers/TableView/Android/TableViewRenderer.cs
Outdated
Show resolved
Hide resolved
src/Controls/src/Core/Compatibility/Handlers/TableView/Android/TableViewRenderer.cs
Show resolved
Hide resolved
| ViewCellContainer c = view.PlatformView.GetParentOfType<ViewCellContainer>(); | ||
| // If the convertView is null we don't want to return the same view, we need to return a new one. | ||
| // We should probably do this for ListView as well | ||
| if (ParentView is TableView) |
There was a problem hiding this comment.
Can we create an issue linking to this comment to remember to review the same changes with the ListView?
|
Android UITest failure unrelated - HideSoftInputOnTappedPageTest |
Description of Change
We started hitting an issue on our legacy tests where the
EntryCellstarted causing the ContentPage to get recycled. AFAICT this was caused by our changes here #19723. That PR disables recycling but we still return the same platformView using our handler recycling. That change was causingOnAttachedToWindowto fire twice which appeared to put theEditTextfield into a state where it would leak.Disabling
ConvertViewbut returning a previously return view appears to causeListViewto behave in unexpected way. The comment here expands on the platform reasons specifically. It basically comes down to Androids expectation when it calls "getView", if the convertView is null then it needs to return a completely new view that's never been used before, if the convertView isn't null then you're expected to use that view. If you violate that pattern then the state of internal affairs gets out of line.The main fix, I would say, are the changes to converting
AT_MOSTtoEXACT. The way we do view recycling is just completely incompatible with howAT_MOSTworks. Ideally we could makeAT_MOSTwork but we would need to figure out how to juggle between the random scrap viewsAndroidcreates vs the actual views attached to the window.Issues Fixed
Fixes #5924