Skip to content

Comments

Only call -layout and -layoutDidFinish if the node is already loaded#285

Merged
nguyenhuy merged 4 commits intoTextureGroup:masterfrom
nguyenhuy:HNLayoutBeforeLoading
May 19, 2017
Merged

Only call -layout and -layoutDidFinish if the node is already loaded#285
nguyenhuy merged 4 commits intoTextureGroup:masterfrom
nguyenhuy:HNLayoutBeforeLoading

Conversation

@nguyenhuy
Copy link
Member

After facebookarchive/AsyncDisplayKit#3263, a layout pass can be triggered even before a node is loaded. This is to ensure the node has valid bounds to fetch its content as soon as it enters preload state, regardless of its backing store.

However, the API contracts of -layout and -layoutDidFinish state that those methods are triggered by the view's layoutSubviews. That means the node must be loaded by the time they are called.

This PR adds a node loaded check to __layout that ensures those 2 methods are called only if the condition meets.

@nguyenhuy nguyenhuy force-pushed the HNLayoutBeforeLoading branch from 11d6526 to 5315e3f Compare May 17, 2017 14:05
@nguyenhuy nguyenhuy requested review from Adlai-Holler and maicki May 17, 2017 14:05
Copy link
Contributor

@maicki maicki left a comment

Choose a reason for hiding this comment

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

Change looks good to me!

We should finally push on finding a better way to have layout code path go through one path ...

@nguyenhuy
Copy link
Member Author

@maicki Agreed...

@ghost
Copy link

ghost commented May 19, 2017

🚫 CI failed with log

@nguyenhuy nguyenhuy merged commit 6aa5ad7 into TextureGroup:master May 19, 2017
bernieperez pushed a commit to AtomTickets/Texture that referenced this pull request Apr 25, 2018
…extureGroup#285)

* Only call -layout and -layoutDidFinish if the node is already loaded

* Minor change

* Update CHANGELOG
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants