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.

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

[iOS] ActivityIndicator should not disappear when used in a ViewCell #495

Merged
merged 9 commits into from Apr 6, 2017

Conversation

Projects
None yet
7 participants
@adrianknight89
Contributor

adrianknight89 commented Nov 1, 2016

Description of Change

ActivityIndicator on iOS disappears when used in a list view cell and it's scrolled off. In RetainElement mode, this happens randomly whereas RecycleElement has this problem for every reused cell. Native view seems to have its IsAnimating property set to false possibly when iOS is preparing cells for reuse, but Element's IsRunning stays true. This PR will sync the two so the indicator can be seen.

Instead of using MessagingCenter, I wanted to observe changes in IsAnimating but could not get the observer to work despite all efforts.

Referencing https://jira.appcelerator.org/browse/TIMOB-15293
and appcelerator/titanium_mobile#4852

Bugs Fixed

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense
@jassmith

This comment has been minimized.

Show comment
Hide comment
@jassmith

jassmith Nov 15, 2016

Member

The message sending should not be needed. The value needing to be checked should be stored on the Forms Element.

Member

jassmith commented Nov 15, 2016

The message sending should not be needed. The value needing to be checked should be stored on the Forms Element.

@jordmax12

This comment has been minimized.

Show comment
Hide comment
@jordmax12

jordmax12 Nov 19, 2016

SO was this fixed? And if so, will there be a xamarin.forms nuget update for visual studio soon?

jordmax12 commented Nov 19, 2016

SO was this fixed? And if so, will there be a xamarin.forms nuget update for visual studio soon?

@adrianknight89

This comment has been minimized.

Show comment
Hide comment
@adrianknight89

adrianknight89 Nov 20, 2016

Contributor

@jassmith I'm not sure what you meant by storing the value on the Forms Element, but I edited the changes so that each cell is searched for activity indicator instances. I'm not sure what this means for cell performance, but let me know if you know of another way to do this.

Contributor

adrianknight89 commented Nov 20, 2016

@jassmith I'm not sure what you meant by storing the value on the Forms Element, but I edited the changes so that each cell is searched for activity indicator instances. I'm not sure what this means for cell performance, but let me know if you know of another way to do this.

@StephaneDelcroix StephaneDelcroix requested a review from jassmith Jan 26, 2017

@StephaneDelcroix

This comment has been minimized.

Show comment
Hide comment
@StephaneDelcroix

StephaneDelcroix Jan 26, 2017

Member

@jassmith could you review this, and if you agree I'll rebase, make sure tests are passing, and merge.

Member

StephaneDelcroix commented Jan 26, 2017

@jassmith could you review this, and if you agree I'll rebase, make sure tests are passing, and merge.

@rmarinho

This comment has been minimized.

Show comment
Hide comment
@rmarinho

rmarinho Mar 8, 2017

Member

can you rebase @adrianknight89 ? thanks

Member

rmarinho commented Mar 8, 2017

can you rebase @adrianknight89 ? thanks

@rmarinho

This comment has been minimized.

Show comment
Hide comment
@rmarinho

rmarinho Mar 9, 2017

Member

This needs rebase @adrianknight89 we changed the visibility of some stuff with the IVT commit

Member

rmarinho commented Mar 9, 2017

This needs rebase @adrianknight89 we changed the visibility of some stuff with the IVT commit

@rmarinho rmarinho removed the waiting-tests label Mar 9, 2017

@rmarinho

Needs rebase to run tests

@adrianknight89

This comment has been minimized.

Show comment
Hide comment
@adrianknight89

adrianknight89 Mar 16, 2017

Contributor

@rmarinho Looks like you've made LogicalChildrenInternal internal so I can't access it from Xamarin.Forms.Platform.iOS.

Contributor

adrianknight89 commented Mar 16, 2017

@rmarinho Looks like you've made LogicalChildrenInternal internal so I can't access it from Xamarin.Forms.Platform.iOS.

adrianknight89 added some commits Mar 16, 2017

@adrianknight89

This comment has been minimized.

Show comment
Hide comment
@adrianknight89

adrianknight89 Mar 16, 2017

Contributor

@rmarinho Should be done. Please make sure to test the test code.

Contributor

adrianknight89 commented Mar 16, 2017

@rmarinho Should be done. Please make sure to test the test code.

@rmarinho rmarinho merged commit 6966dd6 into xamarin:master Apr 6, 2017

6 checks passed

Android-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: UI Tests :: OSX Test Cloud Package - Run Android 6.0.1 : Tests passe…
Details
OSX-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: OSX Debug : Running
Details
Windows-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: Windows Debug : Tests passed: 3749, ignored: 10
Details
iOS10-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS10 : Tests p…
Details
iOS8-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified IOS8 : Tests pa…
Details
iOS9-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS9 : Tests pa…
Details

@samhouts samhouts added D-15.4 and removed cla-already-signed labels Oct 10, 2017

@samhouts samhouts modified the milestones: 2.3.0, 2.3.5 Jun 27, 2018

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