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

Conversation

garrettmoon
Copy link
Contributor

No description provided.

Copy link
Contributor

Choose a reason for hiding this comment

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

This seems a little sparse, but smart :). Multiple loads and unloads of a node is a first for the public ASDK, something we had some challenges implementing a while back when building prototypes of view reuse. I assume each time will use layerBlock or viewBlock accordingly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It appears so.

@appleguy
Copy link
Contributor

appleguy commented Oct 5, 2015

This is looking good, should be able to merge soon...any ideas on testing this? A couple unit tests would be warranted particularly for a change of this size. We're tracking unit test coverage now, and with the big new methods I think that will drop unless we add a couple, aside from the legitimate reasons to have them. Snapshot test may be appropriate using colored boxes. Try enabling shouldRasterize on the layout tests, which are already running against images of colored boxes?

@garrettmoon garrettmoon force-pushed the addSupportForDisablingAndEnablingShouldRasterize branch from c24e1a1 to 1a8f669 Compare October 6, 2015 18:47
@garrettmoon garrettmoon closed this Oct 6, 2015
@garrettmoon garrettmoon reopened this Oct 6, 2015
@appleguy
Copy link
Contributor

appleguy commented Oct 6, 2015

This diff is definitely on the scarier side for me, but I really trust your work!

appleguy added a commit that referenced this pull request Oct 6, 2015
…blingShouldRasterize

Adds support for disabling and re-enabling should rasterize.
@appleguy appleguy merged commit ed7f227 into facebookarchive:master Oct 6, 2015
peter-iakovlev pushed a commit to peter-iakovlev/AsyncDisplayKit that referenced this pull request Jan 9, 2018
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.

3 participants