Enable collection node interactive moves (TextureGroup#735)

* Add support for interactive moves * Enable drag & drop in collection view example * Update changelog * Change the gating logic to match UIKit * Add a warning when we prevent interactive movement due to async layout
AtomTickets · Apr 25, 2018 · a48c64d · a48c64d
1 parent cc0cc79
commit a48c64d
Show file tree

Hide file tree

Showing 7 changed files with 195 additions and 61 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,6 +19,7 @@
 - [ASScrollNode] Invalidate the node's calculated layout if its scrollable directions changed. Also add unit tests for the class. [#637](https://github.com/TextureGroup/Texture/pull/637) [Huy Nguyen](https://github.com/nguyenhuy)
 - Add new unit testing to the layout engine. [Adlai Holler](https://github.com/Adlai-Holler) [#424](https://github.com/TextureGroup/Texture/pull/424)
 - [Automatic Subnode Management] Nodes with ASM enabled now insert/delete their subnodes as soon as they enter preload state, so the subnodes can preload too. [Huy Nguyen](https://github.com/nguyenhuy) [#706](https://github.com/TextureGroup/Texture/pull/706)
+- [ASCollectionNode] Added support for interactive item movement. [Adlai Holler](https://github.com/Adlai-Holler)
 
 ## 2.6
 - [Xcode 9] Updated to require Xcode 9 (to fix warnings) [Garrett Moon](https://github.com/garrettmoon)

diff --git a/Source/ASCollectionNode.h b/Source/ASCollectionNode.h
@@ -630,6 +630,32 @@ NS_ASSUME_NONNULL_BEGIN
  */
 - (NSArray<NSString *> *)collectionNode:(ASCollectionNode *)collectionNode supplementaryElementKindsInSection:(NSInteger)section;
 
+/**
+ * Asks the data source if it's possible to move the specified item interactively.
+ *
+ * See @p -[UICollectionViewDataSource collectionView:canMoveItemAtIndexPath:] @c.
+ *
+ * @param collectionNode  The sender.
+ * @param node            The display node for the item that may be moved.
+ *
+ * @return Whether the item represented by @p node may be moved.
+ */
+- (BOOL)collectionNode:(ASCollectionNode *)collectionNode canMoveItemWithNode:(ASCellNode *)node;
+
+/**
+ * Called when the user has interactively moved an item. The data source
+ * should update its internal data store to reflect the move. Note that you
+ * should not call [collectionNode moveItemAtIndexPath:toIndexPath:] – the
+ * collection node's internal state will be updated automatically.
+ *
+ * * See @p -[UICollectionViewDataSource collectionView:moveItemAtIndexPath:toIndexPath:] @c.
+ *
+ * @param collectionNode        The sender.
+ * @param sourceIndexPath       The original item index path.
+ * @param destinationIndexPath  The new item index path.
+ */
+- (void)collectionNode:(ASCollectionNode *)collectionNode moveItemAtIndexPath:(NSIndexPath *)sourceIndexPath toIndexPath:(NSIndexPath *)destinationIndexPath;
+
 /**
  * Similar to -collectionView:cellForItemAtIndexPath:.
  *

diff --git a/Source/ASCollectionView.mm b/Source/ASCollectionView.mm
@@ -101,6 +101,10 @@ @interface ASCollectionView () <ASRangeControllerDataSource, ASRangeControllerDe
   NSHashTable<ASCellNode *> *_cellsForLayoutUpdates;
   id<ASCollectionViewLayoutFacilitatorProtocol> _layoutFacilitator;
   CGFloat _leadingScreensForBatching;
+
+  // When we update our data controller in response to an interactive move,
+  // we don't want to tell the collection view about the change (it knows!)
+  BOOL _updatingInResponseToInteractiveMove;
   BOOL _inverted;
 
   NSUInteger _superBatchUpdateCount;
@@ -218,6 +222,8 @@ @interface ASCollectionView () <ASRangeControllerDataSource, ASRangeControllerDe
     unsigned int numberOfSectionsInCollectionNode:1;
     unsigned int collectionNodeNumberOfItemsInSection:1;
     unsigned int collectionNodeContextForSection:1;
+    unsigned int collectionNodeCanMoveItem:1;
+    unsigned int collectionNodeMoveItem:1;
 
     // Whether this data source conforms to ASCollectionDataSourceInterop
     unsigned int interop:1;
@@ -454,6 +460,8 @@ - (void)setAsyncDataSource:(id<ASCollectionDataSource>)asyncDataSource
     _asyncDataSourceFlags.collectionNodeNodeBlockForSupplementaryElement = [_asyncDataSource respondsToSelector:@selector(collectionNode:nodeBlockForSupplementaryElementOfKind:atIndexPath:)];
     _asyncDataSourceFlags.collectionNodeSupplementaryElementKindsInSection = [_asyncDataSource respondsToSelector:@selector(collectionNode:supplementaryElementKindsInSection:)];
     _asyncDataSourceFlags.nodeModelForItem = [_asyncDataSource respondsToSelector:@selector(collectionNode:nodeModelForItemAtIndexPath:)];
+    _asyncDataSourceFlags.collectionNodeCanMoveItem = [_asyncDataSource respondsToSelector:@selector(collectionNode:canMoveItemWithNode:)];
+    _asyncDataSourceFlags.collectionNodeMoveItem = [_asyncDataSource respondsToSelector:@selector(collectionNode:moveItemAtIndexPath:toIndexPath:)];
 
     _asyncDataSourceFlags.interop = [_asyncDataSource conformsToProtocol:@protocol(ASCollectionDataSourceInterop)];
     if (_asyncDataSourceFlags.interop) {
@@ -1492,6 +1500,66 @@ - (void)collectionView:(UICollectionView *)collectionView performAction:(nonnull
   }
 }
 
+- (BOOL)collectionView:(UICollectionView *)collectionView canMoveItemAtIndexPath:(NSIndexPath *)indexPath
+{
+  // Mimic UIKit's gating logic.
+  // If the data source doesn't support moving, then all bets are off.
+  if (!_asyncDataSourceFlags.collectionNodeMoveItem) {
+    return NO;
+  }
+
+  // Currently we do not support interactive moves when using async layout. The reason is, we do not have a mechanism
+  // to propagate the "presentation data" element map (containing the speculative in-progress moves) to the layout delegate,
+  // and this can cause exceptions to be thrown from UICV. For example, if you drag an item out of a section,
+  // the element map will still contain N items in that section, even though there's only N-1 shown, and UICV will
+  // throw an exception that you specified an element that doesn't exist.
+  //
+  // In iOS >= 11, this is made much easier by the UIDataSourceTranslating API. In previous versions of iOS our best bet
+  // would be to capture the invalidation contexts that are sent during interactive moves and make our own data source translator.
+  if ([self.collectionViewLayout isKindOfClass:[ASCollectionLayout class]]) {
+    static dispatch_once_t onceToken;
+    dispatch_once(&onceToken, ^{
+      as_log_debug(ASCollectionLog(), "Collection node item interactive movement is not supported when using a layout delegate. This message will only be logged once. Node: %@", ASObjectDescriptionMakeTiny(self));
+    });
+    return NO;
+  }
+
+  // If the data source implements canMoveItem, let them decide.
+  if (_asyncDataSourceFlags.collectionNodeCanMoveItem) {
+    if (auto cellNode = [self nodeForItemAtIndexPath:indexPath]) {
+      GET_COLLECTIONNODE_OR_RETURN(collectionNode, NO);
+      return [_asyncDataSource collectionNode:collectionNode canMoveItemWithNode:cellNode];
+    }
+  }
+
+  // Otherwise allow the move for all items.
+  return YES;
+}
+
+- (void)collectionView:(UICollectionView *)collectionView moveItemAtIndexPath:(NSIndexPath *)sourceIndexPath toIndexPath:(NSIndexPath *)destinationIndexPath
+{
+  ASDisplayNodeAssert(_asyncDataSourceFlags.collectionNodeMoveItem, @"Should not allow interactive collection item movement if data source does not support it.");
+
+  // Inform the data source first, in case they call nodeForItemAtIndexPath:.
+  // We want to make sure we return them the node for the item they have in mind.
+  if (auto collectionNode = self.collectionNode) {
+    [_asyncDataSource collectionNode:collectionNode moveItemAtIndexPath:sourceIndexPath toIndexPath:destinationIndexPath];
+  }
+
+  // Now we update our data controller's store.
+  // Get up to date
+  [self waitUntilAllUpdatesAreCommitted];
+  // Set our flag to suppress informing super about the change.
+  ASDisplayNodeAssertFalse(_updatingInResponseToInteractiveMove);
+  _updatingInResponseToInteractiveMove = YES;
+  // Submit the move
+  [self moveItemAtIndexPath:sourceIndexPath toIndexPath:destinationIndexPath];
+  // Wait for it to finish – should be fast!
+  [self waitUntilAllUpdatesAreCommitted];
+  // Clear the flag
+  _updatingInResponseToInteractiveMove = NO;
+}
+
 - (void)scrollViewDidScroll:(UIScrollView *)scrollView
 {
   // If a scroll happenes the current range mode needs to go to full
@@ -2023,7 +2091,7 @@ - (NSString *)nameForRangeControllerDataSource
 - (void)rangeController:(ASRangeController *)rangeController updateWithChangeSet:(_ASHierarchyChangeSet *)changeSet updates:(dispatch_block_t)updates
 {
   ASDisplayNodeAssertMainThread();
-  if (!self.asyncDataSource || _superIsPendingDataLoad) {
+  if (!self.asyncDataSource || _superIsPendingDataLoad || _updatingInResponseToInteractiveMove) {
     updates();
     [changeSet executeCompletionHandlerWithFinished:NO];
     return; // if the asyncDataSource has become invalid while we are processing, ignore this request to avoid crashes
@@ -2278,21 +2346,4 @@ - (void)setPrefetchingEnabled:(BOOL)prefetchingEnabled
   return;
 }
 
-#if ASDISPLAYNODE_ASSERTIONS_ENABLED // Remove implementations entirely for efficiency if not asserting.
-
-// intercepted due to not being supported by ASCollectionView (prevent bugs caused by usage)
-
-- (BOOL)collectionView:(UICollectionView *)collectionView canMoveItemAtIndexPath:(NSIndexPath *)indexPath NS_AVAILABLE_IOS(9_0)
-{
-  ASDisplayNodeAssert(![self.asyncDataSource respondsToSelector:_cmd], @"%@ is not supported by ASCollectionView - please remove or disable this data source method.", NSStringFromSelector(_cmd));
-  return NO;
-}
-
-- (void)collectionView:(UICollectionView *)collectionView moveItemAtIndexPath:(NSIndexPath *)sourceIndexPath toIndexPath:(NSIndexPath*)destinationIndexPath NS_AVAILABLE_IOS(9_0)
-{
-  ASDisplayNodeAssert(![self.asyncDataSource respondsToSelector:_cmd], @"%@ is not supported by ASCollectionView - please remove or disable this data source method.", NSStringFromSelector(_cmd));
-}
-
-#endif
-
 @end
diff --git a/Source/Details/ASDataController.mm b/Source/Details/ASDataController.mm
@@ -526,7 +526,7 @@ - (void)updateWithChangeSet:(_ASHierarchyChangeSet *)changeSet
 
   BOOL canDelegate = (self.layoutDelegate != nil);
   ASElementMap *newMap;
-  id layoutContext;
+  ASCollectionLayoutContext *layoutContext;
   {
     as_activity_scope(as_activity_create("Latch new data for collection update", changeSet.rootActivity, OS_ACTIVITY_FLAG_DEFAULT));
 

diff --git a/Source/Layout/ASLayoutElement.h b/Source/Layout/ASLayoutElement.h
@@ -192,7 +192,7 @@ extern NSString * const ASLayoutElementStyleLayoutPositionProperty;
 #pragma mark - Sizing
 
 /**
- * @abstract The width property specifies the height of the content area of an ASLayoutElement.
+ * @abstract The width property specifies the width of the content area of an ASLayoutElement.
  * The minWidth and maxWidth properties override width.
  * Defaults to ASDimensionAuto
  */

diff --git a/Source/Private/ASCollectionLayout.mm b/Source/Private/ASCollectionLayout.mm
@@ -158,6 +158,21 @@ - (void)invalidateLayout
   }
 }
 
+/**
+ * NOTE: It is suggested practice on the Web to override invalidationContextForInteractivelyMovingItems… and call out to the
+ * data source to move the item (so that if e.g. the item size depends on the data, you get the data you expect). However, as of iOS 11 this
+ * doesn't work, because UICV machinery will also call out to the data source to move the item after the interaction is done. The result is
+ * that your data source state will be incorrect due to this last move call. Plus it's just an API violation.
+ *
+ * Things tried:
+ *   - Doing the speculative data source moves, and then UNDOING the last one in invalidationContextForEndingInteractiveMovementOfItems…
+ *     but this does not work because the UICV machinery informs its data source before it calls that method on us, so we are too late.
+ *
+ * The correct practice is to use the UIDataSourceTranslating API introduced in iOS 11. Currently Texture does not support this API but we can
+ * build it if there is demand. We could add an id<UIDataSourceTranslating> field onto the layout context object, and the layout client can
+ * use data source index paths when it reads nodes or other data source data.
+ */
+
 - (CGSize)collectionViewContentSize
 {
   ASDisplayNodeAssertMainThread();

diff --git a/examples/ASCollectionView/Sample/ViewController.m b/examples/ASCollectionView/Sample/ViewController.m
@@ -23,29 +23,27 @@
 
 #define ASYNC_COLLECTION_LAYOUT 0
 
+static CGSize const kItemSize = (CGSize){180, 90};
+
 @interface ViewController () <ASCollectionDataSource, ASCollectionDelegateFlowLayout, ASCollectionGalleryLayoutPropertiesProviding>
 
 @property (nonatomic, strong) ASCollectionNode *collectionNode;
-@property (nonatomic, strong) NSArray *data;
+@property (nonatomic, strong) NSMutableArray<NSMutableArray<NSString *> *> *data;
+@property (nonatomic, strong) UILongPressGestureRecognizer *moveRecognizer;
 
 @end
 
 @implementation ViewController
 
 #pragma mark - Lifecycle
 
-- (void)dealloc
-{
-  self.collectionNode.dataSource = nil;
-  self.collectionNode.delegate = nil;
-
-  NSLog(@"ViewController is deallocing");
-}
-
 - (void)viewDidLoad
 {
   [super viewDidLoad];
 
+  self.moveRecognizer = [[UILongPressGestureRecognizer alloc] initWithTarget:self action:@selector(handleLongPress)];
+  [self.view addGestureRecognizer:self.moveRecognizer];
+
 #if ASYNC_COLLECTION_LAYOUT
   ASCollectionGalleryLayoutDelegate *layoutDelegate = [[ASCollectionGalleryLayoutDelegate alloc] initWithScrollableDirections:ASScrollDirectionVerticalDirections];
   layoutDelegate.propertiesProvider = self;
@@ -54,6 +52,7 @@ - (void)viewDidLoad
   UICollectionViewFlowLayout *layout = [[UICollectionViewFlowLayout alloc] init];
   layout.headerReferenceSize = CGSizeMake(50.0, 50.0);
   layout.footerReferenceSize = CGSizeMake(50.0, 50.0);
+  layout.itemSize = kItemSize;
   self.collectionNode = [[ASCollectionNode alloc] initWithFrame:self.view.bounds collectionViewLayout:layout];
   [self.collectionNode registerSupplementaryNodeOfKind:UICollectionElementKindSectionHeader];
   [self.collectionNode registerSupplementaryNodeOfKind:UICollectionElementKindSectionFooter];
@@ -73,34 +72,37 @@ - (void)viewDidLoad
   self.navigationItem.leftBarButtonItem = [[UIBarButtonItem alloc] initWithBarButtonSystemItem:UIBarButtonSystemItemRefresh
                                                                                         target:self
                                                                                         action:@selector(reloadTapped)];
-#endif
-
-#if SIMULATE_WEB_RESPONSE
+  [self loadData];
+#else
   __weak typeof(self) weakSelf = self;
-  void(^mockWebService)() = ^{
-    NSLog(@"ViewController \"got data from a web service\"");
-    ViewController *strongSelf = weakSelf;
-    if (strongSelf != nil)
-    {
-      NSLog(@"ViewController is not nil");
-      strongSelf->_data = [[NSArray alloc] init];
-      [strongSelf->_collectionNode performBatchUpdates:^{
-        [strongSelf->_collectionNode insertSections:[[NSIndexSet alloc] initWithIndexesInRange:NSMakeRange(0, 100)]];
-      } completion:nil];
-      NSLog(@"ViewController finished updating collectionNode");
-    }
-    else {
-      NSLog(@"ViewController is nil - won't update collectionNode");
-    }
-  };
-
-  dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(2 * NSEC_PER_SEC)), dispatch_get_main_queue(), mockWebService);
-  dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(4 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
-    [self.navigationController popViewControllerAnimated:YES];
+  dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(2 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
+    [weakSelf handleSimulatedWebResponse];
   });
 #endif
 }
 
+- (void)handleSimulatedWebResponse
+{
+  [self.collectionNode performBatchUpdates:^{
+    [self loadData];
+    [self.collectionNode insertSections:[NSIndexSet indexSetWithIndexesInRange:NSMakeRange(0, self.data.count)]];
+  } completion:nil];
+}
+
+- (void)loadData
+{
+  // Form our data array
+  typeof(self.data) data = [NSMutableArray array];
+  for (NSInteger s = 0; s < 100; s++) {
+    NSMutableArray *items = [NSMutableArray array];
+    for (NSInteger i = 0; i < 10; i++) {
+      items[i] = [NSString stringWithFormat:@"[%zd.%zd] says hi", s, i];
+    }
+    data[s] = items;
+  }
+  self.data = data;
+}
+
 #pragma mark - Button Actions
 
 - (void)reloadTapped
@@ -115,14 +117,42 @@ - (void)reloadTapped
 - (CGSize)galleryLayoutDelegate:(ASCollectionGalleryLayoutDelegate *)delegate sizeForElements:(ASElementMap *)elements
 {
   ASDisplayNodeAssertMainThread();
-  return CGSizeMake(180, 90);
+  return kItemSize;
+}
+
+- (void)handleLongPress
+{
+  UICollectionView *collectionView = self.collectionNode.view;
+  CGPoint location = [self.moveRecognizer locationInView:collectionView];
+  switch (self.moveRecognizer.state) {
+    case UIGestureRecognizerStateBegan: {
+      NSIndexPath *indexPath = [collectionView indexPathForItemAtPoint:location];
+      if (indexPath) {
+        [collectionView beginInteractiveMovementForItemAtIndexPath:indexPath];
+      }
+      break;
+    }
+    case UIGestureRecognizerStateChanged:
+      [collectionView updateInteractiveMovementTargetPosition:location];
+      break;
+    case UIGestureRecognizerStateEnded:
+      [collectionView endInteractiveMovement];
+      break;
+    case UIGestureRecognizerStateFailed:
+    case UIGestureRecognizerStateCancelled:
+      [collectionView cancelInteractiveMovement];
+      break;
+    case UIGestureRecognizerStatePossible:
+      // nop
+      break;
+  }
 }
 
-#pragma mark - ASCollectionView Data Source
+#pragma mark - ASCollectionDataSource
 
 - (ASCellNodeBlock)collectionNode:(ASCollectionNode *)collectionNode nodeBlockForItemAtIndexPath:(NSIndexPath *)indexPath;
 {
-  NSString *text = [NSString stringWithFormat:@"[%zd.%zd] says hi", indexPath.section, indexPath.item];
+  NSString *text = self.data[indexPath.section][indexPath.item];
   return ^{
     return [[ItemNode alloc] initWithString:text];
   };
@@ -139,18 +169,29 @@ - (ASCellNode *)collectionNode:(ASCollectionNode *)collectionNode nodeForSupplem
 
 - (NSInteger)collectionNode:(ASCollectionNode *)collectionNode numberOfItemsInSection:(NSInteger)section
 {
-  return 10;
+  return self.data[section].count;
 }
 
 - (NSInteger)numberOfSectionsInCollectionNode:(ASCollectionNode *)collectionNode
 {
-#if SIMULATE_WEB_RESPONSE
-  return _data == nil ? 0 : 100;
-#else
-  return 100;
-#endif
+  return self.data.count;
+}
+
+- (BOOL)collectionNode:(ASCollectionNode *)collectionNode canMoveItemWithNode:(ASCellNode *)node
+{
+  return YES;
 }
 
+- (void)collectionNode:(ASCollectionNode *)collectionNode moveItemAtIndexPath:(NSIndexPath *)sourceIndexPath toIndexPath:(NSIndexPath *)destinationIndexPath
+{
+  __auto_type sectionArray = self.data[sourceIndexPath.section];
+  __auto_type object = sectionArray[sourceIndexPath.item];
+  [sectionArray removeObjectAtIndex:sourceIndexPath.item];
+  [self.data[destinationIndexPath.section] insertObject:object atIndex:destinationIndexPath.item];
+}
+
+#pragma mark - ASCollectionDelegate
+
 - (void)collectionNode:(ASCollectionNode *)collectionNode willBeginBatchFetchWithContext:(ASBatchContext *)context
 {
   NSLog(@"fetch additional content");