sameesunkaria / outlineview Goto Github PK

I was able to use this library on Catalina, the only thing that is not compatible is Styles who landed in SwiftUI 2, besides that all works, maybe you can add @available safeguards

Thanks!

Currently, only single selection seems to be supported. How about making Selection support Set<Data.Element>?

Have you thought of adding drag & drop handling? I'd be interested in trying to work on it if you're not already doing so and think it would be a good addition.

NSOutlineViewDelegate.outlineView(_:viewFor:item:) discussion says that the view creation should use reusable cell views:

It is recommended that the implementation of this method first call the NSTableView method makeView(withIdentifier:owner:) passing, respectively, the tableColumn parameter’s identifier and self as the owner to attempt to reuse a view that is no longer visible

The current implementation of OutlineView creates a new NSView for every row and ignores the reusable views aspect of NSOutlineView. This makes implementation simpler (and more SwiftUI-like), but would it be worth adding functionality for reusing views?

I've made a basic implementation of this that increases the complexity of the OutlineView initializers, but maybe there's a better way. Here's how my first-attempt changes the initializer and data-source behavior:

• Adds a new generic type to OutlineView: CellType: NSView
• Replaces OutlineView's var content: (Data.Element) -> NSView closure with two closures--
  • var generateNewCell: () -> CellType which creates an empty cell
  • var configureCell: (CellType, Data.Element) -> Void to apply the Data.Element to the empty cell.
• In OutlineViewDelegate.outlineView(_:viewFor:item:):
  • attempts to get a reused CellType cell from the NSOutlineView, and if it fails to get that, it creates a new one with generateNewCell(), and applies a reuseIdentifier in code, which will allow that cell view to be reused later.
  • After getting the CellType view, calls configureCell closure to apply the Data.Element to the view.

The main downsides to this approach are that we now have an extra generic type in OutlineView, and more parameters for all of its initializers. The upside is that it follows NSOutlineViewDelegate's preference for reusable views, which I guess is more efficient.

I discovered a possible bug while trying to debug another issue I was running into, where pressing 'enter' on a row with editable content was not enabling text editing (very similar to this StackOverflow question). I found that every time the NSOutlineView's selection changed, every unexpanded item visible in the view was being reloaded. This was happening in the following point of OutlineViewUpdater.performUpdates:

if !diff.isEmpty || oldIDs != newIDs {
    // Parent needs to be updated as the children have changed.
    // Children are not reloaded to allow animation.
    outlineView.reloadItem(parent, reloadChildren: false)
}

I'm not sure if excessive reloading could cause any other problems besides blocking the 'enter' key behavior.

In trying to fix the 'enter' issue, the suggested fix from the SO post (calling reloadData(forRowIndexes:columnIndexes:) didn't work, so I tried changing the above code from OutlineViewUpdater to this:

if !diff.isEmpty || oldIDs != newIDs {
    // Parent needs to be updated as the children have changed.
    // Children are not reloaded to allow animation.
    let row = outlineView.row(forItem: parent)
    let oldParentValue = outlineView.item(atRow: row) as? OutlineViewItem<Data>
    if oldParentValue != parent {
        outlineView.reloadItem(parent, reloadChildren: false)
    }
}

That fixed the problem, and resulted in reloadItem not being called so frequently, but did cause OutlineViewUpdaterTests to fail-- outlineView.reloadedItems came out as [0, 1, 3, 6] instead of the expected [nil, 0, 1, 3, 6].

It seems to me that not reloading nil in the test case is probably fine, since reloadItem(nil) causes the entire NSOutlineView's contents to be reloaded, and I don't think that's what we want in the OutlineViewUpdater. But that change of code could lead to other problems. Since equality between oldParentValue and parent is determined by id rather than more detailed equality checks, updating data externally could lead to some rows not being updated (say, if you change the name of an item but not its id). So I'm a bit stumped on what the best way of dealing with this should be. Maybe nothing, unfortunately.

how to add items dynamically and not have the tree refresh