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.

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

Fixed the invalidating heights animation #164

Merged
merged 1 commit into from
Jul 28, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
buildConfiguration = "Debug"
selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
language = ""
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed?

Copy link
Contributor Author

@jay18001 jay18001 Jul 25, 2017

Choose a reason for hiding this comment

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

I think this is an artifact from Xcode 9

shouldUseLaunchSchemeArgsEnv = "YES"
codeCoverageEnabled = "YES">
<Testables>
Expand Down Expand Up @@ -56,6 +57,7 @@
buildConfiguration = "Debug"
selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
language = ""
launchStyle = "0"
useCustomWorkingDirectory = "NO"
ignoresPersistentStateOnLaunch = "NO"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,14 @@ class InvalidateHeightViewController: BrickViewController, HasTitle {

self.view.backgroundColor = .brickBackground

self.registerBrickClass(LabelBrick.self)

brick = LabelBrick(BrickIdentifiers.repeatLabel, backgroundColor: .brickGray1, dataSource: LabelBrickCellModel(text: "BRICK") { cell in
brick = LabelBrick(BrickIdentifiers.titleLabel, backgroundColor: .brickGray1, dataSource: LabelBrickCellModel(text: "BRICK") { cell in
cell.configure()
})

let section = BrickSection(bricks: [
brick,
LabelBrick(BrickIdentifiers.repeatLabel, backgroundColor: .brickGray2, dataSource: LabelBrickCellModel(text: "BRICK", configureCellBlock: LabelBrickCell.configure))
LabelBrick(BrickIdentifiers.repeatLabel, backgroundColor: .brickGray2, dataSource: LabelBrickCellModel(text: "BRICK", configureCellBlock: LabelBrickCell.configure)),
brick,
], inset: 10, edgeInsets: UIEdgeInsets(top: 20, left: 20, bottom: 20, right: 20))

self.setSection(section)
Expand All @@ -55,12 +54,21 @@ class InvalidateHeightViewController: BrickViewController, HasTitle {
}

@objc func toggleHeights() {
let newHeight: BrickDimension
switch brick.height {
case .fixed(_): brick.height = .auto(estimate: .fixed(size: 100))
default: brick.height = .fixed(size: 200)
case .fixed(_):
newHeight = .auto(estimate: .fixed(size: 100))
default:
newHeight = .fixed(size: 200)
}

self.brickCollectionView.invalidateBricks(false)

let size = BrickSize(width: .ratio(ratio: 1), height: newHeight)
brick.size = size

UIView.animate(withDuration: 2) {
self.brickCollectionView.invalidateBricks(false)
}

self.updateNavigationItem()
}

Expand Down
12 changes: 6 additions & 6 deletions Source/Cells/BrickCell.swift
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ open class BaseBrickCell: UICollectionViewCell {
open lazy var topSeparatorLine: UIView = {
return UIView()
}()

open override func apply(_ layoutAttributes: UICollectionViewLayoutAttributes) {
super.apply(layoutAttributes)

Expand All @@ -106,6 +106,7 @@ open class BaseBrickCell: UICollectionViewCell {
// UICollectionView zIndex management 'fixes' the issue
// http://stackoverflow.com/questions/12659301/uicollectionview-setlayoutanimated-not-preserving-zindex
self.layer.zPosition = CGFloat(layoutAttributes.zIndex)
self.layoutIfNeeded()
}

open override func layoutSubviews() {
Expand Down Expand Up @@ -200,7 +201,7 @@ open class BrickCell: BaseBrickCell {
self.tapGesture = gesture
addGestureRecognizer(gesture)
}

reloadContent()
}

Expand All @@ -213,17 +214,17 @@ open class BrickCell: BaseBrickCell {
updateContent()
self._brick.overrideContentSource?.overrideContent(for: self)
}

@objc func didTapCell() {
_brick.brickCellTapDelegate?.didTapBrickCell(self)
}

open override func preferredLayoutAttributesFitting(_ layoutAttributes: UICollectionViewLayoutAttributes) -> UICollectionViewLayoutAttributes {
guard self._brick.height.isEstimate(withValue: nil) else {
return layoutAttributes
}

let preferred = layoutAttributes.copy() as! UICollectionViewLayoutAttributes
let preferred = layoutAttributes
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we revert this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't seem to have any side effects by the looks of it


// We're inverting the frame because the given frame is already transformed
var invertedFrame = layoutAttributes.frame.applying(layoutAttributes.transform.inverted())
Expand All @@ -246,4 +247,3 @@ open class BrickCell: BaseBrickCell {
}

}

10 changes: 2 additions & 8 deletions Source/Layout/BrickFlowLayout.swift
Original file line number Diff line number Diff line change
Expand Up @@ -443,14 +443,8 @@ extension BrickFlowLayout: BrickLayoutSectionDataSource {
var size: CGSize = .zero
switch type {
case .brick:
// Check if the attributes already had a height. If so, use that height
if attributes.frame.height != 0 && _dataSource.brickLayout(self, isEstimatedHeightFor: indexPath) {
let height = attributes.frame.size.height
size = CGSize(width: width, height: height)
} else {
let height = _dataSource.brickLayout(self, estimatedHeightForItemAt: indexPath, containedIn: width)
size = CGSize(width: width, height: height)
}
let height = _dataSource.brickLayout(self, estimatedHeightForItemAt: indexPath, containedIn: width)
size = CGSize(width: width, height: height)
case .section(let section):
let height = _dataSource.brickLayout(self, estimatedHeightForItemAt: indexPath, containedIn: width)
if height == 0 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ extension BrickCollectionView: BrickLayoutDataSource {
return 0
}

// If the brick height is an estimate, check if the cell is on screen
// If so, calculate the height directly, so the estimation is correct from the get-go
if brick.size.height.isEstimate(withValue: nil), let brickCell = self.cellForItem(at: indexPath) as? BrickCell {
return brickCell.heightForBrickView(withWidth: width)
}

let heightDimension = brick.size.height
return heightDimension.value(for: width, startingAt: 0)
}
Expand Down
3 changes: 1 addition & 2 deletions Source/ViewControllers/BrickCollectionView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -231,10 +231,9 @@ open class BrickCollectionView: UICollectionView {
if reloadSections {
self.reloadSections(IndexSet(integersIn: 0..<self.numberOfSections))
}
self.collectionViewLayout.invalidateLayout(with: BrickLayoutInvalidationContext(type: .invalidate))
}, completion: completion)
}

/// Invalidate the visibility
///
/// - parameter completion: A completion handler block to execute when all of the operations are finished. This block takes a single Boolean parameter that contains the value true if all of the related animations completed successfully or false if they were interrupted. This parameter may be nil.
Expand Down
9 changes: 4 additions & 5 deletions Tests/Bricks/LabelBrickTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -201,16 +201,15 @@ class LabelBrickTests: XCTestCase {

func testLabelWithButton() {
brickCollectionView.registerNib(LabelBrickNibs.Button, forBrickWithIdentifier: LabelBrickIdentifier)


let cell = setupLabelBrick("Hello World", configureCellBlock: { cell in
XCTAssertNotNil(cell.button)
cell.button?.setTitle("BUTTON", for: UIControlState())
cell.button?.setTitle("Button", for: .normal)
cell.button?.titleLabel?.font = UIFont(name: "Avenir-Medium", size: 14)
})

XCTAssertNotNil(cell?.button)
XCTAssertEqual(cell?.button?.titleLabel?.text, "BUTTON")
XCTAssertEqual(cell?.button?.titleLabel?.text, "Button")

let buttonWidth = cell?.button?.frame.size.width ?? 61

Expand All @@ -232,7 +231,7 @@ class LabelBrickTests: XCTestCase {

let cell = setupLabelBrick("Hello World", configureCellBlock: { cell in
XCTAssertNotNil(cell.button)
cell.button?.setTitle("BUTTON", for: UIControlState())
cell.button?.setTitle("Button", for: UIControlState())
cell.edgeInsets = UIEdgeInsets(top: 5, left: 5, bottom: 5, right: 5)
cell.button?.titleLabel?.font = UIFont(name: "Avenir-Medium", size: 14)
})
Expand All @@ -241,7 +240,7 @@ class LabelBrickTests: XCTestCase {

let buttonWidth = cell?.button?.frame.size.width ?? 61

XCTAssertEqual(cell?.button?.titleLabel?.text, "BUTTON")
XCTAssertEqual(cell?.button?.titleLabel?.text, "Button")

#if os(iOS)
let cellSize = CGSize(width: 320, height: 42)
Expand Down
61 changes: 61 additions & 0 deletions Tests/ViewControllers/BrickCollectionViewTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,67 @@ class BrickCollectionViewTests: XCTestCase {
XCTAssertEqual(cell?.frame.height, 128)
}

func testResizeBrickBigger() {
let brick = DummyBrick("Brick", width: .ratio(ratio: 1/10))
let section = BrickSection(bricks: [
brick
])
brickView.setupSectionAndLayout(section)

var cell: DummyBrickCell?
cell = brickView.cellForItem(at: IndexPath(item: 0, section: 1)) as? DummyBrickCell
XCTAssertEqual(cell?.frame.width, 32)
XCTAssertEqual(cell?.frame.height, 64)

let size = BrickSize(width: .ratio(ratio: 1/5), height: .fixed(size: 200))
brick.size = size
let expectation = self.expectation(description: "Invalidate Bricks")


brickView.invalidateBricks(true) { (completed) in
expectation.fulfill()
}

waitForExpectations(timeout: 5, handler: nil)

brickView.layoutSubviews()

cell = brickView.cellForItem(at: IndexPath(item: 0, section: 1)) as? DummyBrickCell
cell?.layoutIfNeeded()
XCTAssertEqual(cell?.frame.width, 64)
XCTAssertEqual(cell?.frame.height, 200)
}

func testResizeBrickSmaller() {
let brick = DummyBrick("Brick", width: .ratio(ratio: 1/5))
let section = BrickSection(bricks: [
brick
])
brickView.setupSectionAndLayout(section)

var cell: DummyBrickCell?
cell = brickView.cellForItem(at: IndexPath(item: 0, section: 1)) as? DummyBrickCell
XCTAssertEqual(cell?.frame.width, 64)
XCTAssertEqual(cell?.frame.height, 128)

let size = BrickSize(width: .ratio(ratio: 1/10), height: .fixed(size: 20))
brick.size = size
let expectation = self.expectation(description: "Invalidate Bricks")

brickView.invalidateBricks(true) { (completed) in
expectation.fulfill()
}

waitForExpectations(timeout: 5, handler: nil)

brickView.layoutSubviews()

cell = brickView.cellForItem(at: IndexPath(item: 0, section: 1)) as? DummyBrickCell
cell?.layoutIfNeeded()
XCTAssertEqual(cell?.frame.width, 32)
XCTAssertEqual(cell?.frame.height, 20)
}

func testReloadBricksWithBehaviors() {
let offsetDataSource = FixedMultipleOffsetLayoutBehaviorDataSource(originOffsets: ["DummyBrick": CGSize(width: 10, height: 10)], sizeOffsets: nil)
brickView.layout.behaviors.insert(OffsetLayoutBehavior(dataSource: offsetDataSource))
Expand Down