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.

Sign up for GitHub

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

[ASDisplayNode] -didEnterPreloadState does not need to call -layoutIfNeeded #trivial #411

Merged
merged 1 commit into from
Jul 5, 2017
Merged

[ASDisplayNode] -didEnterPreloadState does not need to call -layoutIfNeeded #trivial #411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Source/ASCollectionNode.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,10 +213,10 @@ - (void)interfaceStateDidChange:(ASInterfaceState)newState fromState:(ASInterfac

- (void)didEnterPreloadState
{
// Intentionally allocate the view here so that super will trigger a layout pass on it which in turn will trigger the intial data load.
// We can get rid of this call later when ASDataController, ASRangeController and ASCollectionLayout can operate without the view.
[self view];
[super didEnterPreloadState];
// Intentionally allocate the view here and trigger a layout pass on it, which in turn will trigger the intial data load.
// We can get rid of this call later when ASDataController, ASRangeController and ASCollectionLayout can operate without the view.
[[self view] layoutIfNeeded];
}

#if ASRangeControllerLoggingEnabled
Expand Down
4 changes: 0 additions & 4 deletions Source/ASDisplayNode.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2915,10 +2915,6 @@ - (void)didEnterPreloadState
ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__);
[_interfaceStateDelegate didEnterPreloadState];

// Trigger a layout pass to ensure all subnodes have the correct size to preload their content.
// This is important for image nodes, as well as collection and table nodes.
[self layoutIfNeeded];
Copy link
Contributor

Choose a reason for hiding this comment

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

If we remove layoutIfNeeded in this case would user with custom ASDisplayNode subclass that relied on the fact that the size was available via bounds / calculatedSize suddenly get an undefined behavior?

They would have to basically do the same as ASTableNode and ASCollectionNode and explicitly allocating the view and trigger a layout if needed if they would like to have the old behavior?

Copy link
Member Author

Choose a reason for hiding this comment

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

@maicki Since this sets it in a downward propagation, it wouldn't necessarily have set the bounds on self, unless the parent had done so first.

Assuming the parent had done so, then yes this could be a behavior change. However, using .calculatedSize actually will not change in behavior.

I think it is quite rare to override this method, and even more rare to use .bounds in it. One potential option could be to throw an assertion if a client DOES access .bounds during this call, and to tell them "this probably won't do what you want, use .calculatedLayout.size instead".


if (_methodOverrides & ASDisplayNodeMethodOverrideFetchData) {
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
Expand Down
6 changes: 3 additions & 3 deletions Source/ASTableNode.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,10 @@ - (void)interfaceStateDidChange:(ASInterfaceState)newState fromState:(ASInterfac

- (void)didEnterPreloadState
{
// Intentionally allocate the view here so that super will trigger a layout pass on it which in turn will trigger the intial data load.
// We can get rid of this call later when ASDataController, ASRangeController and ASCollectionLayout can operate without the view.
[self view];
[super didEnterPreloadState];
// Intentionally allocate the view here and trigger a layout pass on it, which in turn will trigger the intial data load.
// We can get rid of this call later when ASDataController, ASRangeController and ASCollectionLayout can operate without the view.
[[self view] layoutIfNeeded];
}

#if ASRangeControllerLoggingEnabled
Expand Down