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

Refactor table view updates #130

Closed
wants to merge 3 commits into from
Closed

Conversation

benasher44
Copy link
Contributor

@benasher44 benasher44 commented Aug 17, 2018

Changes in this pull request

This cleans up table view updates to no longer manually update cells and headers. All updates to the model now reload the table datasource. With diffing enabled, behavior is the same. This mainly improves behavior with diffing disabled: non-diffing updates can now tell if a full reloadData is necessary or if beginUpdates/endUpdates can be used fo the model update.

This change is covered by existing tests.

Checklist

  • All tests pass. Demo project builds and runs.
  • I added tests, an experiment, or detailed why my change isn't tested.
  • I added an entry to the CHANGELOG.md for any breaking changes, enhancements, or bug fixes.
  • I have reviewed the contributing guide

Sorry, something went wrong.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@benasher44 benasher44 added this to the 0.2.0 milestone Aug 17, 2018
@benasher44
Copy link
Contributor Author

This supersedes #128.

let visibleIndexPaths = self.tableView.indexPathsForVisibleRows ?? []

// Collect the index paths and views models to reload
let indexPathsAndViewModelsToReload = visibleIndexPaths.compactMap { indexPath in
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of the visible row/section business is now handled by just reloading the table.

@@ -226,7 +198,9 @@ extension TableViewDriver: UITableViewDataSource {
guard let tableViewModel = self.tableViewModel, let cellViewModel = tableViewModel[indexPath] else {
fatalError("Table View Model has an invalid configuration: \(String(describing: self.tableViewModel))")
}
return tableView.configuredCell(for: cellViewModel, at: indexPath)
let cell = tableView.configuredCell(for: cellViewModel, at: indexPath)
cell.accessibilityIdentifier = cellViewModel.accessibilityFormat.accessibilityIdentifierForIndexPath(indexPath)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This used to only be handled by refreshViews. By moving this here, we ensure this is always up-to-date.

@benasher44
Copy link
Contributor Author

Will wait to merge until I can test this against PG.

@benasher44
Copy link
Contributor Author

This solution is un-tenable, if you have UITextFields in your cells. Closing.

@benasher44 benasher44 closed this Aug 20, 2018
@benasher44 benasher44 deleted the basher/diffing-cleanup branch August 20, 2018 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant