Added selection API to ASTableNode and ASCollectionNode #2453

george-gw · 2016-10-21T10:49:42Z

refs #2450

ASCollectionNode:

[ASCollectionNode selectItemAtIndexPath:animated:scrollPosition:]
[ASCollectionNode deselectItemAtIndexPath:animated:]

Unsure if you think the following are needed, but I added them just in case.

BOOL allowsSelection -> collectionView.allowsSelection
BOOL allowsMultipleSelection -> collectionView.allowsMultipleSelection

ASTableNode:

[ASTableNode selectRowAtIndexPath:animated:scrollPosition:]
-[ASTableNode deselectRowAtIndexPath:animated:]

Unsure if you think the following are needed, but I added them just in case.

BOOL allowsSelection -> collectionView.allowsSelection
BOOL allowsSelectionDuringEditing -> collectionView.allowsSelectionDuringEditing
BOOL allowsMultipleSelection -> collectionView.allowsMultipleSelection
BOOL allowsMultipleSelectionDuringEditing -> collectionView.allowsMultipleSelectionDuringEditing

…ive#2450)

… the selection tests.

Adlai-Holler · 2016-10-21T16:55:37Z

AsyncDisplayKit/ASCollectionNode.mm

+    _pendingState.allowsMultipleSelection = allowsMultipleSelection;
+  } else {
+    ASDisplayNodeAssert([self isNodeLoaded], @"ASCollectionNode should be loaded if pendingState doesn't exist");
+    self.view.allowsSelection = allowsMultipleSelection;


Good catch on all 4 of those, fixed!

Adlai-Holler · 2016-10-21T16:56:46Z

AsyncDisplayKit/ASTableNode.mm

+    _pendingState.allowsSelectionDuringEditing = allowsSelectionDuringEditing;
+  } else {
+    ASDisplayNodeAssert([self isNodeLoaded], @"ASTableNode should be loaded if pendingState doesn't exist");
+    self.view.allowsSelection = allowsSelectionDuringEditing;


Adlai-Holler · 2016-10-21T16:56:54Z

AsyncDisplayKit/ASTableNode.mm

+    _pendingState.allowsMultipleSelection = allowsMultipleSelection;
+  } else {
+    ASDisplayNodeAssert([self isNodeLoaded], @"ASTableNode should be loaded if pendingState doesn't exist");
+    self.view.allowsSelection = allowsMultipleSelection;


Adlai-Holler · 2016-10-21T16:57:01Z

AsyncDisplayKit/ASTableNode.mm

+    _pendingState.allowsMultipleSelectionDuringEditing = allowsMultipleSelectionDuringEditing;
+  } else {
+    ASDisplayNodeAssert([self isNodeLoaded], @"ASTableNode should be loaded if pendingState doesn't exist");
+    self.view.allowsSelection = allowsMultipleSelectionDuringEditing;


Adlai-Holler

Hey @george-gw thanks for building this! It's really great to have more hands building out these new features.

Adlai-Holler · 2016-10-21T17:04:06Z

AsyncDisplayKit/ASCollectionNode.mm

+
+- (void)selectItemAtIndexPath:(nullable NSIndexPath *)indexPath animated:(BOOL)animated scrollPosition:(UICollectionViewScrollPosition)scrollPosition
+{
+  ASDisplayNodeAssert([self isNodeLoaded], @"ASCollectionNode should be loaded before calling selectItemAtIndexPath");


In the future it will be nice to remove this restriction. It would require that we have a clean method for ensuring that the view is loaded, that the initial reload has been called, and that all updates are committed. Today we could cobble together all that into a method but it would be ugly and there are other potential hazards (what if we're inside willDisplayCell:forItemAtIndexPath: and waiting is unsafe, what if we're zero-size, etc.) That will come later and I think that this is plenty for the medium-term.

Also, would you be willing to add ASDisplayNodeAssertMainThread() in these methods?

I agree, I added a todo referring to your discussion post.

I added the ASDisplayNodeAssertMainThread(), I didn't add it before because the code that this method calls checks for that assertion. But I suppose it's better to have it in case the implementation changes later on.

Adlai-Holler · 2016-10-21T17:05:59Z

test this please

Added asserts for main thread. Updated ASCollectionViewTests for multiple selections for nodes.

george-gw · 2016-10-22T06:39:46Z

I had updated the test case that was using collectionView to use collectionNode, I just added some more tests to check if multiple selection works.

maicki · 2016-10-22T13:45:42Z

AsyncDisplayKit/ASCollectionNode.h

+ * A Boolean value that indicates whether users can select items in the collection node.
+ * If the value of this property is YES (the default), users can select items. If you want more fine-grained control over the selection of items, you must provide a delegate object and implement the appropriate methods of the UICollectionNodeDelegate protocol.
+ */
+@property (nonatomic) BOOL allowsSelection;


Add assign

All done, I took the liberty to change the order of (nonatomic and assign) of two other properties to keep the consistency with the rest of the framework.

maicki · 2016-10-22T13:45:48Z

AsyncDisplayKit/ASCollectionNode.h

+ * This property controls whether multiple items can be selected simultaneously. The default value of this property is NO.
+ * When the value of this property is YES, tapping a cell adds it to the current selection (assuming the delegate permits the cell to be selected). Tapping the cell again removes it from the selection.
+ */
+@property (nonatomic) BOOL allowsMultipleSelection;


Add assign

maicki · 2016-10-22T13:46:10Z

AsyncDisplayKit/ASCollectionNode.mm

@@ -28,6 +28,8 @@ @interface _ASCollectionPendingState : NSObject
 @property (weak, nonatomic) id <ASCollectionDelegate>   delegate;
 @property (weak, nonatomic) id <ASCollectionDataSource> dataSource;
 @property (assign, nonatomic) ASLayoutRangeMode rangeMode;
+@property (nonatomic) BOOL allowsSelection; // default is YES
+@property (nonatomic) BOOL allowsMultipleSelection; // default is NO


Add assign on both

maicki · 2016-10-22T13:47:19Z

AsyncDisplayKit/ASTableNode.h

+ * A Boolean value that determines whether users can select a row.
+ * If the value of this property is YES (the default), users can select rows. If you set it to NO, they cannot select rows. Setting this property affects cell selection only when the table view is not in editing mode. If you want to restrict selection of cells in editing mode, use `allowsSelectionDuringEditing`.
+ */
+@property (nonatomic) BOOL allowsSelection;


Add assign

maicki · 2016-10-22T13:47:22Z

AsyncDisplayKit/ASTableNode.h

+ * A Boolean value that determines whether users can select cells while the table view is in editing mode.
+ * If the value of this property is YES, users can select rows during editing. The default value is NO. If you want to restrict selection of cells regardless of mode, use allowsSelection.
+ */
+@property (nonatomic) BOOL allowsSelectionDuringEditing;


Add assign

maicki · 2016-10-22T13:47:25Z

AsyncDisplayKit/ASTableNode.h

+ * A Boolean value that determines whether users can select more than one row outside of editing mode.
+ * This property controls whether multiple rows can be selected simultaneously outside of editing mode. When the value of this property is YES, each row that is tapped acquires a selected appearance. Tapping the row again removes the selected appearance. If you access indexPathsForSelectedRows, you can get the index paths that identify the selected rows.
+ */
+@property (nonatomic) BOOL allowsMultipleSelection;


Add assign

maicki · 2016-10-22T13:47:29Z

AsyncDisplayKit/ASTableNode.h

+ * A Boolean value that controls whether users can select more than one cell simultaneously in editing mode.
+ * The default value of this property is NO. If you set it to YES, check marks appear next to selected rows in editing mode. In addition, UITableView does not query for editing styles when it goes into editing mode. If you access indexPathsForSelectedRows, you can get the index paths that identify the selected rows.
+ */
+@property (nonatomic) BOOL allowsMultipleSelectionDuringEditing;


Add assign

maicki · 2016-10-22T13:47:38Z

AsyncDisplayKit/ASTableNode.mm

+@property (nonatomic) BOOL allowsSelection;
+@property (nonatomic) BOOL allowsSelectionDuringEditing;
+@property (nonatomic) BOOL allowsMultipleSelection;
+@property (nonatomic) BOOL allowsMultipleSelectionDuringEditing;


Please add assign to all of them.

maicki · 2016-10-22T13:48:36Z

AsyncDisplayKit/ASTableNode.mm

+  [self.view deselectRowAtIndexPath:indexPath animated:animated];
+}
+
+#pragma mark - Querying Data


Add white space below

maicki · 2016-10-22T13:48:40Z

AsyncDisplayKitTests/ASCollectionViewTests.mm

@@ -222,27 +222,60 @@ - (void)testSelection

  NSIndexPath *indexPath = [NSIndexPath indexPathForItem:0 inSection:0];
  ASCellNode *node = [testController.collectionView nodeForItemAtIndexPath:indexPath];
-
+
+  int setSelectedCount = 0;


Use NSInteger

Adlai-Holler · 2016-10-24T16:29:50Z

Nice! Thanks @george-gw!

Added selection API to ASTableNode and ASCollectionNode (facebookarch…

c24c633

…ive#2450)

george-gw changed the title ~~Added selection API to ASTableNode and ASCollectionNode (#2450)~~ Added selection API to ASTableNode and ASCollectionNode Oct 21, 2016

facebook-github-bot added the CLA Signed label Oct 21, 2016

Updated test case to use collectionNode instead of collectionView for…

e544d23

… the selection tests.

Adlai-Holler reviewed Oct 21, 2016

View reviewed changes

Adlai-Holler added the Development in Progress label Oct 21, 2016

Adlai-Holler added this to the 2.0 milestone Oct 21, 2016

facebook-github-bot added the CLA Signed label Oct 22, 2016

Fixed typos.

9ad983d

Added asserts for main thread. Updated ASCollectionViewTests for multiple selections for nodes.

maicki suggested changes Oct 22, 2016

View reviewed changes

Added assigns to the different properties.

0fcda95

Adlai-Holler approved these changes Oct 24, 2016

View reviewed changes

Adlai-Holler merged commit 3638fef into facebookarchive:master Oct 24, 2016

Adlai-Holler mentioned this pull request Oct 24, 2016

[Examples] 2.0 Collection/Table API updates #2390

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added selection API to ASTableNode and ASCollectionNode #2453

Added selection API to ASTableNode and ASCollectionNode #2453

george-gw commented Oct 21, 2016 •

edited

Loading

Adlai-Holler Oct 21, 2016

george-gw Oct 22, 2016

Adlai-Holler Oct 21, 2016

Adlai-Holler Oct 21, 2016

Adlai-Holler Oct 21, 2016

Adlai-Holler left a comment

Adlai-Holler Oct 21, 2016

george-gw Oct 22, 2016

Adlai-Holler commented Oct 21, 2016

george-gw commented Oct 22, 2016 •

edited

Loading

maicki Oct 22, 2016

george-gw Oct 22, 2016 •

edited

Loading

maicki Oct 22, 2016

maicki Oct 22, 2016

maicki Oct 22, 2016

maicki Oct 22, 2016

maicki Oct 22, 2016

maicki Oct 22, 2016

maicki Oct 22, 2016

maicki Oct 22, 2016

maicki Oct 22, 2016

Adlai-Holler commented Oct 24, 2016

Added selection API to ASTableNode and ASCollectionNode #2453

Added selection API to ASTableNode and ASCollectionNode #2453

Conversation

george-gw commented Oct 21, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Adlai-Holler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Adlai-Holler commented Oct 21, 2016

george-gw commented Oct 22, 2016 • edited Loading

Choose a reason for hiding this comment

george-gw Oct 22, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Adlai-Holler commented Oct 24, 2016

george-gw commented Oct 21, 2016 •

edited

Loading

george-gw commented Oct 22, 2016 •

edited

Loading

george-gw Oct 22, 2016 •

edited

Loading