trevordevore / levurehelper-file_browser

Levure helper for creating a file browser control in your application
MIT License
1 stars 2 forks source link

Added functionality #4

Open macMikey opened 5 years ago

macMikey commented 5 years ago
trevordevore commented 5 years ago

@macMikey Thanks for submitting enhancements. In order to keep PRs manageable, each PR should be for a specific feature addition or bug fix rather than combining a number of things into one. It makes it easier to review and provide feedback. If you can focus on one particular feature addition for this PR I would appreciate it.

A couple of additional comments:

Property to expand all nodes: the DataView Tree already has the ExpandAllNodes and CollapseAllNodes commands. There shouldn't be a property with the same name and I don't think it is necessary. The developer can call ExpandAllNodes in order to expand all nodes.

Coding style: I need to put together a document that describes the coding style used in my repos. Some things to be aware of:

macMikey commented 5 years ago

Expand - let's make sure I understand the behavior: If I want a directory structure to be expanded I have to set the dvrootfolder and then expandallnodes, right? Doesn't that mean a draw and then a redraw? I don't see any other solution. After renaming or moving a file, does one have to set the dvrootfolder and then expandallnodes again? Does setting the dvrootfolder maintain the folded/unfolded state of the folders, or do they reset to folded? It would be cool if it kept the folded state, because then a move or rename would only require telling the fb to redraw, and the user's fold preference on the individual folders would be persistent. If the fb does not store the user's preference for each folder, then the draw/redraw is required to show all the files. The goal of the property is to get rid of the redraw.

macMikey commented 5 years ago

AFA the coding style, every time I work on something for levure I find myself going back and redoing at least some of it to get it into the form you prefer, so apologies that this will be a recurring issue.

trevordevore commented 5 years ago

dvRootFolder and expanded nodes - dvRootFolder is a complete reset. Setting the dvTree will always updates the tree array with whatever array you assign to it. Based on your description of I think you only have to account for moving. There is no need to render the whole tree again after a rename. Just update that specific node in the tree. For moving there could be a refresh which loops through all nodes and refreshes the content. If a folder is open then a new file listing would be fetched. This would be more efficient.

Regarding rendering twice - I can think of two approaches. One is don't have dvRootFolder render the tree at all. Require RenderView be called. Or allow a pRender param to be used when setting the dvRootFolder. dvTree allows you to bypass the render if you want to perform other operations before the first render:

https://github.com/trevordevore/levurehelper-dataview_tree/blob/master/dataview_tree.livecodescript#L61

macMikey commented 5 years ago

You can't just update that node, because you will lose the sort for the directory if the new name causes it to move in the sort order. For either dvrootfolder or renderview, you bring up an interesting possibility, refreshing the array instead of rebuilding it. That's something that should be looked at. After reading through the array code, it also looks like a PITA (mainly because I'm trying to get my brain wrapped around your array code), but it should be discussed. For the rendering, why not have both? You can set a property for autoRender, and have a renderView command to do it manually

set the autoRender of group "fbdvt" to false
set the dvRootFolder of group "fbdvt" to "~"
massageDirectoryContents
renderView

or set the dvRootFolder of group "fbdvt" to "~" #when autoRender is true At least in the application I'm working on, the fb should be expanded by default. Yes, there will be plenty of applications where it should be collapsed by default. That's why I think there should be a property of some sort. It could be foldersDefaultFolded. What would be interesting about that property is if the user has set the folded/unfolded of several directories, and then a folder is added to the tree for whatever reason it is either expanded or folded as we default.

trevordevore commented 5 years ago

Updating node after rename – Oh right. But you could just update the sibling nodes under the parent of the node that was renamed. That would be much more efficient. You can set the dvNodeChildren or dvRowChildren properties to just update children of a parent.

As I was thinking about the autoRender property I realized that it would be best to write up a complete API for the File Browser so it could be looked at as a whole. If you are able to write up a first version of the API you envision then it would be easier to provide feedback.

macMikey commented 5 years ago

It probably would be. Much of what I was working on implementing was not API, it was functionality that I was adding. It might be better to start with the documentation for the DV and the DVT and then work into the FB, because I'm not convinced I really understand either the DV or the DVT, and the docs for both are a little sparse.

macMikey commented 5 years ago

Yes, rename is the simpler of the two, but for a move, you have a bigger can of worms, depending on where the file is relocated.