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

Pull in OpenNet changes #27

Draft
wants to merge 221 commits into
base: 2.0-beta
Choose a base branch
from
Draft

Conversation

JosephDuffy
Copy link
Member

@JosephDuffy JosephDuffy commented Jan 18, 2021

This PR is being used to diff between the OpenNet fork.

Hopefully other PRs will be merged before this (e.g. #21), which should lead to this PR having no changes and purely to ensure some of the history is not lost.

JosephDuffy and others added 30 commits December 22, 2020 13:59
Still a little hacky, more like a POC, but is overall much more flexible
…atSection

# Conflicts:
#	Sources/ComposedUI/CollectionView/CollectionCoordinator.swift
# Conflicts:
#	Sources/ComposedUI/CollectionView/CollectionCoordinator.swift
The layout's `sectionInset` property is applied on a per-section basis, but the collection view's `contentInset` was not being honoured.

It was also calculated using `insetBy(dx:dy:)`, which will modify the width by `-dx * 2`, which would double the expected insets.

This is technically a breaking change for those relying on this behaviour, but it's a big enough bug that it should probably still be released without a major version bump. (plus not many are using the porject anyway 😬)
Copy link
Collaborator

@shaps80 shaps80 left a comment

Choose a reason for hiding this comment

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

LGTM

This API should remain private; only the results of its use should be tested
This fixes a crash when reading a header/footer during an update the previously invalidated the header/footer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants