PR #1452 : Gui improvement : Drags and drops in tree view.

Have some feature requests, feedback, cool stuff to share, or want to know where FreeCAD is going? This is the place.
Forum rules
Be nice to others! Read the FreeCAD code of conduct!
plgarcia
Posts: 305
Joined: Wed Jun 17, 2015 9:47 pm
Location: Near Paris (France)

PR #1452 : Gui improvement : Drags and drops in tree view.

Postby plgarcia » Sat Jun 02, 2018 12:54 pm

Hello
Continue discussion https://forum.freecadweb.org/viewtopic.php?f=8&t=26961.
triplus wrote:
Sat Jun 02, 2018 11:49 am
P.S. In addition a discussion in a separate thread would make more sense to me.
This is the new discussion.

The associated pull request is:
https://github.com/FreeCAD/FreeCAD/pull/1452

The material (Specifications, videos, test midel, screenshots) is here:
https://drive.google.com/open?id=1uvnzK ... lM36YiR2hw
The ambition of this change is to correct the behavior of the drags and drops in the tree view that introduces inconsistencies in the model, and to extend the drag and drop functions in order to be able to replace parts in the tree view by another.
triplus wrote:
Sat Jun 02, 2018 11:49 am
I took a look at description and documentation and videos and still have hard time understanding what this is about.
Just open a document, and try in the tree view to select and move parts (drag and drop) in the tree on the current version. The current version accepts <Ctrl> and <Shift> keys to control the behavior, and there are bugs (mainly moves the part instead of copying it, and introduces cyclic references) what introduces incoherence in the model. Can you make this test please?

My version adds the use of <Alt> key to replace a part by another, and corrects the bad behaviors.

Note that I will be available to correct and or amend the behavior as we will decide in this discussion.

Regards
Pascal Garcia
Last edited by plgarcia on Sun Jun 03, 2018 10:00 am, edited 3 times in total.
plgarcia
Posts: 305
Joined: Wed Jun 17, 2015 9:47 pm
Location: Near Paris (France)

Re: PR #1452 : Gui improvement : Drags and drops in tree view.

Postby plgarcia » Sat Jun 02, 2018 3:39 pm

Hello,
In the model test5.FCStd, there is a quite complex structural composition that allows to tests many cases.
Error 1 :
  • In 0.17 version selecting Sphere form cut is not possible. There fore it is not possible to include it in another composed part.
  • In my version you can select it and drop it in compound, but only copy it, as moving it would conduct the cut part to be inconsistent with no base object.
Error 2 :
  • In 0.17 version you can select Fusion004 and drop it in Fusion001 in Compound part. As Fusion001 is part of Fusion004, this introduces a cyclic reference, what is an error.
  • In my version you can drag Fusion004 but you cannot drop it in Fusion001 in Compound part or in anay composing part of Fusion001. But you can replace Fusion001 by Fusion004 in compound part, what does not introduce structural incoherence.
Error 3 :
  • In 0.17 version you can select Fusion001 from Fusion004 and drop it in Compount001. Fusion001 is removed from Fusion004 and Fusion004 is only composed of 1 element what is an error.
etc...
Here is a screen shot to explain the manpulations proposed.
CaptureErrorsDragDrop.PNG
CaptureErrorsDragDrop.PNG (35.99 KiB) Viewed 979 times
Regards
Pascal Garcia
realthunder
Posts: 1809
Joined: Tue Jan 03, 2017 10:55 am

Re: PR #1452 : Gui improvement : Drags and drops in tree view.

Postby realthunder » Sun Jun 03, 2018 11:53 pm

Hi plgarcia, could you please share your implementation details? I am a developer, and I've made heavy change to tree view in my branch. I'd like to know if your proposal is compatible with mine.
Try Assembly3 (latest version 0.11) along with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
plgarcia
Posts: 305
Joined: Wed Jun 17, 2015 9:47 pm
Location: Near Paris (France)

Re: PR #1452 : Gui improvement : Drags and drops in tree view.

Postby plgarcia » Mon Jun 04, 2018 6:37 am

Hello
Your implementation is interesting because the target part is highlighted in the main view while moving the mouse.
But as far as I tested, the drop event is not handled any longer and nothing is done when dropping, what is somehow better than braking the model.
I will give details in the next few days.
I do not understand what donate does exactly that is why I did not do it yet.
Regards
Pascal Garcia
plgarcia
Posts: 305
Joined: Wed Jun 17, 2015 9:47 pm
Location: Near Paris (France)

Re: PR #1452 : Gui improvement : Drags and drops in tree view.

Postby plgarcia » Mon Jun 04, 2018 8:57 am

Hello
I updated the document on GoogleDrive with detailed design information.
I also added the Tree.h and Tree.cpp files for review purposes. Everything is committed in the PR.
Regards
Pascal Garcia
plgarcia
Posts: 305
Joined: Wed Jun 17, 2015 9:47 pm
Location: Near Paris (France)

Re: PR #1452 : Gui improvement : Drags and drops in tree view.

Postby plgarcia » Mon Jun 04, 2018 11:55 am

realthunder wrote:
Sun Jun 03, 2018 11:53 pm
... could you please share your implementation details? ...
Is the level of details I provide in the document what you are expecting?
Regards
realthunder
Posts: 1809
Joined: Tue Jan 03, 2017 10:55 am

Re: PR #1452 : Gui improvement : Drags and drops in tree view.

Postby realthunder » Mon Jun 04, 2018 1:13 pm

plgarcia wrote:
Mon Jun 04, 2018 11:55 am
realthunder wrote:
Sun Jun 03, 2018 11:53 pm
... could you please share your implementation details? ...
Is the level of details I provide in the document what you are expecting?
Regards
Thanks for the extra detail. I have gone through your PR code. I'd say you general idea is fine. There are small issues, like, code of adding intermediate compound does not belong to the core, so it should be handled elsewhere. BTW, my creation of Link is perfect for sharing object with different boolean features.

What concerns me is that, with my introduction of Link, the tree navigation become a lot more complex. There will be a lots of rework is required, either by you or me, depending on who's change is accepted first. But hey, the fate of my change is not set yet, so obviously I am not really in the position of making suggestions.
Try Assembly3 (latest version 0.11) along with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
plgarcia
Posts: 305
Joined: Wed Jun 17, 2015 9:47 pm
Location: Near Paris (France)

Re: PR #1452 : Gui improvement : Drags and drops in tree view.

Postby plgarcia » Mon Jun 04, 2018 2:28 pm

realthunder wrote:
Mon Jun 04, 2018 1:13 pm
... There are small issues, like, code of adding intermediate compound does not belong to the core, so it should be handled elsewhere. ...
I do no understand this point.


The issues I see are:
  • Rename the canDrag and canDrop function (that refer to the gui) in canRemove and canAdd, but this would change quite a lot of class.
  • Some coherence control I make in dragMoveEvent should be moved to the part in order not to add a bit of functional things in the gui.
But my idea is to have the simplest change as possible in a first step, to have it easily accepted.
Adding a compound is based on adding a part and replacing 2 parts that are implemented elsewhere.

For conflicts, I did not see your code but I believe you made changes in dragMoveEvent.
I believe the highlight of the object can be done either at the beginning of this function or in the dropMimeData fnction that is called before the dragMoveEvent function, and keep both functionalities quite separated in the code.

@triplus : Did it became more clear whith the new explanations? Do you want me to make a video with sound to explain more?

Regards
Pascal Garcia
plgarcia
Posts: 305
Joined: Wed Jun 17, 2015 9:47 pm
Location: Near Paris (France)

Re: PR #1452 : Gui improvement : Drags and drops in tree view.

Postby plgarcia » Wed Jun 06, 2018 6:12 am

Hello
realthunder wrote:
Mon Jun 04, 2018 1:13 pm
... There are small issues, like, code of adding intermediate compound does not belong to the core, so it should be handled elsewhere. ...
Where do you think it should be implemented?

Could you please give me more informations on all issues. I will correct them when appliccable.
Regards
Pascal Garcia
realthunder
Posts: 1809
Joined: Tue Jan 03, 2017 10:55 am

Re: PR #1452 : Gui improvement : Drags and drops in tree view.

Postby realthunder » Wed Jun 06, 2018 10:54 am

plgarcia wrote:
Wed Jun 06, 2018 6:12 am
Where do you think it should be implemented?
It is best suited as a context menu action in Part workbench if you want to use compound. However, IMO a draft clone is better suited for the job. And if my changes are accepted, Link is the ultimate solution, which I do have a command implemented as context menu action, called "Replace with link".

Could you please give me more informations on all issues. I will correct them when appliccable.
Calling dropTarget->hasInInListRecursive(dragObj) is inefficient. You can obtain dragObj->outListRecursive() once at the beginning of dragging, and check if any dropTarget is in it.

In my branch, I added a new API called canDragAndDrop() to let object decide if the dropped object will be auto removed from the dragging target. My Link object interpret the dropping action as setting the link path, which obviously should not remove the object from its parent. Besides, there are lots of other complications when dragging and dropping involves context, determined by dragging object's parent, and grandparents. But that's only in my branch, and doesn't concern you at the moment.

Finally, I suggest you check your file changes in github PR page, to avoid unnecessary changes due to line-ending problem. A lot of file is marked as completely changed.
Try Assembly3 (latest version 0.11) along with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal