Asyncdisplaykit: ASViewController/ASPagerNode layout bug since version 1.9.81

Created on 19 Jul 2016  路  25Comments  路  Source: facebookarchive/AsyncDisplayKit

Hi guys,

Since upgrading to 1.9.81 I've got a weird bug that appears in only one of my view controllers: when it's pushed on a UINavigationController, it is first correctly laid out, but then when the push animation finishes, some nodes inside are slightly shifted up, leaving a blank at the bottom of the screen.
A few things to note:

  • The affected nodes are inside a horizontal ASPagerNode. Other nodes in the hierarchy doesn't seem to be affected.
  • The pager node itself fills the whole screen, so it seems correctly laid out (this is what I want). Only its children are incorrect.

This "bug" seems to be related to #1860: it is observed in 82233af but not in 33620b9. It's probably a combinaison of a change in ASDK and something I'm doing wrong. I'm trying to put up a small demo project to reproduce this, but it's not finished yet. In the mean time, do you have any ideas what could be causing this behaviour, given the changes in #1860? Anything I could try?

Cheers,
Flo

P2

All 25 comments

Unfortunately I'm not able to reproduce the bug in a demo project, however I have a bit more details:
screen shot 2016-07-20 at 10 53 03 am
This is the view hierarchy with clipped content shown. Filled with blue is the ASPagerNode. The half-transparent rectangle is the _ASCollectionViewCell (for which I probably don't have access?). Finally the transparent rectangle with blue borders is my custom ASCellNode.
My ASCellNode has the correct size that I set using constrainedSizeForNodeAtIndexPath, however its frame doesn't have the correct origin, which is aligned with the _ASCollectionViewCell. Any idea why the _ASCollectionViewCell would be higher (and taller) than the ASPagerNode itself?

Thanks!

Edit: Here's the (almost) complete view hierarchy:
screen shot 2016-07-20 at 3 39 39 pm
And the node hierarchy goes:

- UINavigationController
  - ASViewController
    - CustomNode: ASDisplayNode
      - 2 custom nodes: ASDisplayNode
      - ASPagerNode <- This is correctly laid out
        - ASCellNode <- This has the same size as the ASPagerNode, but frame.origin != 0
          - CustomNode: ASDisplayNode
            - ASCollectionNode
              ...

I have a similar bug : an ASPagerNode whose children are laid out differently since I updated to 1.9.81. It looks like there are top/bottom insets surrounding its children. Will report back if I find anything !
simulator screen shot 20 jul 2016 12 10 42
screen shot 2016-07-20 at 12 11 02

I have the same issue with ASPagerNode as ASViewController' root view too.

I use this as a workaround, and I don't think it's a good solution.
self. .view.contentInset = UIEdgeInsetsMake(32, .0f, .0f, .0f);

@flovouin @koheku @dssheng Would it be possible of one of your guys provide us with a sample app so we can reproduce the issue / regression? That would be awesome! Thanks!

@flovouin @koheku @dssheng Hey, can you please try out ASDK master? Is it still happening if you build against it? Thanks!

Thanks @maicki and @hannahmbanana for your replies! I just tried the same code using master and the behaviour is different:

  • When the view appears the cell nodes are correctly laid out, and they stay this way until viewDidAppear is called on the controller.
  • After that, the cell nodes shift to the incorrect position (it seems to be the same as with 1.9.81) without any animation, but then animate back to the correct layout.
  • When this unwanted animation finishes, the cell nodes are correctly laid out and life goes on as if it never happened. ;-)
    So there's some "improvement" but the bug is still there. No work is performed on my side in viewWill/DidAppear.

I wasn't able to reproduce the bug with a simple demo project. I'll try to code something closer to my actual node hierarchy. Unfortunately I am away next week so this is going to take some time. Sorry I can't be of any more help right now, but please tell me if I can do anything else to put you on the right track.

Cheers,

@flovouin Sorry about this issue! Can you try the following: on your ASCellNode subclass, set shouldAnimateSizeChanges = NO; and see if that removes the unwanted animation?

@Adlai-Holler
here is my code sample, using 1.9.81

- (ASCellNode *)pagerNode:(ASPagerNode *)pagerNode nodeAtIndex:(NSInteger)index
{
    ASCellNode* controllerCellNode = [[ASCellNode alloc] initWithViewControllerBlock:^UIViewController * _Nonnull{

        switch (index) {
            case 0: {
                ACKFeedListViewController* feedListVC = [ACKFeedListViewController new];
                return feedListVC;
            }
                break;

            default:
                break;
        }
    } didLoadBlock:^(ASDisplayNode * _Nonnull node) {

    }];

    controllerCellNode.preferredFrameSize = pagerNode.bounds.size;
    controllerCellNode.shouldAnimateSizeChanges = NO;

    return controllerCellNode;
}

and shouldAnimateSizeChanges not working.

@Adlai-Holler Sorry for the delay. I checked out the latest master and the issue is back to its original state (no animation, as described in my first post), no matter the value of shouldAnimateSizeChanges for the ASCellNodes.

I also get this warning in the debugger:
the behavior of the UICollectionViewFlowLayout is not defined because: the item height must be less than the height of the UICollectionView minus the section insets top and bottom values, minus the content insets top and bottom values. The relevant UICollectionViewFlowLayout instance is <ASPagerFlowLayout: 0x13a2c5e70>, and it is attached to <ASCollectionView: 0x137118600; baseClass = UICollectionView; frame = (0 0; 375 603); clipsToBounds = YES; gestureRecognizers = <NSArray: 0x136fd7a90>; layer = <<ASPagerNode: 0x13a2cf450>, layer = <_ASDisplayLayer: 0x13a2790a0>>; contentOffset: {0, 0}; contentSize: {1500, 667}> collection view layout: <ASPagerFlowLayout: 0x13a2c5e70>. Make a symbolic breakpoint at UICollectionViewFlowLayoutBreakForInvalidSizes to catch this in the debugger.

@Adlai-Holler @maicki Good news, I was able to put together a sample project: https://github.com/flovouin/ASDK-IncorrectViewControllerLayout
It seems to be somewhat related to the navigation bar _not_ being translucent. In my app I also show and hide the navbar, however this is not necessary to observe the unwanted behaviour. This sample project is not a 100% the behaviour I'm observing in my app, but close. In my actual app, there's an unwanted animation at the end of the transition, as mentioned in one of my previous comments. I wasn't able to reproduce this animation, however a glitch during the transition is still present on master.
Please look at the Podfile in the repo, which briefly describes the observed behaviour for each version of ASDK (1.9.80, 1.9.81, and master):

  • ASDK 1.9.80: The height of the cell is too large (667 vs. 603) and overflows at the top and bottom of the container. The actual cell has the same size as the wrapping _ASCollectionViewCell. This results in a layout warning (see my previous comment).
  • ASDK 1.9.81: The height of the cell is correct (603), but is shifted up at the end of the transition, which leaves a blank at the bottom of the screen. The wrapping _ASCollectionViewCell is larger (identical to 1.9.80). The same warning as before is emitted.
  • ASDK 1.9.90: The height of the cell is correct (603), as well as the height of the _ASCollectionViewCell. However a blank at the top of the view is visible during the transition. The final layout is correct and no warning is emitted.

You'll also find a screenshot of the view hierarchy for each version.

As always, happy to help with more info if needed!

Hi guys,
I've updated the test project to work with the ASDK2-beta, and I can confirm that the bug/glitch is still there. This is preventing me from upgrading to anything above 1.9.80. A work around for me could probably be to set the navigation bar to translucent, but this would imply changing quite a bit of code.
I wouldn't mind investigating this myself, but to be honest I'm not sure what is causing this.
Cheers,

@maicki can you take a look at this?

Thanks for the bump, I can confirm that I'm still interested in this. :-)

Just merged fix into master, closing for now but can you verify @flovouin ?

@garrettmoon The visual glitch is gone (but actually it had already gone when I upgraded to aefff5). However I still get the following warning when the pager node first appears:

The behavior of the UICollectionViewFlowLayout is not defined because:
the item height must be less than the height of the UICollectionView minus the section insets top and bottom values, minus the content insets top and bottom values.
The relevant UICollectionViewFlowLayout instance is <ASPagerFlowLayout: 0x15ddafcc0>, and it is attached to <ASCollectionView: 0x15e8a7800; baseClass = UICollectionView; frame = (0 0; 375 603); clipsToBounds = YES; gestureRecognizers = <NSArray: 0x17424f960>; layer = <ASPagerNode-Layer: 0x174156c60; node = <ASPagerNode: 0x15ddaf8e0>>; contentOffset: {0, 0}; contentSize: {0, 667}> collection view layout: <ASPagerFlowLayout: 0x15ddafcc0>.
Make a symbolic breakpoint at UICollectionViewFlowLayoutBreakForInvalidSizes to catch this in the debugger.

Unfortunately I won't have time to look into it for the next few days, but I'll keep you posted when I get more info on this.

@garrettmoon The initial issue should be fixed as @flovouin says. The other issue around UICollectionViewFlowLayout is a different issue and we should create a new issue for that.

Hi, Just to provide an update, this issue is still present in 2.0-rc.1, for me after I move away from the view controller containing ASPagerNode and come back to it, the cells inside ASPagerNode swift -32px upwards.

@stapleup can you create a new issue for that?

@garrettmoon that won't be necessary, for me setting the automaticallyAdjustScrollViewInsets to false resolved the issue. Thanks for the reply :)

Ok, going to close this one out then :)

I will also add more context what is actually going on within the PR linked above pretty soon.

To finally and hopefully closed this issue out we also updated our documentation around ASPagerNode for the usage as full screen root node and if the navigationbar.translucent property is set to YES: http://asyncdisplaykit.org/docs/containers-aspagernode.html

Thanks, following the updated documentation, I tried setting self.automaticallyAdjustsScrollViewInsets = NO, but it didn't remove the warning. And actually, by looking more closely, it looks like the cells were still animated as "sliding down" from outside (above) the pager node bounds (but ended up in the correct position). As this was happening during the presentation of the view controller, this wasn't as shocking.

HOWEVER, although my navigation bar is not translucent, calling waitUntilAllUpdatesAreCommitted during viewWillAppear did solve both the unwanted animation and the warning.

So I guess that as far as I'm concerned, the issue is finally solved! 馃檪 If that's not how you were expecting it to be solved, tell me and I can run some other tests, but I think that @maicki will be happy to forget about this issue. 馃槃
Thanks again for the hard work!

@flovouin Just fyi and sorry I forgot to mention above: this PR needs to be merged so that the warnings will go away as well as the cell's are not animating down: https://github.com/facebook/AsyncDisplayKit/pull/2736

Was this page helpful?
0 / 5 - 0 ratings