Stability, Layout, and Logging Enhancements; Added Support for Legacy Edge Insets Functionality #202
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
… Edge Insets Functionality Squashed Commit of the following: commit 89ef650 Author: Will Spurgeon <[email protected]> Date: Thu Nov 9 10:16:10 2017 -0500 Added additional verbose logging to refresh/reloading of data. commit cee70e8 Author: Will Spurgeon <[email protected]> Date: Thu Nov 2 15:05:39 2017 -0400 Clear prefetch index paths upon invalidation. Written by nlobue commit c760425 Author: Will Spurgeon <[email protected]> Date: Mon Oct 30 16:50:56 2017 -0400 Fix for a crash that can occur if the first index is greater the number of items. commit a838e8d Author: Will Spurgeon <[email protected]> Date: Mon Oct 23 14:15:58 2017 -0400 Updated height provider code to be a closure (wayfair-archive#191). Fix by vlozko. commit 5e21936 Author: Will Spurgeon <[email protected]> Date: Wed Oct 18 14:06:09 2017 -0400 Fix for crash commit 48846ee Author: Will Spurgeon <[email protected]> Date: Fri Oct 13 10:04:59 2017 -0400 Attempt to fix a crash commit e6d3c6d Author: Will Spurgeon <[email protected]> Date: Thu Oct 5 17:28:58 2017 -0400 Added a flag that determines which version of the inset updating code is run.
Codecov Report
@@ Coverage Diff @@
## master #202 +/- ##
=========================================
Coverage ? 92.82%
=========================================
Files ? 40
Lines ? 4513
Branches ? 376
=========================================
Hits ? 4189
Misses ? 323
Partials ? 1
Continue to review full report at Codecov.
|
jay18001
suggested changes
Nov 16, 2017
| super.reloadData() | ||
| } | ||
|
|
||
|
|
| if !didUpdateEdgeInsets { | ||
| guard let brickAttributes = layoutAttributes as? BrickLayoutAttributes, brickAttributes.isEstimateSize else { | ||
| return layoutAttributes | ||
| if needsLegacyEdgeInsetFunctionality { |
There was a problem hiding this comment.
Is this if statement really necessary
There was a problem hiding this comment.
Yes, this is also a key part of the legacy edge insets fix.
| private var didUpdateEdgeInsets: Bool = false | ||
| @objc open dynamic var edgeInsets: UIEdgeInsets = UIEdgeInsets.zero { | ||
| didSet { | ||
| if edgeInsets == oldValue { | ||
|
|
||
| if edgeInsets == oldValue && needsLegacyEdgeInsetFunctionality { |
There was a problem hiding this comment.
Is needsLegacyEdgeInsetFunctionality really necessary here
There was a problem hiding this comment.
Yes, that's definitely required for the fix/workaround for the edge insets issue. This legacy edge insets support will be removed once the root cause of the issue is identified (there is a ticket for that work).
|
I pushed two commits:
|
jay18001
approved these changes
Nov 20, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Includes a fix (and unit tests) for the crashes that occur due to changing repeat counts. @rubencagnie has looked at this change and agreed it looks good.
Add the flag needsLegacyEdgeInsetFunctionality to BrickCell, which defaults to false, but can be set to true if the legacy handling of edge insets is required.
Remove some forced unwraps in BrickFlowLayout to avoid crashes.
Fix a crash that could occur due to calculating layouts of deleted sections in BrickFlowLayout.
Refactor Height provider to be a closure instead of protocol.
Fix a crash due to unchecked bounds of a range creation in BrickLayoutSection
Clear prefetch index paths upon invalidation.
Add additional verbose logging to refresh/reloading of data.
Squashed Commit of the following:
commit 89ef650
Author: Will Spurgeon [email protected]
Date: Thu Nov 9 10:16:10 2017 -0500
commit cee70e8
Author: Will Spurgeon [email protected]
Date: Thu Nov 2 15:05:39 2017 -0400
commit c760425
Author: Will Spurgeon [email protected]
Date: Mon Oct 30 16:50:56 2017 -0400
commit a838e8d
Author: Will Spurgeon [email protected]
Date: Mon Oct 23 14:15:58 2017 -0400
commit 5e21936
Author: Will Spurgeon [email protected]
Date: Wed Oct 18 14:06:09 2017 -0400
commit 48846ee
Author: Will Spurgeon [email protected]
Date: Fri Oct 13 10:04:59 2017 -0400
commit e6d3c6d
Author: Will Spurgeon [email protected]
Date: Thu Oct 5 17:28:58 2017 -0400