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

transitionLayoutWithSizeRange: should invalidate the calculated layout #463

Closed
Adlai-Holler opened this issue Jul 21, 2017 · 2 comments
Closed

Comments

@Adlai-Holler
Copy link
Member

Otherwise, subsequent calls to layoutThatFits: may return a stale (pre-transition) layout.

transitionLayout is effectively a setNeedsLayoutAnimated: so this makes conceptual sense. transitionLayoutWithAnimation: already calls setNeedsLayout, I believe to ensure it gets an appropriate constrained size from _constrainedSizeForLayoutPass. If I'm wrong about that, then we can simply move that line down into transitionLayoutWithSizeRange:. If I'm right about that, we can insert a [self invalidateCalculatedLayout] in transitionLayoutWithSizeRange:. That's two invalidations, but they're cheap and fast.

@maicki @nguyenhuy (when you're back from vacation) @appleguy Does this make sense?

@Adlai-Holler
Copy link
Member Author

Closed by #464

@Adlai-Holler
Copy link
Member Author

Adlai-Holler commented Jul 27, 2017

Unfortunately this isn't fixed completely by #464. We have a case with the following sequence of events for a node. Here's a trace where a navigation bar is trying to change from 128 height to 84 height:

screen shot 2017-07-26 at 5 25 44 pm

The Node <PISearch… lines near the bottom are explicit calls to layoutThatFits: from client code. The client explicitly measures the node and sets its frame as part of viewWillLayoutSubviews. He passes an unconstrained size range and we return him the pending layout that was generated before the layout transition.

As you can see the height reverts to 128 because of the stale layout that we returned. The layout transition call did not invalidate the node's layout (bump the version) because the size range passed to transitionLayout was a fixed size range that did not match the constrainedSizeForCalculatedLayout present in the logic added in #464. I created the linked PR to unconditionally invalidate the nodes layout in transitionLayout, unless the transition is canceled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant