Buxfixes

[Augustifolia]

Contains fix for issue #23 and issue #20.

I am also planning to include fix for #22

[Augustifolia]

Just pushed a fix for #22.

I also did a minor change to scrolling of the panels in the sidebar: if the whole panel is already visible (and the scrollbar is hidden) you can not scroll it with the mouse wheel anymore.

[fireclawthefox]

Nice additions, thanks for your help!

One thing I just thought about is the ctrl-delete combo. Since the keys are probably a bit far away from each other, I think changing it to ctrl-x might be a better choice (and also goes more in line with what my inspiration for this - blender - does). In addition we can keep the delete without the ctrl prefix. I did re-check the behavior and I already implemented something to not delete the widget if typing in a textfield of its properties, so it should be save.

What do you think about this? Otherwise your PR looks fine and if you don't have time to add those changes yourself or have some reasons against them, I'll merge it as it is now.

[Augustifolia]

I think that sounds reasonable. ctrl-x is already bound to cutting an element, but as long as we just remove the element from the canvas directly when ctrl-x is pressed that should work fine.

By the way, I just noticed that if you try to paste a cut element while no element is selected, the program crashes.

We should probably also add a check to see if the user tries to paste an element to a child-node of itself. Because that causes some problems. Or what do you think about that? Maybe it could be useful, at least when copying elements.

[fireclawthefox]

Ah, forgot about the cut function. Maybe ctrl-d or ctrl-k for delete or kill would work. Of course cutting without pasting it again would kinda be the same, but it will overwrite the clipboard space which may not be desired.

As for the pasting, of cut elements without selection, that is something that should get fixed. I also wanted to move more of that stuff into the external library so it can be used and such errors be fixed in a central place for all the other editors too.

Pasting an element on itself should be fine as long as it's copied and yes, cut elements should be ignored as paste targets.

[Augustifolia]

That is a good point, did not think about that. ctrl-d is quite convenient, although not obvious for a new user, but as long as it is documented that should be fine. The only issue I have found with keeping delete as keyboard shortcut is when writing directly in a DirectEntry. Then delete will just delete the element instead of removing a character.

It would still be nice to get some feedback from the application when something is cut. For example it being removed or greyed out.

I think I have a fix for pasting without selection, will commit that soon. Are you referring to the FRAME-Editor-Base? That would be useful, but I am not that familiar with the code-base yet. So I don't really want to tackle that right now.

Great, then I will take a look on pasting elements on themselves and all of that.

[fireclawthefox]

Right, in a DirectEntry that does not have the entry event set to disable the delete and other commands, this will still remove the widget once the delete key is hit. Not quite sure how to go about that yet.

Giving some visual feedback to the users is important so I'm fully with you on that point. I prefer it being grayed out over removed though since it should only be removed once pasted somewhere else, that's more like the common behavior.

And, correct, the FRAME-Editor-Base is where I try to move most of this stuff. The base of the cut and paste mechanic or more specific, the kill-ring is also in there already. Just the more specific mechanics are in the editors themself yet, but some of that code may also be moved there. That base library is not so big yet and basically just a loose collection of stuff that has been used around the other editors. I try to keep it somewhat structured similarly to the editors so it shouldn't be too hard to find stuff there once you know how one editor is set up.

[Augustifolia]

I would prefer to use ctrl-d and ctrl-del for now. And if we find a better solution for handling DirectEntries we can change it back to delete then. But if you want to use delete instead that is also fine.

Right, then I will take a closer look at the editor-base. Is the plan to make that a dependency of the gui designer and the other editors?

[fireclawthefox]

I think it's fine for now, if I'm using the editor more often again and find it cumbersome and have a good reason, I'll change it back again.

As for the kill-ring, you can think of it as a tree, where every action enlarges a branch. You can always undo and if you do something new afterwards, it will create a new branch with info information. Changing between branches is fine from the toolbar or with a key combination. It's inspired by emacs kill-ring.

The editor base will be somewhat of a dependency for the FRAME and editors I create, but it will not be a forced dependency for others editors to get integrated into the FRAME.

[Augustifolia]

Thank you for the explanation. The kill-ring is really cool! However it would be quite helpful if there was some documentation for it. Especially for new people reading the code. I could start to write some doc-strings for the parts of the code-base that I understand, if that is something you would like to have?

[fireclawthefox]

Documentation is always something good to have, I try to add doc strings where possible and if I have the time to and don't forget them :P In addition writing some end user documentation is still on the agenda but would best be done once the application is more finished so it won't need to be changed too often.

[Augustifolia]

Yeah, I too tend to forget about writing documentation. Then I will start writing some docs soon. Probably during the weekend. End user docs would, of course, be useful. But I agree that that can wait.

There is still a couple of bugs I would like to fix before merging this: The kill ring issue: #25 Pasting onto a scrolledFrame does not paste the element to the canvas

[Augustifolia]

I've been working on a fix for the second issue, but I have a question about the calls to newParent = self.mainView.getEditorRootCanvas().find("**/{}".format(parent.element.getName())) in __copyBranch. What is the reason for doing that? Why not just use parent.element instead?

[fireclawthefox]

Right, this might work with the parent.element. Just need to make sure it always is an object that has the possibility to be reparented to, but I think that should be the case since usually the objects stored in the element should have a class derived from a DirectGui widget which has the required NodePath class features. It could be that this is an old part from the time where I haven't stored the widgets in the element variable and only used the name. It's been a while since I looked at that part of the code.

[Augustifolia]

Right, it seems safe to use parent.element. I did some testing and found no issues with it.

[fireclawthefox]

I think the more problematic elements would be widgets that don't want the children to be patented directly but to something specific like the scrolled frames canvas.

[Augustifolia]

Yes, but that did not work previously with newParent = self.mainView.getEditorRootCanvas().find("**/{}".format(parent.element.getName())). The reason that I wanted to change it to parent.element is that then newParent is still a gui object instead of just a nodePath. Which makes it easier to check if the element is a scrolledFrame and reparent to its canvas instead.

Just did a commit with that change if you want to check it out. If you want to fix this in some other way, that is of course completely fine. This is your project after all.

[fireclawthefox]

Looks good to me, it also read a lot simpler now and if it's working well, I'm happy with that. One thing that just came to my mind is saving and loading the GUI project. Sometimes that's also affected by such minor things... I should definitely go thorough the code again and check what's important for the save and load mechanism. On a side note, I won't be available much throughout the coming week, but if everything goes as planned, I may have more time for this and other parts of FRAME again the week after. If you don't mind, I'll also merge your changes then. You've done a great job so far, thanks again.

[Augustifolia]

Oh, loading and saving is something I did not think about testing, but that does not seem to be affected by this change. However I just noticed a bug with the save system: after saving and loading the childnodes of a scrolledFrame is no longer parented to its canvas.

That is no problem, we can merge this later when you have time. And thank you for always beeing so quick to respond!

Thank you for the kind words, that means a lot.

[fireclawthefox]

You're welcome and good catch on the loading of child nodes, will you create a bug report or are you trying to fix this yourself? I guess it should also just be a check during the loading somewhere.

[Augustifolia]

I think I have found a fix. Will just have to do some testing

[fireclawthefox]

Thank you so much again for all your work. This was a big help, sorry for taking so long to get to it again.

[Augustifolia]

You're welcome :)