Skip to content
This repository has been archived by the owner on Feb 10, 2022. It is now read-only.

Fix crash upon rotation while buttons are displayed #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tylermann
Copy link

@tylermann tylermann commented Jul 20, 2017

No description provided.

Copy link
Author

@tylermann tylermann left a comment

Choose a reason for hiding this comment

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

Change looks good, just a few comments around block retain semantics.

self.swipeOffset = 0;
self.swipeOffset = currentOffset;
_triggerStateChanges = prevValue;
__block MGSwipeTableCell *blockSelf = self;
Copy link
Author

Choose a reason for hiding this comment

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

using the __block here is effectively the same as just using the original self reference as it creates a strong reference when captured below in the block. It would be preferred to use __weak, as it will create an auto-niling weak reference see (https://developer.apple.com/library/content/releasenotes/ObjectiveC/RN-TransitioningToARC/Introduction/Introduction.html) for more info

__block MGSwipeTableCell *blockSelf = self;
[self setSwipeOffset:0 animated:NO completion:^{
[blockSelf setSwipeOffset:currentOffset animated:NO completion:^{
_triggerStateChanges = prevValue;
Copy link
Author

Choose a reason for hiding this comment

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

accessing this instance variable inside of the block also implicitly captures it as a strong reference. The recommended technique of accessing an instance variable like this inside a block is called "weak-strong dance". Where you use a weak reference as mentioned above and then convert it to a strong reference within the block and verify it is not nil before accessing the instance variable.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant