-
-
Notifications
You must be signed in to change notification settings - Fork 529
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add jsTree based FileTreeSelector #6837
base: main
Are you sure you want to change the base?
Conversation
This is great. It was always my hope that panel-jstree would get put into panel someday. I am gonna put some comments on some parts for the things I was trying to work on when I had time. The next thing I was trying to do for a side project I help with that was trying to use panel-jstree was create a composite widget, which here https://github.com/madeline-scyphers/panel-jstree/blob/1e364fcf5ff1947912ce8ba2b1f4d9753c4498d8/src/panel_jstree/widgets/jstree.py#L290 def _set_data_from_directory(self, *event):
self._data = [{"id": fullpath(self.directory),
"text": Path(self.directory).name,
"icon": self._folder_icon,
"state": {"opened": True},
"children": self._get_children_cb(Path(self.directory).name, self.directory, depth=1)
}] is a FileTree cb triggered on the directory change to swap out the entire data from scratch. I found this worked, but was a bit laggy (lag came from recreate the tree in the browser, not the directory search). Maybe the asynchronous tree will help, but if not, I was trying to play around with the massload plugin to see if that could also help but I never finished it with trying to finish my master's https://www.jstree.com/api/#/?f=$.jstree.defaults.massload One thing I also wanted to mention was that I was trying really hard to make sure that there was a general tree implementation someone could use that would be independent of a FileTree if they just wanted to explore Tree data. It looks like that is still working properly, but I just want to underscore that I think that is really important, and ideally as many features can be generalized to work for a generalized Tree, not just a FileTree. I added support for a number of jsTree's plugins, but there are a few more that might be cool to do. drag and drop was one I wanted to do next, which allows the user to just rearrange the Tree, though by default it can move a leaf to another node, so maybe not for the FileTree. Sort also is another one that might be nice. There is a search plugin as well. That is most of most of what I had left that I wanted to do. I think the remote file provider is super cool, as well as the other features you all are adding. I would love to help with the last things to get this in. |
f"{config.npm_cdn}/[email protected]/dist/themes/default/style.min.css", | ||
] | ||
|
||
__resources__ = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I replaced these icons, let's see if we can get rid of this.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6837 +/- ##
==========================================
- Coverage 81.73% 81.54% -0.19%
==========================================
Files 326 328 +2
Lines 48006 48376 +370
==========================================
+ Hits 39237 39448 +211
- Misses 8769 8928 +159 ☔ View full report in Codecov by Sentry. |
checkbox: { | ||
three_state: this.model.cascade, | ||
cascade: this.model.cascade ? "undetermined" : "down+undetermined", | ||
cascade_to_disabled: false, | ||
tie_selection: false, | ||
whole_node: false, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need more options for cascade. Especially for generic trees outside of just file trees. I am using my version of jstree right now for something at work to model some genome taxonomy and trying to use the panel-jstree widget I was building until this is ready to be able to cleanly subselect by any level of taxonomy (remove all shown samples with this class, phylum, domain, or species) for example.
So when deselecting a class, everything below it should deselect, but when selecting a certain domain, it shouldn't select everything below it. That behavior isn't achievable right now with these two cascade options, and I think for generic, non file tree use cases, that would be something wanted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am rethinking what I want in my own situation actually 🤣 but probably the option to choose more cascade choices would still be good.
props = super()._process_param_change(params) | ||
if 'value' in props: | ||
props['checked'] = props.pop('value') | ||
return props |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this section could be moved to the base tree, since it I think should always happen in any concrete implementation, and the generic Tree implementation needs it too.
def _reindex(self, reset=True): | ||
if reset: | ||
self._index = {} | ||
for node in self._nodes: | ||
for subnode in self._traverse(node): | ||
self._index[subnode['id']] = subnode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential problem with this, jstree doesn't require the initial node json data to include the id (https://www.jstree.com/docs/json/). If you don't include it, then jstree will provide it for you. Obviously the FileTree can assume an id because it is coded that way and it makes sense to make the id the unique file path, but a generic tree using the Tree class may not be constructed with an id and the nodes will only get an id after the first render with the jstree library.
Since the FileTree uses this method to initialize the _index cache on on init, I wanted to mention that this won't work when work is completed to get the generic Tree class working again.
So for my job, I am building a dashboard for some genome taxonomy data that is the output of a model. And I am trying to use my version of panel-jstree and eventually this version to filter based on taxonomy rank (domain, phylum, etc.). While doing this, I have a little function that allows me to convert an edgelist dataframe into a format that jstree needs. So this edgelist df source target
0 a a-1
1 a a-2
2 b b-1
3 b b-2
4 b b-3
5 b-1 b-1-1
6 b-2 b-2-1
7 b-2 b-2-2 Gets transformed into this node list>>>build_tree(edge_df, state={"opened": False, "selected": False})
[{'text': 'a',
'children': [{'text': 'a-1',
'children': [],
'state': {'opened': False, 'selected': False}},
{'text': 'a-2',
'children': [],
'state': {'opened': False, 'selected': False}}],
'state': {'opened': False, 'selected': False}},
{'text': 'b',
'children': [{'text': 'b-1',
'children': [{'text': 'b-1-1',
'children': [],
'state': {'opened': False, 'selected': False}}],
'state': {'opened': False, 'selected': False}},
{'text': 'b-2',
'children': [{'text': 'b-2-1',
'children': [],
'state': {'opened': False, 'selected': False}},
{'text': 'b-2-2',
'children': [],
'state': {'opened': False, 'selected': False}}],
'state': {'opened': False, 'selected': False}},
{'text': 'b-3',
'children': [],
'state': {'opened': False, 'selected': False}}],
'state': {'opened': False, 'selected': False}}] And I was thinking it would be nice to be able to construct the generic Tree class from a edgelist df, maybe as a class_method? I can put in a PR into this branch to add that. It isn't that complicated of a function, and I tried to do it without something like NetworkX to not add any dependencies. Though I imagine that would speed it up. So there could be an optional dependency on NetworkX to speed it up? |
if icon: | ||
jsn["icon"] = icon | ||
else: | ||
jsn["icon"] = "jstree-leaf" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to let people specify any icon from tabler-icons.io from other places? This was always something I wanted to try get working but never got around to. It would especially be useful in a generic Tree when people are using it for all sorts of stuff. Having DNA icons for taxonomy things for my genome projects I am working on, or using local custom icons for a branching history tree, or any other tree.
Thanks for all your helpful review comments @madeline-scyphers! Will try to get back to this PR soon. |
Adapts the panel-jstree components to support selecting both local and remote filesystems.
Done
RemoteFileProvider
ToDo
max_depth
parameter settingTree
implementation usable (or hide it for now)Nice To Have
s3://
) and create a FileSystem