Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[Android] Fix crash using CarouselView inside Expander #10836

Merged
merged 9 commits into from
Jul 30, 2020
Merged

[Android] Fix crash using CarouselView inside Expander #10836

merged 9 commits into from
Jul 30, 2020

Conversation

jsuarezruiz
Copy link
Contributor

Description of Change

Fix crash using CarouselView inside Expander.

In the CarouselViewRenderer using GetPosition method to get the index by CurrentItem was getting an invalid -1 value. Then, using StartSmoothScroll method from ScrollHelper.cs we got the exception.

Issues Resolved

API Changes

None

Platforms Affected

  • Android

Behavioral/Visual Changes

None

Before/After Screenshots

Before

Java.Lang.IllegalArgumentException: 'Invalid target position'

After

fix10830

Testing Procedure

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

@jsuarezruiz jsuarezruiz added t/bug 🐛 p/Android i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often a/carouselview labels May 25, 2020
@samhouts samhouts added this to Ready for Review (PRs) in Sprint 171 May 28, 2020
@samhouts samhouts added this to In Review in CarouselView May 28, 2020
@rmarinho
Copy link
Member

@jsuarezruiz is it possible to add a UITest ?

rmarinho
rmarinho suggested changes Jun 2, 2020
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.

We should add a UITest since it's a crash.

Also current tests seem to be failing

rmarinho
rmarinho approved these changes Jun 5, 2020
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.

Added a UITest

@samhouts samhouts moved this from Ready for Review (PRs) to Returned to backlog in Sprint 171 Jun 14, 2020
@rmarinho
Copy link
Member

Failing tests not related

@samhouts samhouts added this to In Progress in v4.6.0 Jun 20, 2020
@samhouts samhouts changed the base branch from 4.6.0 to 4.7.0 June 25, 2020 22:25
@samhouts samhouts removed this from In Progress in v4.6.0 Jun 25, 2020
@samhouts samhouts added this to In Progress in 4.7.0 Jun 25, 2020
@samhouts samhouts changed the base branch from 4.7.0 to 4.8.0 July 8, 2020 18:15
@samhouts samhouts removed this from In Progress in 4.7.0 Jul 8, 2020
@samhouts samhouts added this to In Review in Expander Jul 13, 2020
@samhouts samhouts added this to In Review in CollectionView Jul 13, 2020
@samhouts samhouts merged commit 9d7e9d9 into 4.8.0 Jul 30, 2020
CollectionView automation moved this from In Review to Done Jul 30, 2020
CarouselView automation moved this from In Review to Done Jul 30, 2020
Expander automation moved this from In Review to Done Jul 30, 2020
@samhouts samhouts deleted the fix-10830 branch July 30, 2020 00:14
@samhouts samhouts added this to In Progress in vCurrent (4.8.0) Jul 30, 2020
@samhouts samhouts moved this from In Progress to Done in vCurrent (4.8.0) Jul 30, 2020
@samhouts samhouts added this to Done in Sprint 174 Jul 30, 2020
@samhouts samhouts added this to the 4.8.0 milestone Jul 30, 2020
@samhouts samhouts removed this from Done in CollectionView Aug 29, 2020
@samhouts samhouts removed this from Done in CarouselView Aug 29, 2020
@samhouts samhouts removed this from Done in Expander Aug 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rmarinho rmarinho rmarinho approved these changes

@samhouts samhouts samhouts approved these changes

@PureWeen PureWeen Awaiting requested review from PureWeen

Assignees

@rmarinho rmarinho

@PureWeen PureWeen

Labels
a/carouselview a/collectionview a/expander i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often p/Android t/bug 🐛
Projects
No open projects
Sprint 171
  
Returned to backlog
Sprint 174
  
Done
vCurrent (4.8.0)
  
Done
Milestone
4.8.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jsuarezruiz @rmarinho @samhouts @PureWeen