Skip to content
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

Interactive Reordering #976

Closed
wants to merge 29 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
fc5659b
Interactive Reordering - initial support
jverdi Oct 24, 2017
345385d
Interactive Reordering - Linter cleanup
jverdi Oct 24, 2017
8f7fb5a
Interactive Reordering - addressing minor feedback
jverdi Oct 24, 2017
a3c61c0
Interactive Reordering - remove adapter delegate, rely on section con…
jverdi Oct 24, 2017
d9bf7fc
Interactive Reordering - moveItemAtIndexPath cleanup
jverdi Oct 24, 2017
54e0b07
Interactive Reordering - list adapter datasource feedback
jverdi Oct 25, 2017
d3bedfc
Interactive Reordering - Tests
jverdi Oct 25, 2017
2c8a42d
Interactive Reordering - Stacked Section Controller support
jverdi Oct 28, 2017
f6dce23
Merge branch 'master' into feature/interactive-reordering
jverdi Oct 28, 2017
893a8be
Merge branch 'master' into feature/interactive-reordering
jverdi Oct 28, 2017
9f355f1
Interactive Reordering - Stacked Section Controller Tests
jverdi Oct 28, 2017
3a70048
Interactive Reordering - Update Changelog
jverdi Oct 28, 2017
5b20441
Examples: pod umbrella file is missing a header reference
jverdi Oct 28, 2017
42188c0
Interactive Reordering - removing assertion that’s no longer necessary
jverdi Oct 28, 2017
16e35b3
Interactive Reordering - swizzle layout methods to reorder past last …
jverdi Nov 3, 2017
6e74899
Interactive Reordering - cleanup stray NSLog
jverdi Nov 3, 2017
fd7d721
Merge branch 'master' into feature/interactive-reordering
jverdi Nov 21, 2017
50b81a4
Interactive Reordering - create a separate moveDelegate for section m…
jverdi Nov 21, 2017
ee69e66
Interactive Reordering - swizzling associated list adapter should be …
jverdi Nov 21, 2017
4ce6401
Interactive Reordering - cleanup tests, run swizzling through tests
jverdi Nov 21, 2017
7aaf51e
Interactive Reordering - make moveDelegate available on iOS 9+ only
jverdi Nov 22, 2017
400a0bc
Interactive Reordering - style feedback
jverdi Nov 22, 2017
1ecb145
Interactive Reordering - respect layout’s w/custom invalidationContex…
jverdi Nov 22, 2017
85d8849
Interactive Reordering - more iOS 9+ markers/conditionals to suppress…
jverdi Nov 22, 2017
f63cb30
Interactive Reordering - per feedback, adapter method is heavy-handed…
jverdi Nov 22, 2017
35b1c4e
Interactive Reordering - style
jverdi Nov 22, 2017
e0b3773
Interactive Reordering - respect layout’s w/custom flowlayout invalid…
jverdi Nov 22, 2017
1d808f0
Merge branch 'master' into feature/interactive-reordering
jverdi Nov 22, 2017
fad162a
Merge branch 'master' into feature/interactive-reordering
jverdi Nov 22, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,10 @@ extension GridItem: ListDiffable {
final class GridSectionController: ListSectionController {

private var object: GridItem?
private let isReorderable: Bool

override init() {
required init(isReorderable: Bool = false) {
self.isReorderable = isReorderable
super.init()
self.minimumInteritemSpacing = 1
self.minimumLineSpacing = 1
Expand Down Expand Up @@ -83,6 +85,10 @@ final class GridSectionController: ListSectionController {
self.object = object as? GridItem
}

override func canMoveItem(at index: Int) -> Bool {
return isReorderable
}

override func moveObject(from sourceIndex: Int, to destinationIndex: Int) {
guard let object = object else { return }
let item = object.items.remove(at: sourceIndex)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ import IGListKit
final class UserSectionController: ListSectionController {

private var user: User?
private let isReorderable: Bool

required init(isReorderable: Bool = false) {
self.isReorderable = isReorderable
super.init()
}

override func sizeForItem(at index: Int) -> CGSize {
return CGSize(width: collectionContext!.containerSize.width, height: 55)
Expand All @@ -36,4 +42,7 @@ final class UserSectionController: ListSectionController {
self.user = object as? User
}

override func canMoveItem(at index: Int) -> Bool {
return isReorderable
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -108,21 +108,13 @@ final class MixedDataViewController: UIViewController, ListAdapterDataSource {
func listAdapter(_ listAdapter: ListAdapter, sectionControllerFor object: Any) -> ListSectionController {
switch object {
case is String: return ExpandableSectionController()
case is GridItem: return GridSectionController()
default: return UserSectionController()
case is GridItem: return GridSectionController(isReorderable: true)
default: return UserSectionController(isReorderable: true)
}
}

func emptyView(for listAdapter: ListAdapter) -> UIView? { return nil }

func listAdapter(_ listAdapter: ListAdapter, canMoveObjectInSection sectionIndex: Int, at index: Int) -> Bool {
if data[sectionIndex] as? String != nil {
// for testing purposes, allow moving grid items and users, but not simple strings
return false
}
return true
}

func listAdapter(_ listAdapter: ListAdapter, moveSectionAt sourceIndex: Int, to destinationIndex: Int) {
let obj = data.remove(at: sourceIndex)
data.insert(obj, at: destinationIndex)
Expand Down
9 changes: 0 additions & 9 deletions Source/IGListAdapterDataSource.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,6 @@ NS_SWIFT_NAME(ListAdapterDataSource)

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: remove

@optional
Copy link
Contributor

Choose a reason for hiding this comment

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

just noting:

we can keep this for 3.2 to avoid breaking clients and remove for 4.0.

after this merges, let's be sure to open a new issue to track this.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jverdi could we move this to just be a new delegate on IGListAdapter called IGListAdapterMoveDelegate? Then we:

  • Don't have to use @optional at all
  • Can let it be nil by default
  • Don't add any boilerplate to clients that don't use moving (all of them at the beginning). This will saves hundreds of empty implementations within Instagram 😉


/**
Asks the datasource whether a list object should be movable as the result of interactive reordering.

@param listAdapter The list adapter sending this information.
@param sectionIndex The index of the section proposed to be moved.
@param index The index of the object proposed to be moved.
*/
- (BOOL)listAdapter:(IGListAdapter *)listAdapter canMoveObjectInSection:(NSInteger)sectionIndex atIndex:(NSInteger)index;

/**
Notifies the datasource that a list object should move as the result of interactive reordering.

Expand Down
13 changes: 2 additions & 11 deletions Source/Internal/IGListAdapter+UICollectionView.m
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,9 @@ - (UICollectionReusableView *)collectionView:(UICollectionView *)collectionView
}

- (BOOL)collectionView:(UICollectionView *)collectionView canMoveItemAtIndexPath:(NSIndexPath *)indexPath {
id <IGListAdapterDataSource> dataSource = self.dataSource;
IGAssert(dataSource != nil, @"Found a nil dataSource when requesting canMoveItemAtIndexPath for interactive reordering");

const NSInteger sectionIndex = indexPath.section;
const NSInteger itemIndex = indexPath.item;

// for ease of implementation, first we check with the listAdapter
if ([dataSource respondsToSelector:@selector(listAdapter:canMoveObjectInSection:atIndex:)]) {
return [dataSource listAdapter:self canMoveObjectInSection:sectionIndex atIndex:itemIndex];
}

// then we fall back to the sectionController method
IGListSectionController *sectionController = [self sectionControllerForSection:sectionIndex];
return [sectionController canMoveItemAtIndex:itemIndex];
}
Expand All @@ -79,7 +70,7 @@ - (void)collectionView:(UICollectionView *)collectionView moveItemAtIndexPath:(N
const NSInteger sourceItemIndex = sourceIndexPath.item;
const NSInteger destinationItemIndex = destinationIndexPath.item;
const NSInteger sourceSectionIndex = sourceIndexPath.section;
const NSInteger destinationSectionIndex = destinationIndexPath.section;
NSInteger destinationSectionIndex = destinationIndexPath.section;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: const all of it


IGListSectionController *sourceSectionController = [self sectionControllerForSection:sourceSectionIndex];
IGListSectionController *destinationSectionController = [self sectionControllerForSection:destinationSectionIndex];
Expand All @@ -104,7 +95,7 @@ - (void)collectionView:(UICollectionView *)collectionView moveItemAtIndexPath:(N
} else if (destinationItemIndex == 1 && destinationSectionIndex < sourceSectionIndex) {
// the "item" representing our section was dropped
// into the end of a destination section rather than the beginning
// so it really belongs one section before the section where it landed
// so it really belongs one section after the section where it landed
destinationSectionIndex += 1;
}

Expand Down