Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the legacy UIKit Notes list to use UITableViewDiffableDataSource backed by an NSFetchedResultsController, replacing the prior custom FRC manager approach.
Changes:
- Replaced the custom
NotesFRCManagerwrapper with an in-controllerNSFetchedResultsController+ diffable data source. - Removed
NotesFRCManager.swiftfrom the target and updated background/foreground delegate wiring inAppDelegate. - Updated the Xcode project file to drop the deleted source file and (additionally) introduced a local folder reference to
../swiftnextcloudui.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| iOCNotes.xcodeproj/project.pbxproj | Removes NotesFRCManager.swift from the project; adds a local folder reference to ../swiftnextcloudui (problematic). |
| Source/Screens/Notes/NotesTableViewController.swift | Implements diffable data source + NSFetchedResultsController updates and removes reliance on the deleted manager. |
| Source/Screens/Notes/NotesFRCManager.swift | Deletes the old custom FRC management layer. |
| Source/AppDelegate.swift | Switches background/foreground handling to the new FRC enable/disable methods on NotesTableViewController. |
| F389ABFE2C9C40210056C059 /* Custom.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = Custom.xcassets; sourceTree = "<group>"; }; | ||
| F3E3C8D82C6B7C2200A80504 /* NCBrand.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NCBrand.swift; sourceTree = "<group>"; }; | ||
| F3E3C8DB2C6B7DA600A80504 /* UIColor+Extension.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "UIColor+Extension.swift"; sourceTree = "<group>"; }; | ||
| F3E3D3052F97C514007FC9F0 /* swiftnextcloudui */ = {isa = PBXFileReference; lastKnownFileType = wrapper; name = swiftnextcloudui; path = ../swiftnextcloudui; sourceTree = SOURCE_ROOT; }; |
There was a problem hiding this comment.
This adds a folder reference to ../swiftnextcloudui (outside the repo). That path won’t exist for other developers/CI, leaving a broken reference in the project and potentially causing build or integration issues. If the intent is to use the Swift Package dependency, remove this external file reference and rely on the existing XCRemoteSwiftPackageReference "swiftnextcloudui" instead (or vendor it into the repo/submodule if it must be local).
| F3E3D3052F97C514007FC9F0 /* swiftnextcloudui */ = {isa = PBXFileReference; lastKnownFileType = wrapper; name = swiftnextcloudui; path = ../swiftnextcloudui; sourceTree = SOURCE_ROOT; }; |
| F30037412E6ED2DC00A49C0D /* IOCNotesUnitTests */, | ||
| D0CFE5351888A7BD00165839 /* Frameworks */, | ||
| D0CFE5341888A7BD00165839 /* Products */, | ||
| F3E3D3052F97C514007FC9F0 /* swiftnextcloudui */, |
There was a problem hiding this comment.
The root PBXGroup now includes the swiftnextcloudui folder reference pointing outside the repository (../swiftnextcloudui). This should be removed to avoid committing a developer-machine-specific dependency path into the shared project.
| F3E3D3052F97C514007FC9F0 /* swiftnextcloudui */, |
| } | ||
|
|
||
| do { | ||
| try fetchedResultsController.performFetch() |
There was a problem hiding this comment.
configureFetchedResultsController(performFetch:) performs a fetch and immediately applies a snapshot, but it doesn’t update disclosureSections first. If new Core Data sections were added/removed while the delegate was disabled (e.g. during background sync), those titles won’t be present in disclosureSections, and tapping a newly-added section header won’t toggle collapse/expand because toggleSection can’t find an entry. Consider calling updateSectionExpandedInfo() after performFetch() (and before applySnapshot) when performFetch is true, so the persisted collapsed-state list stays in sync with the fetched sections.
| try fetchedResultsController.performFetch() | |
| try fetchedResultsController.performFetch() | |
| updateSectionExpandedInfo() |
| dataSource = UITableViewDiffableDataSource<String, NSManagedObjectID>(tableView: tableView) { [weak self] tableView, indexPath, _ in | ||
| guard let self, | ||
| let note = self.note(at: indexPath), | ||
| let cell = tableView.dequeueReusableCell(withIdentifier: "NoteCell", for: indexPath) as? NoteTableViewCell else { |
There was a problem hiding this comment.
In UITableViewDiffableDataSource’s cell provider closure, the item identifier is ignored (_) and the code calls note(at:), which queries the data source again to get the identifier. Using the provided identifier directly avoids redundant lookups/work during cell configuration.
| dataSource = UITableViewDiffableDataSource<String, NSManagedObjectID>(tableView: tableView) { [weak self] tableView, indexPath, _ in | |
| guard let self, | |
| let note = self.note(at: indexPath), | |
| let cell = tableView.dequeueReusableCell(withIdentifier: "NoteCell", for: indexPath) as? NoteTableViewCell else { | |
| dataSource = UITableViewDiffableDataSource<String, NSManagedObjectID>(tableView: tableView) { [weak self] tableView, indexPath, noteID in | |
| guard let self, | |
| let cell = tableView.dequeueReusableCell(withIdentifier: "NoteCell", for: indexPath) as? NoteTableViewCell, | |
| let noteObject = try? self.fetchedResultsController.managedObjectContext.existingObject(with: noteID), | |
| let note = noteObject as? CDNote else { |
No description provided.