Skip to content
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

Added functionality #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

macMikey
Copy link
Contributor

  • Properties to expand all nodes
  • Function to return root folder
  • Filter for file extensions

* Properties to expand all nodes
* function to return root folder
* filter for file extensions
@trevordevore
Copy link
Owner

@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:

  • Code needs to be explicitVars safe.
  • Don't use #<COMMENT>, #</COMMENT> to wrap code or use # COMMENT to mark the end of if or repeat blocks.
  • Code should not be indented under comments.

@macMikey
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Contributor Author

macMikey commented Feb 26, 2019

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
Copy link
Owner

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
Copy link
Contributor Author

macMikey commented Mar 5, 2019

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
Copy link
Contributor Author

macMikey commented Mar 5, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants