bootstrap-treeview
bootstrap-treeview copied to clipboard
New filterResults search mode
I have implemented a search result filtering mode, #47. I'm currently testing it in my application; it seems to be working rather well. Can you please review and let me know if there's anything else I can do to get it merged?
Thanks, -Roman
Thanks for your effort in attempting this feature. I can see it may have taken you some time to implement.
However, during my testing I did come across a number of issues. Mainly around the expand / collapsed state, illustrated in the following two screenshots.
1. Parent icon indicates collapsed state, but it's expanded

2. Similar issue when only a subset of children match the search criteria

Long story, short, I had a look at your code, then hacked around with it and found that most of it was unnecessary and slightly convoluted. I can appreciate the angle you where coming and for the most part it made sense. However, based on my understanding of the design I can see a much simpler and elegant way to implement this feature.
As a result, I have hacked around with your feature and put together a basic proof of concept of how I believe it would be best implemented. It's not finished by any means, the code needs some serious cleaning up (please see TODOs) and I haven't touched tests (which now fail), example or documentation etc. However, based on the limited testing I've had time to do, it appears to work correctly.
I couldn't update this PR, or push to your branch, the only way I can see to share these changes with you is to add as attachments. Please see files bootstrap-treeview.js.txt and bootstrap-treeview.css.txt below.
bootstrap-treeview.js.txt bootstrap-treeview.css.txt
It would be helpful if you could finish this feature, continuing along the lines that I suggest. Otherwise I will eventually get around to doing it myself, probably not for a few weeks.
At this point, my only remaining concern is the relation between revealResults and filterResults. In order for filterResults to work it implies revealResults. I'm wondering if there's a better way to define these options, but at the very least this relationship would need to be documented.
Thanks for looking it over!
I went with an approach that doesn't introduce any extra 'filtered' node state. My reasoning was that since you already have the set of matched nodes, anything that is filtered is the set difference between all nodes and the matched nodes.
I'm also using the set differences between the previous matched nodes and the current matched set in order to only show/hide the nodes that changed since the previous search; that should mean fewer DOM updates. These things do complicate the code.
Regarding the collapsed/expanded state issues that you encountered - that was actually done by design. If a parent node has filtered children, it's state remains 'collapsed', so that you could still expand the parent and see the list of filtered children. That's handled by the call to _updateExpandedStatusOnFilteredSearch
.
Granted, I think it's a little confusing - it may be best to correct it by keeping it in 'expanded' state, and introducing another icon to expand all filtered children.
Or we could just hide the collapse/expand icons when doing result filtering and keep it simple.
In your attached code, if you filter by, say 'Parent', you'll get 'Parent 1' with a collapsed icon - which is also confusing (that's what the call to _updateExpandedStatusOnFilteredSearch
is designed to fix).
-Roman
I think a filtered node state would be preferred. Something I want to be able to achieve in the near future, is the ability to save a tree's state and subsequently recreate the exact tree. Although the code I proposed is far from optimal at this point, it could be improved drastically using a separate node state along with findNode and/or $.grep style methods to isolate the minimal DOM updates.
Updating the expanded/ collapsed state is something I really don't think we should be doing, with the exception of revealNode which handles the expansion from leaf to root node. The filter method should change only a nodes visible state, it shouldn't be changing expanded state. For example, when you clear a search filter I would expect the tree to return to it's previous user generated state.
However, having looked back over the functionality provided by your implementation vs my quick hack. I do prefer the fact that your implementation allows the user to click expand to reveal the filtered children. As a user you can apply a filter and still use the tree to navigate. It's just the icon displayed is a little confusing, as well as the fact that you can view filter results i.e. the filtering is not enforced.
Perhaps I need to revisit your implementation and see if we can find some middle ground between the two approaches. I'm a bit busy with other things at the moment, but will try and schedule some time soon.
Hi guys I just implemented the current version of the bootstrap treeview and it's fantastic. Thanks! The filter feature is something I would really love to have, but my coding isn't good enough to contribute. Any idea, when the feature will make into a stable version?
- Charles
How do I use this filter feature? Thanks, Carmen
Any update on this?