Skip to content

Nested visible frame inset fix - #119

Open
casperzandbergenyaacomm wants to merge 7 commits into
SoySauceLab:masterfrom
casperzandbergenyaacomm:nested-visibleFrameInset-fix
Open

Nested visible frame inset fix#119
casperzandbergenyaacomm wants to merge 7 commits into
SoySauceLab:masterfrom
casperzandbergenyaacomm:nested-visibleFrameInset-fix

Conversation

@casperzandbergenyaacomm

Copy link
Copy Markdown
Collaborator

From commit messages:

After this fix there is one more issue:

When a horizontal layout section with visible frame inset is inside a section provider with vertical layout (or the other way around) the visible frame inset is actually twice the set value. This is due to the optimised index search not checking vertical position. Right now I added a guard to not show the section when visible frame is empty instead of when there is no frames inside the visibleFrame. Not sure if this is better optimised or not.

We can't really fix this since we can't actually check if the insets are in the right direction because the VisibleFrameInsetLayout might get transposed after checking.

Casper Zandbergenand others added 7 commits November 28, 2018 11:04
…th the visible indexes and the frame they appear in. This frame can be different from the input frame if the layout changes it (like VisibleFrameInsetLayout does).
…d by taking the order of indexes returned by the layout into account. This does add two filters when `visible(for:)` is called on a SectionProvider.
…ns a rect for the section to add its visible frame inset to.
…t (or the other way around). After this fix there is one more issue:
When a horizontal layout section with visible frame inset is inside a section provider with vertical layout (or the other way around) the visible frame inset is actually twice the set value. This is due to the optimised index search not checking vertical position. Right now I added a guard to not show the section when visible frame is empty instead of when there is no frames inside the visibleFrame. Not sure if this is better optimised or not.
A way to fix it would be to check if rootLayout is a vertical or horizontal layout and adjusting the inset inside the VisibleFrameInset init, but this is not the root of the problem so I'm not sure how to do it instead.
…e insets are in the right direction because the VisibleFrameInsetLayout might get transposed later.
@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #119 into master will increase coverage by 0.38%.
The diff coverage is 73.43%.

Impacted file tree graph

@@ Coverage Diff @@## master #119 +/- ##
==========================================
+ Coverage 73.46% 73.84% +0.38% 
==========================================
Files 41 41 Lines 1123 1147 +24 ==========================================
+ Hits 825 847 +22 - Misses 298 300 +2
Impacted FilesCoverage Δ
Sources/Protocol/Provider.swift100% <ø> (ø)⬆️
Sources/Layout/WrapperLayout.swift20% <0%> (ø)⬆️
Sources/Layout/VisibleFrameInsetLayout.swift0% <0%> (ø)⬆️
Sources/Layout/Layout.swift20% <0%> (ø)⬆️
Sources/Provider/EmptyCollectionProvider.swift0% <0%> (ø)⬆️
Sources/Layout/InsetLayout.swift0% <0%> (ø)⬆️
Sources/Provider/FlattenedProvider.swift93.47% <100%> (+1.07%)⬆️
Sources/CollectionView.swift96.93% <100%> (ø)⬆️
Sources/Layout/TransposeLayout.swift89.47% <100%> (+0.58%)⬆️
Sources/Protocol/LayoutableProvider.swift100% <100%> (ø)⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b78e4b3...910f732. Read the comment docs.

@casperzandbergenyaacomm

Copy link
Copy Markdown
CollaboratorAuthor

Ah there is an unused CGRect extension in there by accident

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@casperzandbergenyaacomm@codecov-io