This repository was archived by the owner on Mar 5, 2023. It is now read-only.
Fixes, enhancements and back porting to macOS 10.11 #3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Removed Grouped outline item and moved it to demo instead. Replaced with a subclass. Simplified code to use OutlineViewItem as a concrete type Improvements to demo. Only nodes with children show an expansion indicator. More documentation WIP - Need to fix drag and drop
Reload parent item (old and new) after a move as expansion states don't otherwise refresh
…ving items would otherwise remain broken. Reload parent items when removing / inserting
…g "CollectionDifference" related methods Renamed collection difference methods to avoid clashes on macOS 10.15+
…h deletes child items too Documentation and cleanup Perform reloadData() instead of animating if changes above a certain threshold New test to confirm moving items with children Fixed NSOutlineView glitch where expansion arrow would overlap the label after a remove + insert
Renamed some variables for clarity Renamed IndexedID to IndexedNode Added itemID to Node for consistency
shpakovski
reviewed
Oct 20, 2021
Collaborator
shpakovski
left a comment
shpakovski
left a comment
There was a problem hiding this comment.
Thanks for the PR. I’m happy to see new ideas. However this looks like a new component. Not sure 100% how to handle such a massive branch…
But I’m not going to merge it right now, sorry. Instead I’d like to ask you to maintain your public version and make another request for the README with a URL of your implementation.
Thanks for understanding 🙏
| let viewController = NSTabViewController() | ||
| viewController.tabStyle = .unspecified | ||
| viewController.transitionOptions = [] | ||
| viewController.view.wantsLayer = false |
Collaborator
There was a problem hiding this comment.
Why do you need this change?
| Parent 3 / Child 31 | ||
| Parent 3 / Child 32 | ||
| Parent 3 / Child 33 | ||
| Cars / Toyota |
| var snapshot: DiffableDataSourceSnapshot = .init() | ||
| snapshot.fillItem(nil, with: textView.string) | ||
| snapshotBinding.wrappedValue = snapshot | ||
| snapshotBinding.wrappedValue = snapshot |
Collaborator
There was a problem hiding this comment.
Please check the whitespace leftover.
|
|
||
| /// Necessary for sets. | ||
| public override var hash: Int { title.hash } | ||
|
|
||
| /// Necessary for outline view reloading. |
Collaborator
There was a problem hiding this comment.
Could you please explain why this isn’t necessary anymore?
| override var hash: Int { title.hash } | ||
|
|
||
| /// Necessary for outline view reloading. | ||
| override func isEqual(_ object: Any?) -> Bool { |
| if parentItemToAppendTo != nil && appendToParentNode == nil { | ||
| #if DEBUG | ||
| print("insertItems - Cannot find parent item \(String(describing: parentItemToAppendTo))") | ||
| #endif |
Collaborator
There was a problem hiding this comment.
Care to remove spacing in the beginning? 😊
| if targetItem != nil && targetItemNode == nil { | ||
| #if DEBUG | ||
| print("Cannot find item \(String(describing: targetItem))") | ||
| #endif |
| public func outlineView(_ outlineView: NSOutlineView, isItemExpandable item: Any) -> Bool { | ||
| guard let item = item as? OutlineViewItem else { return true } | ||
| return item.isExpandable | ||
| return item.isExpandable && currentSnapshot.childrenOfItem(item).count > 0 |
Collaborator
There was a problem hiding this comment.
This may not work after reloading. I introduced isExpandable and count > 0 explicitly to permit the disclosure indicator without children.
|
|
||
| public extension OutlineViewDiffableDataSource { | ||
|
|
||
| /// Returns current state of the data source. This property is thread-safe. |
| } | ||
| // When a data source is not set, reload the first time | ||
| guard animatingDifferences, let _ = outlineView?.dataSource else { | ||
| reloadData() |
Collaborator
There was a problem hiding this comment.
Is this effective enough?
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR includes a number of enhancements, fixes and a back-port of the diffing algorithm (taken from the open source Swift implementation) so that the module itself can be used on macOS 10.11.
I've also fixed a number of bugs present within the demo with some additional optimizations / tests. Duplicate code has been removed from a few places, plus the code is now easier to follow.
Along the way I've also fixed a few known glitches within NSOutlineView (such as parent nodes appearing with overlapping glyphs).
The PR also includes more documentation and some refactoring.