Interesting observations

#1

  1. FileTreeComponent::setSelectedFile does not look like it could ever work. The target file is passed in correctly, but then it looks through a list of selected files and if it matches one it selects it else it deselects them all. I would have thought it should select the target file whether it was selected or not and if not found, then deselect everything.

  2. I notice your listnox has the following elements:
    a. Outline colour and size
    b. item click and double click (which is in the TreeView component)
    c. item delete key pressed and return key pressed
    d. setselecteditem also works.
    I would think that your treeview component and all other components of that nature should all have consistent capabilities. I certainly could use it for a consistent look and feel to my VST panels. It would also be nice if labels also had the outline capability so it could be used to further consistent look and feel.

I am using Ctrlr, so I realise some issue could be caused by that, but I believe the issues I have identified here are at the JULES class level rather than the Ctrlr level as I have looked at the documentation and the source code I have downloaded and it appears to have the same limitations.

Thanks.

#2

No… either I’m being dim, or you’ve misread the setSelectedFile method. It looks ok to me…?

Re: adding outline colour to other classes - TBH if I re-wrote the listbox today, I’d remove that from it - things like that belong in the look+feel class, not the component class itself, and it’s certainly not the kind of thing that should be extended to other classes. The fact that the listbox does it is just for legacy reasons, really.

Don’t really understand your point about other components needing to respond to clicks, double-clicks etc - obviously any component can respond to any kind of input event.

#3

setSelectedFile is what I was using, but in single stepping the code the file gets passed in but is never set as the selected file.

void FileTreeComponent::setSelectedFile (const File& target)
{
for (int i = getNumSelectedItems(); --i >= 0;)
{
FileListTreeItem* t = dynamic_cast <FileListTreeItem*> (getSelectedItem (i));
if (t != nullptr && t->file == target)
{
t->setSelected (true, true);
return;
}
}
clearSelectedItems();
}

It appears to get the number of selected items. Compares that list to the target file. If one matches it sets it as selected. It does not appear to look through the entire list of files and then if it locates a match, set it as selected.

Now I too may be dim, but that is what it looks like the code is doing, is it not?

#4

(Doh, sorry, I was looking at FileListComponent instead!)

Yes, you’re right, that function was indeed nonsense! I’ve done a quick fix, didn’t have time to test it, so let me know if it works…

#5

Cool, thanks, not sure when I can test it as it depends on how quickly Roman updates Ctrlr, but at least when he does it should work.

On the other topics, I agree that they should all use look and feel to do those things but it would be nice if look and feel provided methods to do them rather than having to use using C++ brute force, by using the paint method.

The click and double click capability is provided for in both the list box and the file tree component, but I will be damned if I can find out how to get the delete key and the return key callbacks in the file tree like they are in the list box. They are working great for me in the list box.

Thanks once again.

#6

This is sort of a bump to my previous post.

I like the idea of Look and Feel providing all the appearance functions, like outline etc. That would be great across the board.

Still on with the returnKeyPressed and deleteKeyPressed. These are in the ListBoxModel and function for a ListBox and a FileListBox, etc. Should they not be available in the FileTree Component and such components also?

#7

Well not really… those methods would be redundant in a Component subclass, because they already have keyPressed and mouse-event methods that could be used to do the same job. ListBoxModel has them because it’s not a Component, and there’s no other way to catch them.

And FileTreeComponent is only designed to be very simple - if you’re writing a custom file tree that goes beyond what it can do, then you’d probably just want to write your own tree model class anyway. As you can see, FileTreeComponent is only a tiny class, it wouldn’t be difficult to write your own version of it.

#8

That may be true, but why does the FileL:istCompnent have them then, like the ListBoxModel. As per:
void FileListComponent::deleteKeyPressed (int /currentSelectedRow/)\r
252 {\r
253 }\r
254 \r
255 void FileListComponent::returnKeyPressed (int currentSelectedRow)\r
256 {\r
257 sendDoubleClickMessage (fileList.getFile (currentSelectedRow));\r
258 }\r

It looked to me like it might have been an oversight on the FileTreeComponent.

#9

The change you made did not work. I did not track down where your variable file came from but it had a fullpath of D:\ whereas the filetreecomponent had a full path of C:\Temp\ and the file passed in was c:\Temp\aaa.syx and so it did not work at all. The code it failed in was:
bool selectFile (const File& target)
{
if (file == target)
{
setSelected (true, true);
return true;
}

    if (target.isAChildOf (file))
    {
        setOpen (true);

        for (int i = 0; i < getNumSubItems(); ++i)
            if (FileListTreeItem* f = dynamic_cast <FileListTreeItem*> (getSubItem (i)))
                if (f->selectFile (target))
                    return true;
    }

    return false;
}

It compared file to target and they were not equal, then it asked if target is a child of file. As they were two separate disk drives this was not true either so it all failed.

#10

Well of course… if the tree doesn’t contain the file you’re trying to select, what did you expect it to do?

The root folder is specified in the FileTreeComponent’s constructor. setSelectedFile can’t change that for you just because you pass it some random filename.

#11

I do not think you get my point. I do not know where the D:\ came from. Why don’t you look and find out where ‘file’ came from? The root.fullPath is set to C:\Temp and target is set to C:\Temp\111.syx, so how is it that file is set to D:??? This is not what has been set in the component!!! There is an issue here and I did test it like you asked. I still do NOT know where the variable file is being set!!!

#12

Sorry, maybe I misunderstood you, but I still can’t see any problems with this…

If you mean FileListTreeItem::file, then it’s set in the constructor - it’s a const.

#13

Okay, thx, now I see what is happening. The FileTreeComponent is constructed using a DirectoryContentsList. When initially constructed the FileTreeCompnent creates the FileListTreeItem using D:\ as the file_. This value then becomes ‘file’. This DirectoryContentsList then has the setDirectory method called setting D:\ as the directory. Another call is made to the DirectoryContentsList::setDirectory and this time C:\Temp is set as the directory. Next there are more FileListTreeItem called setting the contects of the C:\Temp directory. Now we create our new file 111.syx. Next setDirectory sets the directory to empty and root goes to “”, and then called again and set to C:\Temp. This is performed to redraw the FileTreeComponent with the new added file 111.syx. Now FileTreeComponent::setSelectedFile is called with C:\Temp\111.syx as the target. This call t->selectFile passing the target file correctly but “file” is set as D:\ not as C:\Temp. Now I guess it is up to you to perhaps explain why this is being used.

#14

You need to call refresh() if you make the DirectoryContentsList point somewhere else.

#15

Okay, cool, so I tried the refresh(). First I tried:
directoryContentsList->setDirectory (File(getProperty(property)), true, true);
directoryContentsList->refresh ();
and nothing changed. ‘file’ was still D:. Then I tried:
directoryContentsList->setDirectory (File(getProperty(property)), true, true);
treeComponent->refresh ();
and it had an access violation on the following GetMessage:

bool MessageManager::dispatchNextMessageOnSystemQueue (const bool returnIfNoPendingMessages)
{
using namespace WindowsMessageHelpers;
MSG m;

if (returnIfNoPendingMessages && ! PeekMessage (&m, (HWND) 0, 0, 0, 0))
    return false;

if (GetMessage (&m, (HWND) 0, 0, 0) >= 0)

where should I go from here?

#16

Sorry, my comment wasn’t too clear - I actually meant call FileTreeComponent::refresh, not DirectoryContentsList::refresh. Otherwise, the tree has no way to know that its root has changed.

#17

Thanks, yes, we figured that one out and unfortunately in Ctrlr, there is an access violation on a GetMessage as I said in my previous post. Atom is helping me with that one now to determine the issue.

As fas as setSelectedFile, if the file is in a subdirectory that is not open, it opens the subdirector, but does not select the file. If the subdirectory is open it selects the file. Should it not select the file regardless?

#18

Looking at FileListTreeItem::selectFile(), I can’t see how it could get as far as opening a subfolder without selecting the file… If you can show me a bug in that method, please do!

#19

The problem appears to be thjat when selectFile is first called with target and the subdirectory is not open, all is correct but when it gets to the isAChild of true gets returned and it enters the code, performs a setOpen and then goes down the list. The only problem is that the list it is going down is not the list with the newly opened directory, so it will never perform the selectFile on the target on this pass of setSelectedFile. It will set it the next time it is called, as the file is in the original FileList that is passed in.

I hope that is clear, if not please let me know as I could put together an rather easily reproducible way to get this to happen so you too could single step it.

#20

No, sorry, that wasn’t very clear at all! I think I see the problem now, which may or may not be what you were trying to explain… but try this (untested):

[code] bool selectFile (const File& target)
{
if (file == target)
{
setSelected (true, true);
return true;
}

    if (target.isAChildOf (file))
    {
        setOpen (true);

        int maxCount = 5000;
        while (subContentsList != nullptr && subContentsList->isStillLoading() && --maxCount > 0)
            Thread::sleep (1);

        changeListenerCallback (nullptr);

        for (int i = 0; i < getNumSubItems(); ++i)
            if (FileListTreeItem* f = dynamic_cast <FileListTreeItem*> (getSubItem (i)))
                if (f->selectFile (target))
                    return true;
    }

    return false;
}

[/code]