Skip to content

Commit

Permalink
[ASScrollNode] Fix small bugs and add unit tests (TextureGroup#637)
Browse files Browse the repository at this point in the history
* Add unit tests for ASScrollNode

* Make sure ASScrollNode's size is clamped against its size range

* Invalidate ASScrollNode's calculated layout if its scrollable directions changed

* Update comment

* Update CHANGELOG

* Address Adlai's comments
  • Loading branch information
nguyenhuy authored and bernieperez committed Apr 25, 2018
1 parent 79e5130 commit 92d52a4
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 3 deletions.
4 changes: 4 additions & 0 deletions AsyncDisplayKit.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,7 @@
E5775B041F16759F00CAC9BC /* ASCollectionLayoutCache.mm in Sources */ = {isa = PBXBuildFile; fileRef = E5775B031F16759F00CAC9BC /* ASCollectionLayoutCache.mm */; };
E5855DEF1EBB4D83003639AE /* ASCollectionLayoutDefines.m in Sources */ = {isa = PBXBuildFile; fileRef = E5855DED1EBB4D83003639AE /* ASCollectionLayoutDefines.m */; };
E5855DF01EBB4D83003639AE /* ASCollectionLayoutDefines.h in Headers */ = {isa = PBXBuildFile; fileRef = E5855DEE1EBB4D83003639AE /* ASCollectionLayoutDefines.h */; settings = {ATTRIBUTES = (Private, ); }; };
E586F96C1F9F9E2900ECE00E /* ASScrollNodeTests.m in Sources */ = {isa = PBXBuildFile; fileRef = E586F96B1F9F9E2900ECE00E /* ASScrollNodeTests.m */; };
E58E9E421E941D74004CFC59 /* ASCollectionFlowLayoutDelegate.h in Headers */ = {isa = PBXBuildFile; fileRef = E58E9E3D1E941D74004CFC59 /* ASCollectionFlowLayoutDelegate.h */; settings = {ATTRIBUTES = (Public, ); }; };
E58E9E431E941D74004CFC59 /* ASCollectionFlowLayoutDelegate.m in Sources */ = {isa = PBXBuildFile; fileRef = E58E9E3E1E941D74004CFC59 /* ASCollectionFlowLayoutDelegate.m */; };
E58E9E441E941D74004CFC59 /* ASCollectionLayoutContext.h in Headers */ = {isa = PBXBuildFile; fileRef = E58E9E3F1E941D74004CFC59 /* ASCollectionLayoutContext.h */; settings = {ATTRIBUTES = (Public, ); }; };
Expand Down Expand Up @@ -936,6 +937,7 @@
E5775B031F16759F00CAC9BC /* ASCollectionLayoutCache.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ASCollectionLayoutCache.mm; sourceTree = "<group>"; };
E5855DED1EBB4D83003639AE /* ASCollectionLayoutDefines.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASCollectionLayoutDefines.m; sourceTree = "<group>"; };
E5855DEE1EBB4D83003639AE /* ASCollectionLayoutDefines.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASCollectionLayoutDefines.h; sourceTree = "<group>"; };
E586F96B1F9F9E2900ECE00E /* ASScrollNodeTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ASScrollNodeTests.m; sourceTree = "<group>"; };
E58E9E3D1E941D74004CFC59 /* ASCollectionFlowLayoutDelegate.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASCollectionFlowLayoutDelegate.h; sourceTree = "<group>"; };
E58E9E3E1E941D74004CFC59 /* ASCollectionFlowLayoutDelegate.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASCollectionFlowLayoutDelegate.m; sourceTree = "<group>"; };
E58E9E3F1E941D74004CFC59 /* ASCollectionLayoutContext.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASCollectionLayoutContext.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1219,6 +1221,7 @@
E51B78BD1F01A0EE00E32604 /* ASLayoutFlatteningTests.m */,
052EE0651A159FEF002C6279 /* ASMultiplexImageNodeTests.m */,
058D0A32195D057000B7D73C /* ASMutableAttributedStringBuilderTests.m */,
E586F96B1F9F9E2900ECE00E /* ASScrollNodeTests.m */,
3C9C128419E616EF00E942A0 /* ASTableViewTests.mm */,
CC4981B21D1A02BE004E13CC /* ASTableViewThrashTests.m */,
058D0A33195D057000B7D73C /* ASTextKitCoreTextAdditionsTests.m */,
Expand Down Expand Up @@ -2177,6 +2180,7 @@
CC7FD9E11BB5F750005CCB2B /* ASPhotosFrameworkImageRequestTests.m in Sources */,
052EE0661A159FEF002C6279 /* ASMultiplexImageNodeTests.m in Sources */,
058D0A3C195D057000B7D73C /* ASMutableAttributedStringBuilderTests.m in Sources */,
E586F96C1F9F9E2900ECE00E /* ASScrollNodeTests.m in Sources */,
CC8B05D81D73979700F54286 /* ASTextNodePerformanceTests.m in Sources */,
CC583AD91EF9BDC600134156 /* ASDisplayNode+OCMock.m in Sources */,
697B315A1CFE4B410049936F /* ASEditableTextNodeTests.m in Sources */,
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
- [Collection/Table] Added direct support for mapping section indexes between data spaces. [Adlai Holler](https://github.com/Adlai-Holler) [#651](https://github.com/TextureGroup/Texture/pull/660)
- [ASCornerLayoutSpec] New layout spec class for declarative corner element layout. [#657](https://github.com/TextureGroup/Texture/pull/657) [huangkun](https://github.com/huang-kun)
- [Layout] Fix an issue that causes a pending layout to be applied multiple times. [Huy Nguyen](https://github.com/nguyenhuy) [#695](https://github.com/TextureGroup/Texture/pull/695)
- [ASScrollNode] Ensure the node respects the given size range while calculating its layout. [#637](https://github.com/TextureGroup/Texture/pull/637) [Huy Nguyen](https://github.com/nguyenhuy)
- [ASScrollNode] Invalidate the node's calculated layout if its scrollable directions changed. Also add unit tests for the class. [#637](https://github.com/TextureGroup/Texture/pull/637) [Huy Nguyen](https://github.com/nguyenhuy)

## 2.6
- [Xcode 9] Updated to require Xcode 9 (to fix warnings) [Garrett Moon](https://github.com/garrettmoon)
Expand Down
11 changes: 8 additions & 3 deletions Source/ASScrollNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,12 @@ - (ASLayout *)calculateLayoutThatFits:(ASSizeRange)constrainedSize
// To understand this code, imagine we're containing a horizontal stack set within a vertical table node.
// Our parentSize is fixed ~375pt width, but 0 - INF height. Our stack measures 1000pt width, 50pt height.
// In this case, we want our scrollNode.bounds to be 375pt wide, and 50pt high. ContentSize 1000pt, 50pt.
// We can achieve this behavior by: 1. Always set contentSize to layout.size. 2. Set bounds to parentSize,
// We can achieve this behavior by:
// 1. Always set contentSize to layout.size.
// 2. Set bounds to a size that is calculated by clamping parentSize against constrained size,
// unless one dimension is not defined, in which case adopt the contentSize for that dimension.
_contentCalculatedSizeFromLayout = layout.size;
CGSize selfSize = parentSize;
CGSize selfSize = ASSizeRangeClamp(constrainedSize, parentSize);
if (ASPointsValidForLayout(selfSize.width) == NO) {
selfSize.width = _contentCalculatedSizeFromLayout.width;
}
Expand Down Expand Up @@ -161,7 +163,10 @@ - (ASScrollDirection)scrollableDirections
- (void)setScrollableDirections:(ASScrollDirection)scrollableDirections
{
ASDN::MutexLocker l(__instanceLock__);
_scrollableDirections = scrollableDirections;
if (_scrollableDirections != scrollableDirections) {
_scrollableDirections = scrollableDirections;
[self setNeedsLayout];
}
}

@end
139 changes: 139 additions & 0 deletions Tests/ASScrollNodeTests.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
//
// ASScrollNodeTests.m
// Texture
//
// Copyright (c) 2017-present, Pinterest, Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//

#import <UIKit/UIKit.h>
#import <XCTest/XCTest.h>
#import <AsyncDisplayKit/AsyncDisplayKit.h>
#import "ASXCTExtensions.h"

@interface ASScrollNodeTests : XCTestCase

@property (nonatomic) ASScrollNode *scrollNode;
@property (nonatomic) ASDisplayNode *subnode;

@end

@implementation ASScrollNodeTests

- (void)setUp
{
ASDisplayNode *subnode = [[ASDisplayNode alloc] init];
self.subnode = subnode;

self.scrollNode = [[ASScrollNode alloc] init];
self.scrollNode.scrollableDirections = ASScrollDirectionVerticalDirections;
self.scrollNode.automaticallyManagesContentSize = YES;
self.scrollNode.automaticallyManagesSubnodes = YES;
self.scrollNode.layoutSpecBlock = ^ASLayoutSpec * _Nonnull(__kindof ASDisplayNode * _Nonnull node, ASSizeRange constrainedSize) {
return [[ASWrapperLayoutSpec alloc] initWithLayoutElement:subnode];
};
[self.scrollNode view];
}

- (void)testSubnodeLayoutCalculatedWithUnconstrainedMaxSizeInScrollableDirection
{
CGSize parentSize = CGSizeMake(100, 100);
ASSizeRange sizeRange = ASSizeRangeMake(parentSize);

[self.scrollNode layoutThatFits:sizeRange parentSize:parentSize];

ASSizeRange subnodeSizeRange = sizeRange;
subnodeSizeRange.max.height = CGFLOAT_MAX;
XCTAssertEqual(self.scrollNode.scrollableDirections, ASScrollDirectionVerticalDirections);
ASXCTAssertEqualSizeRanges(self.subnode.constrainedSizeForCalculatedLayout, subnodeSizeRange);

// Same test for horizontal scrollable directions
self.scrollNode.scrollableDirections = ASScrollDirectionHorizontalDirections;
[self.scrollNode layoutThatFits:sizeRange parentSize:parentSize];

subnodeSizeRange = sizeRange;
subnodeSizeRange.max.width = CGFLOAT_MAX;

ASXCTAssertEqualSizeRanges(self.subnode.constrainedSizeForCalculatedLayout, subnodeSizeRange);
}

- (void)testAutomaticallyManagesContentSizeUnderflow
{
CGSize subnodeSize = CGSizeMake(100, 100);
CGSize parentSize = CGSizeMake(100, 200);
ASSizeRange sizeRange = ASSizeRangeUnconstrained;

self.subnode.style.preferredSize = subnodeSize;

[self.scrollNode layoutThatFits:sizeRange parentSize:parentSize];
[self.scrollNode layout];

ASXCTAssertEqualSizes(self.scrollNode.calculatedSize, parentSize);
ASXCTAssertEqualSizes(self.scrollNode.view.contentSize, subnodeSize);
}

- (void)testAutomaticallyManagesContentSizeOverflow
{
CGSize subnodeSize = CGSizeMake(100, 500);
CGSize parentSize = CGSizeMake(100, 200);
ASSizeRange sizeRange = ASSizeRangeUnconstrained;

self.subnode.style.preferredSize = subnodeSize;

[self.scrollNode layoutThatFits:sizeRange parentSize:parentSize];
[self.scrollNode layout];

ASXCTAssertEqualSizes(self.scrollNode.calculatedSize, parentSize);
ASXCTAssertEqualSizes(self.scrollNode.view.contentSize, subnodeSize);
}

- (void)testAutomaticallyManagesContentSizeWithSizeRangeSmallerThanParentSize
{
CGSize subnodeSize = CGSizeMake(100, 100);
CGSize parentSize = CGSizeMake(100, 500);
ASSizeRange sizeRange = ASSizeRangeMake(CGSizeMake(100, 100), CGSizeMake(100, 200));

self.subnode.style.preferredSize = subnodeSize;

[self.scrollNode layoutThatFits:sizeRange parentSize:parentSize];
[self.scrollNode layout];

ASXCTAssertEqualSizes(self.scrollNode.calculatedSize, sizeRange.max);
ASXCTAssertEqualSizes(self.scrollNode.view.contentSize, subnodeSize);
}

- (void)testAutomaticallyManagesContentSizeWithSizeRangeBiggerThanParentSize
{
CGSize subnodeSize = CGSizeMake(100, 200);
CGSize parentSize = CGSizeMake(100, 100);
ASSizeRange sizeRange = ASSizeRangeMake(CGSizeMake(100, 150));

self.subnode.style.preferredSize = subnodeSize;

[self.scrollNode layoutThatFits:sizeRange parentSize:parentSize];
[self.scrollNode layout];

ASXCTAssertEqualSizes(self.scrollNode.calculatedSize, sizeRange.min);
ASXCTAssertEqualSizes(self.scrollNode.view.contentSize, subnodeSize);
}

- (void)testAutomaticallyManagesContentSizeWithInvalidCalculatedSizeForLayout
{
CGSize subnodeSize = CGSizeMake(100, 200);
CGSize parentSize = CGSizeMake(CGFLOAT_MAX, CGFLOAT_MAX);
ASSizeRange sizeRange = ASSizeRangeUnconstrained;

self.subnode.style.preferredSize = subnodeSize;

[self.scrollNode layoutThatFits:sizeRange parentSize:parentSize];
[self.scrollNode layout];

ASXCTAssertEqualSizes(self.scrollNode.calculatedSize, subnodeSize);
ASXCTAssertEqualSizes(self.scrollNode.view.contentSize, subnodeSize);
}

@end

0 comments on commit 92d52a4

Please sign in to comment.