dotnet
[Android] CarouselView: Remove rounding from SizedItemContentView that results in off-by-one pixel error
Description of Change
CarouselViewHandler on Android has a measurement logic that converts back and forth between sizes in different units (Context.FromPixels/Context.ToPixels). The logic in SizedItemContentView was inadvertently rounding the values, resulting in some size measurement having an off-by-one width on certain devices and emulators (eg. 1079 instead of 1080). This in turn resulted in the carousel being scrolled to incorrect item on re-measurement triggered from closing a modal page.
Before:
After:
Issues Fixed
![azure-pipelines[bot] avatar](https://avatars.githubusercontent.com/in/9426?v=4 "Published by a pub.dev verified publisher"){kind=small}
The AddItemsToCarouselViewWorks UITest is failing on Android.
I am not surprised it fails. I'm surprised it ever worked...
The item is added, which triggers MauiCarouselRecyclerView.CollectionItemsSourceChanged. This schedules the following code block to the dispatcher:
https://github.com/dotnet/maui/blob/c21b5184d1894426ece6bd8a298d1aea7f0bb468/src/Controls/src/Core/Handlers/Items/Android/MauiCarouselRecyclerView.cs#L270-L288
Next, TestCarouselView.ScrollTo(5, animate: false); correctly scrolls the carousel to 5th item...
... which is then reset back by the dispatched code above.
Removing the delayed dispatch fixes this particular test. ~~I'm not sure what was it supposed to handle.~~ Unfortunately, this breaks this part of the logic: https://github.com/dotnet/maui/blob/c21b5184d1894426ece6bd8a298d1aea7f0bb468/src/Controls/src/Core/Handlers/Items/Android/MauiCarouselRecyclerView.cs#L263
Seems like there was a lot of timeouts ... and a diff for the test above, which actually shows the fixed size:
@filipnavara sorry it was my fault I screwed up the force push I think . Can you reopen with your local copy ? Thanks
@filipnavara sorry it was my fault I screwed up the force push I think . Can you reopen with your local copy ? Thanks
Gotcha. I'll reopen and force push a rebased version.
I wonder if this is still an issue after
The two PRs fixed unrelated issues even if they touched on the same topic.
Needs to be update this screenshot on android for CarouselViewShouldRenderCorrectly
Needs to be update this screenshot on android for CarouselViewShouldRenderCorrectly
Yeah, there was a conflict for this one after the last merge. :-/ I updated it.
Is it possible to rebase and also add a UI test that replicates the issue?
I can rebase it but it will likely fail the UI tests again because of the changed baseline and the images will need to be updated. Frankly, it's becoming quite cumbersome doing this for several months now.
Replicating the issue with wrong scrolling may be a bit tricky. Replicating the wrong rounding resulting in wrongly sized items is already covered by existing tests.