Bug: Sparse dir listing on tree collapse

[dereks]

If using config.listFiles set to false, which enables showing just sub-directories in the tree, there is a weird UX error.

When you click on a dir in the tree, to expand it and show its contents, everything works as expected. All files are shown in the detail pane, but only dirs are shown in the tree.

But if you click the dir in the tree again, to collapse the (expanded) node, there is a bug where the detail pane suddenly shows only the sub-dirs. The files are suddenly hidden.

The error is in this function:

this.openNode = function(node) {
	if(node.rdo.type === 'file') {
		getDetailView(node.rdo);
	}
	if(node.rdo.type === 'folder' && node.isLoaded()) {
		var childrenObjects = [];
		if(node.children().length) {
			$.each(node.children(), function(index, cNode) {
				childrenObjects.push(cNode.rdo);
			});
		}
		model.currentPath(node.rdo.id);
        model.breadcrumbsModel.splitCurrent();
		model.itemsModel.setList(childrenObjects);
	}
};

When openNode() is called the second time (after the node has been expanded), node.isLoaded() is true, and it falls to re-using the previously loaded children with model.itemsModel.setList(childrenObjects); But thanks to config.listFiles only the sub-dirs are present in childrenObjects. This causes the detail pane to suddenly show only the sub-dirs, when all the user wanted to do was collapse the tree element.

My first attempt at a fix fell back to doing an AJAX load of dir contents everytime, regardless of whether it was previously expanded or not. That fixes the symptom described above.

But after playing around with the tree for a while, I feel strongly that it's much more intuitive if collapsing the node doesn't change the detail pane at all (i.e., collapsing a dir does not changeDir into that dir). It should just be a passive "collapse this dir node -- I'm done with it", rather than a "collapse this dir node, but also switch to it in the detail pane".

So I will submit a PR with this fix shortly. It will fix the bug but also change the behavior so that collapsing a dir node does not show that dir's contents -- the user is now able to collapse tree nodes but stay with their current detail view.

Thanks!

[dereks]

PR with fix submitted. (This PR is in a clean branch from dev, and excludes my other Markdown-related changes. But this fix will be merged into my Markdown branch too.)

Thanks!

[psolom]

Good catch, thanks for the report and PR. I will review it soon.

[dereks]

Please note, part of this PR includes updating the (hard-coded) reloadOnClick option to true, so that dirs are always reloaded when expanding them (but not when collapsing them). That might cause a merge conflict if you have made reloadOnClick an editable option in the config file.

(IMHO, reloadOnClick should be hardcoded to true and not even made configurable. It can cause hard-to-understand and diagnose "missing files" issues for shared filesystems. But if set to false, the only advantage is slightly fewer AJAX calls to the connector -- not worth the documentation and support headache.)

Thanks!

[psolom]

1. I have made ajax calls for filetree and fileview are independent. When you click upon tree node and "reloadOnClick" is set to true the application performs 2 ajax requests. That should fix the bug. Check it on the dev branch, please.

2. I have created new section in the json configuration file - "filetree" and moved all associated options to it, including "reloadOnClick". By default it is set to true. Note that "listFiles" is also has been moved to this section and renamed to "foldersOnly".

[dereks]

I tested it with dev and it works great, thanks.

I noticed a pleasant UX change. It used to be that clicking on either the dir name, or the minus-sign button to the left of it, would cause the expanded dir to collapse. But now clicking the dir name just reloads the dir, and does not collapse it. That feels more natural to me. "Click-to-refresh" is all the user needs to remember. Thanks for this change!

As an aside, I was thinking it might be possible to reduce the number of lines of code in filemanager.js if we used a tree library. My personal favorite is the popular JSTree. Please take a quick look, and click around on their interactive demos on the right side of the page:

https://www.jstree.com/

It has a polished UX, lazy loading, theme support, multi-select, good docs, MIT license, and a plugin community -- including a plugin for mobile devices. I think the tree widget in RichFilemanager is elegant and well done, but JSTree might look even better, while providing additional features and fewer lines of code and CSS to maintain.

I believe updating to this library would require replacing much of the TreeModel and TreeNodeModel code with registered event callback functions, using the config API used by JSTree. The user interface logic could be removed, but the core AJAX event callbacks for updating the ItemsModel() would remain the same.

Would you be interested in seeing an integration of JSTree in RichFilemanager? I'd be willing to take a crack at it, if there is interest.

[psolom]

Hi @dereks

I used some of tree plugins before, but came to the current solution. All of the plugins have limitations and not flexible as much as it's required. Each one that I have tested is perfect at first sight, but later you are facing with a lot of problems. Lazy load will be implemented in one of the next releases. Some ideas I will get from #99, #101 PRs, which are suggested by @Iworb, but the final implementation will differ.

At the moment I'm working to adapt you markdown feature PR to the different configuration cases and fixing bugs. It have brought a lot of troubles )

[dereks]

fixing bugs. It have brought a lot of troubles )

Thanks for working on it!

I am happy to help fix any bugs caused by the Markdown feature, just let me know. (But I understand that writing up a bug in a GitHub Issue is sometimes more work than just fixing the bug yourself.)

Each one that I have tested is perfect at first sight, but later you are facing with a lot of problems

Hmm, very interesting. This is different from my experience. Could you tell me some of the problems you encountered?

I just started playing around with an experimental jsTree branch. I'm hoping I can get it to render, just to see what it looks like. The list of supported jsTree events is very extensive (before_open, open, after_open, etc.):

https://www.jstree.com/api/#/?q=.jstree%20Event

[psolom]

This one was the last:
https://github.com/zTree/zTree_v3
http://www.treejs.cn/v3/api.php

It has a very wide API, but there were some issues, that I wasn't able to handle. Unfortunately I don't remember which ones. Also I don't remember a previous tree plugin that I tested.

New experience is nice, so keep playing with jsTree. I expect for the feedback. But keep in mind that there are a lot of various cases for RichFilemanager based on the configuration options.