Iglistkit: Animating cell size change don't-know-how

Created on 1 Feb 2017  路  13Comments  路  Source: Instagram/IGListKit

Hi IGListGang,

I'm facing one odd problem - but I admit it is the lack of my experience with IGListKit. So please, if you find it interesting enough, answer it, pretty please. I apologise if you had similar question, I glanced all of them without success.

I must have a recipients selector, a horizontal collection view with circles representing contacts and with initials inside. When user clicks on one, it should expand a bit and display some additional information.

Like this:

explain

When user clicks on the cell, it should glide nicely into expanded position and vice versa; preferably with some custom springy-thingy dance. You can take a look into "Classic" approach in provided project, quite straight-forward, less then one screen example...

Then I said, ok let's IGListify this. I have decided to go with single section controller with multiple items inside, because it feels natural to put NSInteger selectedIndex; that will keep information about which model is in selected state, inside section controller.

But, animation was jerky and eye-poking.

Then I have created the same example with one item per section controller (I saw that almost all provided samples in your project use one item per controller), and with some mumbo-jumbo-selectedIndex-delegate-weak-reference, it worked. ...But with the same result...

Here I have provided small project that runs all three examples. Again, please, show me the right path, it feels like it should work, only I don't know where to put this animating thing and how.... Also if you have a better design in mind, I would really appreciate it. Please remove performBitchUpdates if possible. (:-) _Hi Ryan!_)

RecipientsViewController.zip

br
IGor

New issue checklist

General information

  • IGListKit version: HEAD
  • iOS version(s): latest
  • CocoaPods/Carthage version: latest
  • Xcode version: latest
  • Devices/Simulators affected:
  • Reproducible in the demo project? (Yes/No): Yes
  • Related issues:
question

All 13 comments

@iostriz thanks for the sample project! TIL that you can animate UICollectionView batch updates w/ the spring animation API. That's pretty cool.

The reason that this doesn't work is b/c IGListKit asynchronously coalesces updates. That is, it collects multiple batch updates during the runloop and unloads them at once. So the call to adapter.performUpdates(animated: true) doesn't actually trigger collection view's batch updates until later (async dispatch calls).

We might be able to do something about this, though. I wonder if we can get the currently stacked animations when batch updates is called, store them somewhere, then hand them to the IGListAdapterUpdater so it can apply them later.

Tbh I have no idea if CATransaction has a way to do this? It might be private-ish (like getting/setting the transaction properties).

Time to dig into some Core Animation...

Hi Ryan, thanks for taking a peek into project.
I would be satisfied with the present situation, only if it does not behave bad in some cases.
E.g. selecting a circle on the left side of the currently selected one works well, but selecting the one on the right side animates poorly.

@iostriz @rnystrom I did some digging on your sample code:

selecting a circle on the left side of the currently selected one works well, but selecting the one on the right side animates poorly.

  • IMHO, the reason why this happens is that instead of updating(reloading) the original cell, IGListCollectionView (actually UICollectionView in general) deletes the old cell and inserts a new one. See here.
  • Moreover, the reason why this code
[UIView animateWithDuration:0.35 delay:0.0 usingSpringWithDamping:0.7 initialSpringVelocity:0.0 options:0 
  animations:^{
    [_collectionView performBatchUpdates:nil completion:nil];
  } completion:nil];

works like magic is that, since the update block is nil, the only thing collectionView does here is invalidating its layout.

So we might be able to bring your springy animation back once we add #360.

@iostriz ok I did more digging on your setup and got it working a little better, but its not perfect.

A couple recommendations:

  • If the expected behavior is that only a single item can be "expanded" at a time, then you should use a single section controller w/ many items inside it. Why? Because the state is shared among many items (e.g. expanding one collapses another). Sending events back up to the parent just to trickle that state back down breaks the point of keeping section controllers isolated.
  • @zhubofei is right that reloading in batch updates doesn't work b/c we convert them to deletes+inserts, however I see you're using reloadInSectionController:atIndexes: w/out the batch update block, which turns straight into a batch update.

So what I did was update -[RecipientsSectionController didSelectItemAtIndex:] to this:

- (void)didSelectItemAtIndex:(NSInteger)index
{
    NSMutableIndexSet *indexes = [NSMutableIndexSet indexSetWithIndex:_model.selectedIndex];
    [indexes addIndex:index];
    _model.selectedIndex = index;

    [UIView
     animateWithDuration:0.35
     delay:0.0
     usingSpringWithDamping:0.7
     initialSpringVelocity:0.0
     options:0
     animations:^{
         [self.collectionContext reloadInSectionController:self
                                                 atIndexes:indexes];
     }
     completion:nil];
}

Like I said its not perfect, sometimes the animation doesn't spring and just fades, which is boring.


I've honestly never seen the spring animations work like this, so its pretty cool! I wonder if we should add another IGListCollectionContext that calls an empty batch update internally to do things like this? @zhubofei I attempted this w/ invalidating the layout, but it doesn't look like we can animate that.

This made me realize that the reload methods are worth keeping, which is contrary to #392. I think that delete+insert+move have to be done in a batch update, but reloads are valid outside of that scope, especially for stuff like this.

This issue was really valuable!

@rnystrom Right, animation over layout invalidation doesn't work (cells spring just fine, but there are some random white views flashing in the background). Don't know why 馃.

I wonder if we should add another IGListCollectionContext that calls an empty batch update internally to do things like this

This would be great! Actually, I think we can allow sending in an update block as well. So that we can perform some layout-only updates. Something like:

[self.collectionview performBatchUpdates:^{
    [self.collectionview.collectionViewLayout invalidateLayout];
    [self.collectionview setCollectionViewLayout: newLayout animated:YES];
} completion:^(BOOL finished) {
 ...
}];

@rnystrom Glad that I have a "valuable issue"... Maybe you can buy me a sandwich? :-)
Just kidding, this framework is much more valuable to me then this issue is to you.
Looking fwd to the updates on this topic...

@zhubofei good point. Reminds me that the layout that we're using in #450 caches values, so you can't just change w/e is returned in the size methods, you have to invalidate the layout.

Maybe the layout methods need to be smarter, something like:

- (void)invalidateLayoutForSectionController:(IGListSectionController<IGListSectionType> *)sectionController
  indexes:(NSIndexSet *)indexes
  animated:(BOOL)animated;

And the implementation inside IGListAdapter looks something like:

- (void)invalidateLayoutForSectionController:(IGListSectionController<IGListSectionType> *)sectionController
  indexes:(NSIndexSet *)indexes
  animated:(BOOL)animated {
  NSArray<NSIndexPath *> paths = // create paths from SC + indexes
  UICollectionViewLayoutInvalidationContext *context = // new from layout
  [context invalidateItemsAtIndexPaths:paths];

  void (^updates)() = ^{
    [self.collectionView.collectionViewLayout invalidateLayoutWithContext:context];
  };

  if (animated) {
    [self.collectionView performBatchUpdates:updates completion:nil];
  } else {
    updates();
  }
}

Some thoughts:

  • Idk if we want to add a completion option
  • I tested this w/ @iostriz's example just now and it still does the spring animation. This is probably how we want to do layout invalidation w/ animations.

@rnystrom Glad you find this valuable. Maybe you can buy me a sandwich! :-)
Just kidding, this framework is more valuable to me than this issue is to you.
Looking fwd to the updates on this.

@rnystrom

Idk if we want to add a completion option

probably not 馃槀.

@rnystrom Thanks Ryan for fixing #499. Looking fwd to try it out.

Like mentioned in https://github.com/roberthein/BouncyLayout/issues/6 I have trouble using this new feature with a custom UICollectionViewFlowLayout. When changing a section controller color and height, the color won't change unless I call adapter.reloadObjects([]) and the height won't change : a strange space equal to what the cell should have increased its height of is created like 6 cells more down.

simulator screen shot 13 mai 2017 a 21 19 01
simulator screen shot 13 mai 2017 a 21 19 17

It looks like a cache problem in the layout, but I don't see any cache mecanism in BouncyLayout.

Please find attach my example :
BouncyLayout-master.zip

Maybe I'm doing something wrong. I start to wonder if a "IGListCollectionView**Flow**Layout" won't be necessary in the future.

I progressed a bit, I found a way to do it but it's a bit dirty (see delegate method func didSelect(_ style: CellStyle) and it required that I change BouncyLayout class UIDynamicAnimator variable to open. This animator was the guilty one for caching old values and has to be empty manually before each addition/deletion/update of cells. Any insight regarding your experience guys ?

New code :
BouncyLayout-master.zip

@LivioGama do you mind opening a new issue to track this?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

rnystrom picture rnystrom  路  3Comments

iwasrobbed picture iwasrobbed  路  3Comments

kanumuri9593 picture kanumuri9593  路  3Comments

joseph-francis picture joseph-francis  路  3Comments

racer1988 picture racer1988  路  3Comments