Skip to content

Fixed CarouselView Loop="False" rendering items incorrectly#25545

Merged
rmarinho merged 2 commits intodotnet:mainfrom
kubaflo:fix-25192
Nov 18, 2024
Merged

Fixed CarouselView Loop="False" rendering items incorrectly#25545
rmarinho merged 2 commits intodotnet:mainfrom
kubaflo:fix-25192

Conversation

@kubaflo
Copy link
Copy Markdown
Contributor

@kubaflo kubaflo commented Oct 28, 2024

Issues Fixed

Fixes #25192

Before After

@kubaflo kubaflo requested a review from a team as a code owner October 28, 2024 00:16
@kubaflo kubaflo requested review from Eilon and jfversluis October 28, 2024 00:16
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

dotnet-policy-service bot commented Oct 28, 2024

Hey there @kubaflo! 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 dotnet-policy-service bot added the community ✨ Community Contribution label Oct 28, 2024
kubaflo
kubaflo commented Oct 28, 2024
View reviewed changes
src/Controls/src/Core/Handlers/Items/Android/MauiCarouselRecyclerView.cs
protected override void OnMeasure(int widthMeasureSpec, int heightMeasureSpec)
{
if (Carousel.Loop)
// If the height or width are unbounded and the user is set to
Copy link
Copy Markdown
Contributor Author

@kubaflo kubaflo Oct 28, 2024

Choose a reason for hiding this comment

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

I only removed the if (Carousel.Loop) condition but indentation changes make it harder to spot

@kubaflo kubaflo added platform/android area-controls-collectionview CollectionView, CarouselView, IndicatorView labels Oct 28, 2024
@jsuarezruiz
Copy link
Copy Markdown
Contributor

jsuarezruiz commented Oct 28, 2024

/azp run

@jsuarezruiz jsuarezruiz requested review from rmarinho and removed request for Eilon October 28, 2024 06:55
@rmarinho
Copy link
Copy Markdown
Member

rmarinho commented Oct 29, 2024

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Oct 29, 2024

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz
Copy link
Copy Markdown
Contributor

jsuarezruiz commented Oct 29, 2024

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Oct 29, 2024

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz
Copy link
Copy Markdown
Contributor

jsuarezruiz commented Nov 12, 2024

@rmarinho Thoughts?

@rmarinho rmarinho merged commit 7143174 into dotnet:main Nov 18, 2024
rmarinho added a commit that referenced this pull request Nov 18, 2024
@rmarinho
Revert "Fixed CarouselView Loop="False" rendering items incorrectly (#…
3fc340b 
…25545)"

This reverts commit 7143174.
@rmarinho rmarinho mentioned this pull request Nov 18, 2024
Merged
rmarinho added a commit that referenced this pull request Nov 18, 2024
@rmarinho
Revert "Fixed CarouselView Loop="False" rendering items incorrectly (#…
ce52028 
…25545)" (#25922)

This reverts commit 7143174.
@rmarinho
Copy link
Copy Markdown
Member

rmarinho commented Nov 18, 2024

Seems this was failing UITests when rebased on main, so I added to revert.

The iOS seems not to render the next item, or the padding is different.

image

@kubaflo kubaflo mentioned this pull request Nov 18, 2024
Merged
@kubaflo
Copy link
Copy Markdown
Contributor Author

kubaflo commented Nov 18, 2024 •
edited
Loading

Seems this was failing UITests when rebased on main, so I added to revert.

The iOS seems not to render the next item, or the padding is different.

image

@rmarinho ah I see sorry!
Item spacing was not working on iOS at the time of adding snapshot. The CarouselView2 for iOS fixed it, so I've updated the iOS screenshot to fix UI test

I created this PR with a correct screenshot: #25924

@samhouts samhouts added fixed-in-9.0.21 fixed-in-net8.0-nightly This may be available in a nightly release! labels Dec 16, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jan 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@rmarinho rmarinho rmarinho approved these changes

@jfversluis jfversluis Awaiting requested review from jfversluis jfversluis was automatically assigned from dotnet/dotnet-maui-reviewers

+1 more reviewer

@jsuarezruiz jsuarezruiz jsuarezruiz left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-controls-collectionview CollectionView, CarouselView, IndicatorView community ✨ Community Contribution fixed-in-9.0.21 fixed-in-net8.0-nightly This may be available in a nightly release! platform/android

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

CarouselView Loop="False" renders incorrectly items ( net8, net9 rc2)

4 participants

@kubaflo @jsuarezruiz @rmarinho @samhouts