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] CarouselView with CarouselViewHandler2 make app crash when Loop="False" and user scroll to the last position - fix #26868

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[iOS] CarouselView with CarouselViewHandler2 make app crash when Loop="False" and user scroll to the last position - fix #26868

wants to merge 2 commits into from
+80 −1

Conversation

kubaflo
Copy link
Contributor

@kubaflo kubaflo commented Dec 30, 2024 •
edited
Loading

Issues Fixed

Fixes #26863

@rmarinho

@kubaflo kubaflo requested a review from a team as a code owner December 30, 2024 16:57
@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Dec 30, 2024
@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

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.

mattleibow
mattleibow reviewed Jan 6, 2025
View reviewed changes
Copy link
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

This looks sensible, is it possible to add a test for this? I feel like a device test that just renders the CV should trigger this?

Can you set the position via code, or does the user have to scroll manually for this to occur?

@mattleibow
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@kubaflo
Copy link
Contributor Author

kubaflo commented Jan 6, 2025

@mattleibow added :)

@rmarinho
Copy link
Member

rmarinho commented Jan 7, 2025 •
edited by jfversluis
Loading

/azp run

@azure-pipelines Azure Pipelines

This comment was marked as outdated.

Sign in to view
rmarinho
rmarinho requested changes Jan 7, 2025
View reviewed changes
Copy link
Member

@rmarinho rmarinho left a comment

Choose a reason for hiding this comment

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

Seems the test is failing

AppShouldNotCrashOnScrollingCarouselViewWithoutLoop

@kubaflo kubaflo force-pushed the CarouselView-with-CarouselViewHandler2-make-app-crash-when-Loop=-False-and-user-scroll-to-the-last-position- branch from 0cf6808 to a913297 Compare January 7, 2025 17:23
@kubaflo
Copy link
Contributor Author

kubaflo commented Jan 7, 2025

@rmarinho can we try again?

@jfversluis jfversluis added the area-controls-collectionview CollectionView, CarouselView, IndicatorView label Jan 9, 2025
@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattleibow mattleibow mattleibow left review comments

@rmarinho rmarinho rmarinho requested changes

@StephaneDelcroix StephaneDelcroix Awaiting requested review from StephaneDelcroix StephaneDelcroix is a code owner automatically assigned from dotnet/dotnet-maui-reviewers

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
area-controls-collectionview CollectionView, CarouselView, IndicatorView collectionview-cv2 community ✨ Community Contribution platform/iOS 🍎
Projects
None yet
Milestone
No milestone
4 participants
@kubaflo @mattleibow @rmarinho @jfversluis