Instagram / IGListKit

A data-driven UICollectionView framework for building fast and flexible lists.
https://instagram.github.io/IGListKit/
MIT License
12.85k stars 1.54k forks source link

Update [IGListSectionType didSelectItemAtIndex:] to pass back the cell #382

Closed jessesquires closed 7 years ago

jessesquires commented 7 years ago

We have a situation where we want to animate cell subviews when tapped.

The hook for this is -[IGListSectionType didSelectItemAtIndex:], however it doesn't pass back the cell.

It would be nice to do:

- (void)didSelectItemAtIndex:(NSInteger)index cell:(UICollectionViewCell *)cell {
     [cell animateSubview];
}

Current possible workarounds:

We can't do this:

- (void)didSelectItemAtIndex:(NSInteger)index {
    MyCell *cell = [self cellForItemAtIndex:index];
    // do stuff with cell
}

Because cellForItemAtIndex: will dequeue a new cell.

jessesquires commented 7 years ago

Actually, a better spelling for this would be:

- (void)didSelectCell:(__kindof UICollectionViewCell *)cell atIndex:(NSInteger)index
mattyohe commented 7 years ago

Because cellForItemAtIndex: will dequeue a new cell.

cellForRowAtIndexPath will not dequeue a new cell.

As the generated headers say:

public func cellForRowAtIndexPath(indexPath: NSIndexPath) -> UITableViewCell? // returns nil if cell is not visible or index path is out of range

Is this not the case for collection views?

edit: It appears that collection views have the same documentation: The cell object at the corresponding index path or nil if the cell is not visible or indexPath is out of range.

rnystrom commented 7 years ago

@mattyohe UICollectionView docs say the same

The cell object at the corresponding index path or nil if the cell is not visible or indexPath is out of range.

@jessesquires actually we can probably close this. The UICollectionViewDelegate API is

- (void)collectionView:(UICollectionView *)collectionView didSelectItemAtIndexPath:(NSIndexPath *)indexPath;

So we'd still have to use -[UICollectionView cellForItemAtIndexPath:] to get the cell 😄

edit: I'm still open to changing this to be more convenient, but at least there aren't any bugs/concerns w/ the current setup.

jessesquires commented 7 years ago

Oops 😳 derp.

Yep, we can use the existing method: https://github.com/Instagram/IGListKit/blob/master/Source/IGListCollectionContext.h#L49-L50