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

Internal improvements to both iOS and macOS #351

Merged
merged 45 commits into from
Nov 5, 2016

Conversation

zenangst
Copy link
Owner

@zenangst zenangst commented Nov 2, 2016

This PR improves the macOS implementation of Spots by adding the ability to reload with components.

  • It also refactors both implementations of SpotScrollView to properly cleanup observers on subviews.
  • Adds a missing completion closure when prepending items on macOS
  • Implements sanitize on methods that could cause an inconsistency with indexes.
  • Improves tests on macOS to test the controller implementation
  • Adds beforeUpdate and afterUpdate on Spotable protocol.
  • Adds fallback kind identifier for all Spotable objects on macOS.

@mention-bot
Copy link

@zenangst, thanks for your PR! By analyzing the history of the files in this pull request, we identified @onmyway133 and @vadymmarkov to be potential reviewers.

@zenangst zenangst changed the title [WIP] Internal improvements to both iOS and macOS Internal improvements to both iOS and macOS Nov 2, 2016
@zenangst
Copy link
Owner Author

zenangst commented Nov 2, 2016

@zenangst
Copy link
Owner Author

zenangst commented Nov 2, 2016

Should improve test coverage for macOS #279

Copy link
Collaborator

@vadymmarkov vadymmarkov left a comment

Choose a reason for hiding this comment

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

Looks great!

/// - parameter completion: A completion closure that runs when your updates are done.
public func reloadIfNeeded(_ changes: ItemChanges, withAnimation animation: Animation = .automatic, updateDataSource: () -> Void, completion: Completion) {
collectionView.process((insertions: changes.insertions, reloads: changes.reloads, deletions: changes.deletions), updateDataSource: updateDataSource) {
if changes.updates.isEmpty {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@zenangst Should we use weak self here?


if item.size.width == 0.0 {
item.size.width = view.bounds.width
if item.size.height == 0.0 { item.size.height = itemView.preferredViewSize.height }
Copy link
Collaborator

Choose a reason for hiding this comment

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

@zenangst I prefer start on the new line, just because it's easier to debug.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I did this to fix the linting which... was a mistake 😎
I refactored it into it's own method now, without one-liners.

@codecov-io
Copy link

codecov-io commented Nov 2, 2016

Current coverage is 69.77% (diff: 78.31%)

Merging #351 into master will decrease coverage by 0.21%

@@             master       #351   diff @@
==========================================
  Files            57         58     +1   
  Lines          4236       4348   +112   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           2965       3034    +69   
- Misses         1271       1314    +43   
  Partials          0          0          

Powered by Codecov. Last update e447697...2299359

@vadymmarkov vadymmarkov merged commit 1068c04 into master Nov 5, 2016
@vadymmarkov vadymmarkov deleted the improve/macos-implementation branch November 5, 2016 11:28
Sign up for free to 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.

None yet

4 participants