ivhTreeviewMgr service doesn't honor options set through directive attributes

[AbhishekGarg]

Came across this issue while I was testing "selectAll" function with my custom tree object.
http://jsbin.com/xojexahale/1/edit?html,js,output

This is a clone of the demo bin for "Select/Deselect All". Only the children attribute is changed to use a capital case version. Same has been provided as attribute to the Treeview div in HTML.

While debugging, I found that the global options were not being overridden in the ivhTreeviewMgr service. It takes the options as what are being set in the provider.
I see that "selectAll" function and others can be passed options. But ideally, the options should be what have already been passed via the directive attributes.

Proposed fix: Create setter function in ivhTreeviewMgr service and use in directive controller to set the merged options.

Please do let me know in case I am missing something.

[jtrussell]

This behavior is by design. The ivhTreeviewMgr service is shared throughout your application and is used internally by the ivhTreeview directive. It was intended to be stateless (post config phase) because it may need to work with many different tree data stores all of which have different configs. With the current setup you can even use the same tree data to power multiple instances of the treeview directive while using different options for each instance. You may also desire to manipulate tree data with the service before your views even run.

A setter function would not help much in the general case - what if you have two trees using different options? You would need to call the setter each time just in case it has since been called with another tree's options. What if you want to work with your tree data and you couldn't be sure the correct controller had already been instantiated? You would need to call the setter function yet again.

In the future we may provide an ivhTreeviewMgrFactory which you could use like:

var mgrOne = ivhTreeviewMgrFactory(localOptsOne)
  , mgrTwo = ivhTreeviewMgrFactory(localOptsTwo);

// ... mgrOne and mgrTwo have been imprinted with different local options

In the meantime you could create a simple wrapper for ivhTreeviewMgr that accomplishes this - I don't believe we'll be getting around to it soon (it's not even officially in a milestone yet).

I don't think we'll create a strong coupling between ivhTreeviewMgr and local options though. Have you tried configuring ivhTreeviewOptionsProvider?

[AbhishekGarg]

@jtrussell.. This makes sense, ivhTreeviewMgr being stateless as it needs to work for multiple trees.

But, how do we solve the issue depicted in the jsbin? "Select all" functionality should work whether options are provided through config or directive attributes. Otherwise, I will have to keep a copy of the options object and pass to selectAll every time.
Is this the intended usage?

Thanks for the suggestion of the wrapper, will try that.

Yes, I have configured some global options through the provider also. They work as expected. The issue is with local options.

[jtrussell]

It is the intended usage. Ideally there would be some uniformity to your tree structures and you could get by with just modifying the defaults. When that's not possible this is the ended usage, though note that you can do something like this:

// Controller
var opts = $scope.treeOpts = {
  childrenAttribute: 'Children'
  // + possibly other options
};

// Use opts with ivhTreeviewMgr

<!-- view -->
<div ivh-treeview
     ivh-treeview-options="treeOpts">
</div>

[AbhishekGarg]

Yep, I did think about that. Thank you for the quick responses.
This seems related to #49.

It would be really great if this can be addressed, since this seems like reasonable behavior. Passing the options with every call to a manager function looks a little cumbersome to me.

PS: Many thanks for the awesome work on the plugin. Made life a lot easier 👍