-
-
Notifications
You must be signed in to change notification settings - Fork 12
Support item expanding and collapsing #146
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
base: main
Are you sure you want to change the base?
Changes from all commits
450c49a
e145eaf
eb1868b
238a869
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -38,6 +38,10 @@ public protocol CellViewModel: DiffableViewModel, ViewRegistrationProvider { | |||||||||
| /// This corresponds to the delegate method `collectionView(_:contextMenuConfigurationForItemAt:point:)`. | ||||||||||
| var contextMenuConfiguration: UIContextMenuConfiguration? { get } | ||||||||||
|
|
||||||||||
| /// Returns an array of children for the cell. | ||||||||||
| /// These children correspond to the items that have been appended to this item as part of a `NSDiffableDataSourceSectionSnapshot`. | ||||||||||
|
Comment on lines
+41
to
+42
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| var children: [AnyCellViewModel] { get } | ||||||||||
|
|
||||||||||
| /// Configures the provided cell for display in the collection. | ||||||||||
| /// - Parameter cell: The cell to configure. | ||||||||||
| @MainActor | ||||||||||
|
|
@@ -90,6 +94,9 @@ extension CellViewModel { | |||||||||
| /// Default implementation. Returns `true`. | ||||||||||
| public var shouldHighlight: Bool { true } | ||||||||||
|
|
||||||||||
| /// Default implementation. Returns `[]`. | ||||||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| public var children: [AnyCellViewModel] { [] } | ||||||||||
|
|
||||||||||
| /// Default implementation. Returns `nil`. | ||||||||||
| public var contextMenuConfiguration: UIContextMenuConfiguration? { nil } | ||||||||||
|
|
||||||||||
|
|
@@ -198,6 +205,9 @@ public struct AnyCellViewModel: CellViewModel { | |||||||||
| /// :nodoc: | ||||||||||
| public var shouldHighlight: Bool { self._shouldHighlight } | ||||||||||
|
|
||||||||||
| /// :nodoc: | ||||||||||
| public var children: [AnyCellViewModel] { self._children } | ||||||||||
|
|
||||||||||
| /// :nodoc: | ||||||||||
| public var contextMenuConfiguration: UIContextMenuConfiguration? { self._contextMenuConfiguration } | ||||||||||
|
|
||||||||||
|
|
@@ -251,6 +261,7 @@ public struct AnyCellViewModel: CellViewModel { | |||||||||
| private let _shouldDeselect: Bool | ||||||||||
| private let _shouldHighlight: Bool | ||||||||||
| private let _contextMenuConfiguration: UIContextMenuConfiguration? | ||||||||||
| private let _children: [AnyCellViewModel] | ||||||||||
| private let _configure: @Sendable @MainActor (CellType) -> Void | ||||||||||
| private let _didSelect: @Sendable @MainActor (CellEventCoordinator?) -> Void | ||||||||||
| private let _didDeselect: @Sendable @MainActor (CellEventCoordinator?) -> Void | ||||||||||
|
|
@@ -276,6 +287,7 @@ public struct AnyCellViewModel: CellViewModel { | |||||||||
| self._shouldSelect = viewModel.shouldSelect | ||||||||||
| self._shouldDeselect = viewModel.shouldDeselect | ||||||||||
| self._shouldHighlight = viewModel.shouldHighlight | ||||||||||
| self._children = viewModel.children | ||||||||||
| self._contextMenuConfiguration = viewModel.contextMenuConfiguration | ||||||||||
| self._configure = { | ||||||||||
| viewModel._configureGeneric(cell: $0) | ||||||||||
|
|
||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to replace Note: maybe a place we can optimize performance? You have to call allCellsByIdentifier for both source and dest in _findItemsToReconfigure which size-wise can be >>> the # of cells you actually care about--visible items--which is probably around ~10 on mobile depending on ur design |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,16 +65,19 @@ public struct CollectionViewModel: DiffableViewModel { | |
|
|
||
| // MARK: Accessing Cells | ||
|
|
||
| /// Returns the cell for the specified `id`. | ||
| /// Returns the cell for the specified `id` by traversing each section and all of the children of each cell. | ||
| /// | ||
| /// - Parameter id: The identifier for the cell. | ||
| /// - Returns: The cell, if it exists. | ||
| public func cellViewModel(for id: UniqueIdentifier) -> AnyCellViewModel? { | ||
| for section in self.sections { | ||
| for cell in section.cells where cell.id == id { | ||
| return cell | ||
| for cell in section.cells { | ||
| if let foundViewModel = cellViewModel(in: cell, with: id) { | ||
| return foundViewModel | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
|
|
@@ -84,14 +87,42 @@ public struct CollectionViewModel: DiffableViewModel { | |
| /// - Returns: The cell at `indexPath`. | ||
| /// | ||
| /// - Precondition: The specified `indexPath` must be valid. | ||
jessesquires marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| public func cellViewModel(at indexPath: IndexPath) -> AnyCellViewModel { | ||
| public func cellViewModel(at indexPath: IndexPath, in collectionView: UICollectionView) -> AnyCellViewModel { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All of the changes here seem wrong. I'm very weary and skeptical of referencing the collection view, datasource, and snapshot here. This breaks a lot of encapsulation. My first thought is that this method shouldn't change. But that raises the question: what is the index path of a nested child cell? How does that work?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. per the video here, an item's index path changes depending on if there are expanded items preceding it. I think this was part of why I went with the approach I did, because we need to know what's visible at the moment. Still need to look around some more to refresh myself.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another thing we can do is get the section from the collection view model, then ask the data source for the snapshot of that section, then grab the something like: let section = self.viewModel.sectionViewModel(at: sectionIndex)
let snapshot = self._dataSource.snapshot(for: section.id)
let visibleItems = Array(snapshot.visibleItems)
// ...
let identifier = visibleItems[indexPath.item]
if let cell = self.cellViewModel(for: identifier) {
return cell
}
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This behavior seems to be mostly undocumented, as mentioned here in the first section: https://swiftsenpai.com/development/undocumented-section-snapshot-facts/
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ahh thanks. Super helpful.
I am still extremely apprehensive about having The entire idea behind I have not experimented with any code yet, so the open questions on the feasibility of this approach are: Do we have to keep How exactly would this work? The How does all of this work with UI interactions?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I'm understanding right, I think the DiffableDataSource would need a way to access the CollectionViewModel upon expand/collapse events, so it can then get the right cellViewModel and update its property. Is that what you had in mind, some sort of way for the data source to communicate up (or otherwise) that those events have happened? |
||
| precondition(indexPath.section < self.count) | ||
| let section = self.sectionViewModel(at: indexPath.section) | ||
|
|
||
| let cells = section.cells | ||
| precondition(indexPath.item < cells.count) | ||
|
|
||
| return cells[indexPath.item] | ||
| guard let diffableDataSource = collectionView.dataSource as? DiffableDataSource | ||
| else { | ||
| return cells[indexPath.item] | ||
| } | ||
|
|
||
| let snapshot = diffableDataSource.snapshot(for: section.id) | ||
| let id = snapshot.visibleItems[indexPath.item] | ||
|
|
||
| guard let cellViewModel = cellViewModel(for: id) | ||
| else { | ||
| return cells[indexPath.item] | ||
| } | ||
|
|
||
| return cellViewModel | ||
| } | ||
jessesquires marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| /// Recursively traverse the children array of each child to locate a matching cell view model | ||
| private func cellViewModel(in viewModel: AnyCellViewModel, with id: UniqueIdentifier) -> AnyCellViewModel? { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should push some of this functionality to
|
||
| if viewModel.id == id { | ||
| return viewModel | ||
| } | ||
|
|
||
| for child in viewModel.children { | ||
| if let foundChildViewModel = cellViewModel(in: child, with: id) { | ||
| return foundChildViewModel | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // MARK: Accessing Supplementary Views | ||
|
|
@@ -168,12 +199,12 @@ public struct CollectionViewModel: DiffableViewModel { | |
| return self.sectionViewModel(at: index) | ||
| } | ||
|
|
||
| func _safeCellViewModel(at indexPath: IndexPath) -> AnyCellViewModel? { | ||
| func _safeCellViewModel(at indexPath: IndexPath, in collectionView: UICollectionView) -> AnyCellViewModel? { | ||
| guard let section = self._safeSectionViewModel(at: indexPath.section), | ||
| indexPath.item < section.cells.count else { | ||
| return nil | ||
| } | ||
jessesquires marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return self.cellViewModel(at: indexPath) | ||
| return self.cellViewModel(at: indexPath, in: collectionView) | ||
| } | ||
|
|
||
| func _safeSupplementaryViewModel(ofKind kind: String, at indexPath: IndexPath) -> AnySupplementaryViewModel? { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's revert all these changes to the Example app.
I want to leave the existing tabs and models as-is and have a new example that's fully independent of everything else.
What I'd prefer to do is:
This gives us multiple opportunities for various layers of nesting.
Alternatively, we could do something similar with SF Symbols.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're up for it, it would be nice to get the basic ground work for this done in a separate PR -- just getting the basic scaffolding setup.