Skip to content
This repository has been archived by the owner on Feb 2, 2023. It is now read-only.

[WIP][ASLayoutTransition] Relayout a node if one of its subnodes has a new size after layout transition #2862

Closed

Conversation

nguyenhuy
Copy link
Contributor

@nguyenhuy nguyenhuy commented Jan 4, 2017

  • Revert 7872cfb because a transition isn't complete until its CAAnimation finishes. If _transitionInProgress flag is turned off before that, any layout pass occurs afterward, but before -transitionContext:didComplete: is called, can erase all pending states of the transition, causing old subviews to remain in the hierarchy forever.
  • If a layout transition causes the node to have a different size, notify its supernode to resize. Currently we're already doing this with ASCellNode via measurement completion block, but not with normal ASDisplayNode. ASScrollNode in particular isn't notified and thus doesn't update its content size.

@maicki, @garrettmoon, @appleguy, @Adlai-Holler: Would love to have your feedbacks here.

@hannahmbanana Please help me to test this diff with Pinterest master. I already did that, as well as with ASDKLayoutTransition example and the sample projects in #2841 and #2816. However, with all the branch and context switchings, I may overlooked something. Gonna test it more tomorrow!

@nguyenhuy nguyenhuy force-pushed the HNLayoutTransitionBugs branch from 27cf9df to f88f4bb Compare January 4, 2017 22:33
@nguyenhuy nguyenhuy force-pushed the HNLayoutTransitionBugs branch from f88f4bb to 14ce2bd Compare January 4, 2017 22:36
// Hold onto __instanceLock__ until at least after the animation of this transtion kicked off,
// to make sure this transition isn't invalidated after it passed the abort test below
__instanceLock__.lock();
if ([self _shouldAbortTransitionWithID:transitionID]) {
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this unlock here before the return?

Copy link
Contributor Author

@nguyenhuy nguyenhuy Jan 5, 2017

Choose a reason for hiding this comment

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

Good catch! It's very hard to hit this line (excepts for super busy nodes, like closeup) but it's a legitimate mistake of mine!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@hannahmbanana
Copy link
Contributor

I tested this out on my product code and it doesn't work in my case. Huy has the repro steps and will double check to see if it's a bug on my side or ASDK's side.

@nguyenhuy nguyenhuy changed the title [ASLayoutTransition] Relayout a node if one of its subnodes has a new size after layout transition [WIP][ASLayoutTransition] Relayout a node if one of its subnodes has a new size after layout transition Jan 5, 2017
@nguyenhuy nguyenhuy changed the title [WIP][ASLayoutTransition] Relayout a node if one of its subnodes has a new size after layout transition [ASLayoutTransition] Relayout a node if one of its subnodes has a new size after layout transition Jan 5, 2017
@nguyenhuy nguyenhuy changed the title [ASLayoutTransition] Relayout a node if one of its subnodes has a new size after layout transition [WIP][ASLayoutTransition] Relayout a node if one of its subnodes has a new size after layout transition Jan 5, 2017
@nguyenhuy
Copy link
Contributor Author

Closing this in favor of an overhaul of the layout system.

@nguyenhuy nguyenhuy closed this Jan 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants