This repository has been archived by the owner on Feb 2, 2023. It is now read-only.
[ASEnvironment] Don't relayout as a result of clearing a traitCollection's context #1759
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I'm not completely sure this change is the best solution. Here is context: An ASEnvironmentTraitCollection has a pointer to an optional context that an ASVC is the owner of. When the ASVC is dealloc'ed, we go through all subnodes of the VC and clear out the context so that the struct isn't holding on to a garbage pointer. Setting the traitCollection on ASCollectionNode/ASTableNode causes the cells to relayout if the trait collection changed (this is a special case for these two nodes since their cells are not actually subnodes). Setting the context to nil registered as a trait collection change and was causing a layout even as we were dealloc'ing the VC. The logic I'm implementing here is: If the trait collection changed AND the displayContext did not, then we should relayout. If the trait collection changed AND the new displayContext is non-nil then we should layout In the case where the trait collection change was caused soley by the displayContext going from non-nil to nil, then we should NOT layout. ``` // At this point we know that the two traits collections are NOT equal for some reason BOOL needsLayout = (oldTraits.displayContext == currentTraits.displayContext) || currentTraits.displayContext != nil; ``` Is there a better place/safer way to do this?
ghost
added
the
|
@rcancro interesting...this seems safe to me, albeit slightly unusual. Maybe we should have a comparator method of some kind that decides if a layout update is required for any given old / new traits. |
aimalygin
pushed a commit
to aimalygin/AsyncDisplayKit
that referenced
this pull request
Sep 16, 2020
aimalygin
pushed a commit
to aimalygin/AsyncDisplayKit
that referenced
this pull request
Sep 16, 2020
* master_up: (29 commits) [ASDisplayNode] Resolve background color using system trait collection (facebookarchive#1777) Prevent crashing during non critical logging at rotation (facebookarchive#1770) Fix calling CALayer out of the main thread (facebookarchive#1762) Pruning ASExperimentalRemoveTextKitInitialisingLock code. (facebookarchive#1766) Fix typo in examples/README.md (facebookarchive#1759) Lint podspec on all pull requests (facebookarchive#1758) Link to IGListDiffKit in our IGListKit subspec (facebookarchive#1756) Omit UIUserInterfaceLevel from tvOS build (facebookarchive#1757) Add empty ASViewController initializer to facilitate subclassing (facebookarchive#1754) Remove references to xcpretty-travis-formatter (facebookarchive#1755) Simplify push_back `GroupNotify` (facebookarchive#1736) First pass at supporting new traits (facebookarchive#1568) Use `queue` in `ASMainSerialQueue` (facebookarchive#1738) Bump up IGListKit version (facebookarchive#1749) [ASCollectionLayout] Fix element lookup for supplementary attributes (facebookarchive#1707) Delete unused macro in `ASControlNode` (facebookarchive#1734) Improve recursive unfair lock: (facebookarchive#1742) [Docs] minor fixes in `Layout` (facebookarchive#1735) Delete unused header in `ASRunLoopQueue` (facebookarchive#1737) Fix tint color dead lock facebookarchive#1731 (facebookarchive#1732) ...
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm not completely sure this change is the best solution. Here is context:
An ASEnvironmentTraitCollection has a pointer to an optional context that an ASVC is the owner of. When the ASVC is dealloc'ed, we go through all subnodes of the VC and clear out the context so that the struct isn't holding on to a garbage pointer.
Setting the traitCollection on ASCollectionNode/ASTableNode causes the cells to relayout if the trait collection changed (this is a special case for these two nodes since their cells are not actually subnodes). Setting the context to nil registered as a trait collection change and was causing a layout even as we were dealloc'ing the VC.
The logic I'm implementing here is:
If the trait collection changed AND the displayContext did not, then we should relayout.
If the trait collection changed AND the new displayContext is non-nil then we should layout
In the case where the trait collection change was caused soley by the displayContext going from non-nil to nil, then we should NOT layout.
Is there a better place/safer way to do this?