Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Delete item causes display corruption in flow layout #275

Merged
merged 6 commits into from

5 participants

@alexanderedge

Hi Pete,

This pull request addresses #270. I've updated the example to use a more complicated data set which originally illustrated the problem.

The previous implementation would check if a given PSTCollectionViewItemKey is
present in the new visible views dictionary. This did not work for the following
case:

Item [0,0]
Item [0,1]
Item [0,2]

Deleting item at [0,1] should result in the alpha fade and eventual removal
from the superview and recycle, but actually since [0,1] is a valid index in the
new layout, [0,2] gets removed instead and [0,1] is faded.

This solution adds views that are to be deleted into a specific NSDictionary
that gets enumerated after all animations are finished. All views contained
inside are recycled.

@steipete
Owner

Thanks Alexander, good work. Need to do some testing if this doesn't break anything. @gavrix recently worked on the previouslyVisibleViewsDict (before animations were even more trouble). Seems that we can get rid of that variable altogether now.

@alexanderedge
@steipete
Owner

The examples definitely need some work, especially to better test the animation features - would appreciate any time you put into that.

@umairnow

Hi,
I am new to this library, can anyone tell me that is there any bug in deleteItemsAtIndexPaths: method, When I delete an item from center, it deletes that item but also deletes the last item.

@alexanderedge

@umairnow While I can't be sure that you have the same problem as the one that this pull request purports to fix, it sounds similar. You could download my fork and test that it a) doesn't break anything and b) fixes your issue

@quake

This pull resolve my similar issue, thank you @alexanderedge , @steipete could you help to merge it to master?

@steipete
Owner

Alex, could you rebase to latest master? No longer merges cleanly.

alexanderedge added some commits
@alexanderedge alexanderedge Set indexPath regardless of if it is present in the new model 985a605
@alexanderedge alexanderedge Assemble a dictionary of views that are to be removed
The previous implementation would check if a given PSTCollectionViewItemKey is
present in the new visible views dictionary. This did not work for the following
case:

Item [0,0]
Item [0,1]
Item [0,2]

Deleting item at [0,1] should result in the alpha fade and eventual removal
from the superview and recycle, but actually since [0,1] is a valid index in the
new layout, [0,2] gets removed instead.

This solution adds views that are to be deleted into a specific NSDictionary
that gets enumerated after all animations are finished. All views contained
inside are recycled.
6c0ab0b
@alexanderedge alexanderedge Add array backing to illustrate deletion 04123be
@alexanderedge alexanderedge Fix compiler warning - unused variable 76db9be
@alexanderedge alexanderedge Fix double 'batchUpdates' ce0afad
@alexanderedge alexanderedge Uncomment removing deleted view from visible views dictionary
Since `previouslyVisibleViewsDict` is no longer used, we can safely remove
the deleted view from `_allVisibleViewsDict`.
549f468
@alexanderedge

I've rebased accordingly.

@rhult

I've tested this patch in a project of mine and it fixes an issue of the same kind that I've been seeing and I haven't seen any regressions.

@steipete steipete merged commit 5bfc45f into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 20, 2013
  1. @alexanderedge
  2. @alexanderedge

    Assemble a dictionary of views that are to be removed

    alexanderedge authored
    The previous implementation would check if a given PSTCollectionViewItemKey is
    present in the new visible views dictionary. This did not work for the following
    case:
    
    Item [0,0]
    Item [0,1]
    Item [0,2]
    
    Deleting item at [0,1] should result in the alpha fade and eventual removal
    from the superview and recycle, but actually since [0,1] is a valid index in the
    new layout, [0,2] gets removed instead.
    
    This solution adds views that are to be deleted into a specific NSDictionary
    that gets enumerated after all animations are finished. All views contained
    inside are recycled.
  3. @alexanderedge
  4. @alexanderedge
  5. @alexanderedge
  6. @alexanderedge

    Uncomment removing deleted view from visible views dictionary

    alexanderedge authored
    Since `previouslyVisibleViewsDict` is no longer used, we can safely remove
    the deleted view from `_allVisibleViewsDict`.
This page is out of date. Refresh to see the latest.
View
70 Examples/CollectionView-Simple/CollectionView/ViewController.m
@@ -53,11 +53,45 @@ OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
NSString *kDetailedViewControllerID = @"DetailView"; // view controller storyboard id
NSString *kCellID = @"cellID"; // UICollectionViewCell storyboard id
+@interface ViewController(){
+
+ NSMutableArray *_sections;
+
+}
+@end
+
@implementation ViewController
+- (NSInteger)numberOfSectionsInCollectionView:(UICollectionView *)collectionView{
+
+ return [_sections count];
+
+}
+
- (NSInteger)collectionView:(UICollectionView *)view numberOfItemsInSection:(NSInteger)section;
{
- return 32;
+ return [_sections[section] count];
+}
+
+- (void)loadView{
+ [super loadView];
+
+ _sections = [NSMutableArray array];
+
+ for (NSUInteger i = 0; i < 3; i++) {
+
+ NSMutableArray *tempArray = [NSMutableArray arrayWithArray:@[[NSNull null],[NSNull null],[NSNull null]]];
+
+ [_sections addObject:tempArray];
+ }
+
+}
+
+- (void)viewDidLoad{
+ [super viewDidLoad];
+
+ self.collectionView.allowsMultipleSelection = YES;
+
}
- (PSUICollectionViewCell *)collectionView:(PSUICollectionView *)cv cellForItemAtIndexPath:(NSIndexPath *)indexPath;
@@ -94,4 +128,38 @@ - (void)prepareForSegue:(UIStoryboardSegue *)segue sender:(id)sender
}
}
+- (IBAction)delete:(id)sender{
+
+ [self.collectionView performBatchUpdates:^{
+
+ NSArray *indexPaths = [self.collectionView indexPathsForSelectedItems];
+
+ NSArray *sortedArray = [indexPaths sortedArrayUsingComparator:^NSComparisonResult(id obj1, id obj2) {
+
+ return [obj2 compare:obj1];
+
+ }];
+
+ [sortedArray enumerateObjectsUsingBlock:^(NSIndexPath *indexPath, NSUInteger idx, BOOL *stop) {
+
+ [_sections[indexPath.section] removeObjectAtIndex:indexPath.item];
+
+ }];
+
+ [self.collectionView deleteItemsAtIndexPaths:indexPaths];
+
+
+
+ } completion:^(BOOL finished) {
+
+ [[self.collectionView indexPathsForSelectedItems] enumerateObjectsUsingBlock:^(id obj, NSUInteger idx, BOOL *stop) {
+
+ [self.collectionView deselectItemAtIndexPath:obj animated:NO];
+
+ }];
+
+ }];
+
+}
+
@end
View
15 Examples/CollectionView-Simple/CollectionView/en.lproj/MainStoryboard.storyboard
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
-<document type="com.apple.InterfaceBuilder3.CocoaTouch.Storyboard.XIB" version="2.0" toolsVersion="2843" systemVersion="11G63" targetRuntime="iOS.CocoaTouch" propertyAccessControl="none" initialViewController="Pxb-db-V9M">
+<document type="com.apple.InterfaceBuilder3.CocoaTouch.Storyboard.XIB" version="2.0" toolsVersion="3084" systemVersion="12C60" targetRuntime="iOS.CocoaTouch" propertyAccessControl="none" initialViewController="Pxb-db-V9M">
<dependencies>
- <plugIn identifier="com.apple.InterfaceBuilder.IBCocoaTouchPlugin" version="1929"/>
+ <plugIn identifier="com.apple.InterfaceBuilder.IBCocoaTouchPlugin" version="2083"/>
</dependencies>
<scenes>
<!--Navigation Controller-->
@@ -64,7 +64,13 @@
<outlet property="delegate" destination="dmE-Kn-TpH" id="pKl-zO-lPR"/>
</connections>
</collectionView>
- <navigationItem key="navigationItem" title="Collection View" id="Qqd-gG-ZaX"/>
+ <navigationItem key="navigationItem" title="Collection View" id="Qqd-gG-ZaX">
+ <barButtonItem key="rightBarButtonItem" systemItem="trash" id="8uS-PN-C12">
+ <connections>
+ <action selector="delete:" destination="dmE-Kn-TpH" id="QWs-eh-jsA"/>
+ </connections>
+ </barButtonItem>
+ </navigationItem>
<simulatedNavigationBarMetrics key="simulatedTopBarMetrics" barStyle="blackOpaque" prompted="NO"/>
</collectionViewController>
<placeholder placeholderIdentifier="IBFirstResponder" id="2FN-eK-xyQ" userLabel="First Responder" sceneMemberID="firstResponder"/>
@@ -127,6 +133,9 @@
</class>
<class className="ViewController" superclassName="PSUICollectionViewController">
<source key="sourceIdentifier" type="project" relativePath="./Classes/ViewController.h"/>
+ <relationships>
+ <relationship kind="action" name="delete:"/>
+ </relationships>
</class>
</classes>
<simulatedMetricsContainer key="defaultSimulatedMetrics">
View
64 PSTCollectionView/PSTCollectionView.m
@@ -1576,6 +1576,11 @@ - (void)updateWithItems:(NSArray *)items {
NSMutableArray *animations = [[NSMutableArray alloc] init];
NSMutableDictionary *newAllVisibleView = [[NSMutableDictionary alloc] init];
+ NSMutableDictionary *viewsToRemove = [NSMutableDictionary dictionaryWithObjectsAndKeys:
+ [NSMutableArray array], @(PSTCollectionViewItemTypeCell),
+ [NSMutableArray array], @(PSTCollectionViewItemTypeDecorationView),
+ [NSMutableArray array], @(PSTCollectionViewItemTypeSupplementaryView),nil];
+
for (PSTCollectionViewUpdateItem *updateItem in items) {
if (updateItem.isSectionOperation) continue;
@@ -1593,8 +1598,13 @@ - (void)updateWithItems:(NSArray *)items {
finalAttrs.alpha = 0;
}
[animations addObject:@{@"view": view, @"previousLayoutInfos": startAttrs, @"newLayoutInfos": finalAttrs}];
+
[_allVisibleViewsDict removeObjectForKey:key];
+
+ [viewsToRemove[@(key.type)] addObject:view];
+
}
+
}
else if(updateItem.updateAction == PSTCollectionUpdateActionInsert) {
NSIndexPath *indexPath = updateItem.indexPathAfterUpdate;
@@ -1659,13 +1669,15 @@ - (void)updateWithItems:(NSArray *)items {
newGlobalIndex = [oldToNewIndexMap[oldGlobalIndex] intValue];
}
NSIndexPath *newIndexPath = newGlobalIndex == NSNotFound ? nil : [_update[@"newModel"] indexPathForItemAtGlobalIndex:newGlobalIndex];
+ NSIndexPath *oldIndexPath = oldGlobalIndex == NSNotFound ? nil : [_update[@"oldModel"] indexPathForItemAtGlobalIndex:oldGlobalIndex];
+
if (newIndexPath) {
PSTCollectionViewLayoutAttributes* startAttrs = nil;
PSTCollectionViewLayoutAttributes* finalAttrs = nil;
-
- startAttrs = [_layout initialLayoutAttributesForAppearingItemAtIndexPath:newIndexPath];
+
+ startAttrs = [_layout initialLayoutAttributesForAppearingItemAtIndexPath:oldIndexPath];
finalAttrs = [_layout layoutAttributesForItemAtIndexPath:newIndexPath];
NSMutableDictionary *dic = [NSMutableDictionary dictionaryWithDictionary:@{@"view":view}];
@@ -1676,6 +1688,7 @@ - (void)updateWithItems:(NSArray *)items {
PSTCollectionViewItemKey* newKey = [key copy];
[newKey setIndexPath:newIndexPath];
newAllVisibleView[newKey] = view;
+
}
} else if (key.type == PSTCollectionViewItemTypeSupplementaryView) {
PSTCollectionViewLayoutAttributes* startAttrs = nil;
@@ -1711,7 +1724,6 @@ - (void)updateWithItems:(NSArray *)items {
}
}
- NSDictionary *previouslyVisibleViewsDict = _allVisibleViewsDict;
_allVisibleViewsDict = newAllVisibleView;
for(NSDictionary *animation in animations) {
@@ -1719,13 +1731,15 @@ - (void)updateWithItems:(NSArray *)items {
PSTCollectionViewLayoutAttributes *attr = animation[@"previousLayoutInfos"];
[view applyLayoutAttributes:attr];
};
-
+
+
+
[UIView animateWithDuration:.3 animations:^{
_collectionViewFlags.updatingLayout = YES;
[CATransaction begin];
[CATransaction setAnimationDuration:.3];
-
+
// You might wonder why we use CATransaction to handle animation completion
// here instead of using the completion: parameter of UIView's animateWithDuration:.
// The problem is that animateWithDuration: calls this completion block
@@ -1741,26 +1755,31 @@ - (void)updateWithItems:(NSArray *)items {
// to call _updateCompletionHandler with that flag.
// Ideally, _updateCompletionHandler should be called along with the other logic in
// CATransaction's completionHandler but I simply don't know where to get that flag.
-
+
[CATransaction setCompletionBlock:^{
- // Iterate through all the views previously visible and search for those which are no more visible.
- [previouslyVisibleViewsDict enumerateKeysAndObjectsUsingBlock:
- ^(PSTCollectionViewItemKey *key, PSTCollectionReusableView* view, BOOL *stop) {
- if (!_allVisibleViewsDict[key]) {
- // View for this key isn't visible any more, so it should be reused.
- if(key.type == PSTCollectionViewItemTypeCell) {
- [self reuseCell:(PSTCollectionViewCell *)view];
- } else if (key.type == PSTCollectionViewItemTypeSupplementaryView) {
- [self reuseSupplementaryView:view];
- } else if (key.type == PSTCollectionViewItemTypeDecorationView) {
- [self reuseDecorationView:view];
- }
- }
- }];
-
+ // Iterate through all the views that we are going to remove.
+
+ [viewsToRemove enumerateKeysAndObjectsUsingBlock:^(NSNumber *keyObj, NSArray *array, BOOL *stop) {
+
+ PSTCollectionViewItemType type = [keyObj unsignedIntegerValue];
+
+ [array enumerateObjectsUsingBlock:^(id view, NSUInteger idx, BOOL *stop) {
+
+ if(type == PSTCollectionViewItemTypeCell) {
+ [self reuseCell:(PSTCollectionViewCell *)view];
+ } else if (type == PSTCollectionViewItemTypeSupplementaryView) {
+ [self reuseSupplementaryView:view];
+ } else if (type == PSTCollectionViewItemTypeDecorationView) {
+ [self reuseDecorationView:view];
+ }
+
+ }];
+
+ }];
+
_collectionViewFlags.updatingLayout = NO;
}];
-
+
for (NSDictionary *animation in animations) {
PSTCollectionReusableView* view = animation[@"view"];
PSTCollectionViewLayoutAttributes* attrs = animation[@"newLayoutInfos"];
@@ -1768,6 +1787,7 @@ - (void)updateWithItems:(NSArray *)items {
}
[CATransaction commit];
} completion:^(BOOL finished) {
+
if(_updateCompletionHandler) {
_updateCompletionHandler(finished);
_updateCompletionHandler = nil;
View
6 PSTCollectionView/PSTCollectionViewLayout.m
@@ -304,9 +304,9 @@ - (void)prepareForCollectionViewUpdates:(NSArray *)updateItems {
if(index != NSNotFound) {
index = [update[@"oldToNewIndexMap"][index] intValue];
- if(index != NSNotFound) {
- [attr setIndexPath:[update[@"newModel"] indexPathForItemAtGlobalIndex:index]];
- }
+
+ [attr setIndexPath:[attr indexPath]];
+
}
}
_initialAnimationLayoutAttributesDict[[PSTCollectionViewItemKey collectionItemKeyForLayoutAttributes:attr]] = attr;
Something went wrong with that request. Please try again.