primefaces / primeng

The Most Complete Angular UI Component Library
https://primeng.org
Other
10.29k stars 4.56k forks source link

Tree dropNdrop incorrect data in onNodeDrop event object #9502

Open Halynsky opened 3 years ago

Halynsky commented 3 years ago

I'm submitting a ... (check one with "x")

[x] bug report => Search github for a similar issue or PR before submitting

Previous issue. I am reopening a new one because this one was closed. https://github.com/primefaces/primeng/issues/5002

Plunkr Case (Bug Reports)

https://primeng-treedragdrop-demo-ptht6y.stackblitz.io

Current behavior

Wrong "dropNode" in the event object of onNodeDrop function

*Also, weird thing that sometimes I get "dropIndex" object in the event and in other cases its name just "index" Its seems like when you dropping on a folder you get "index" key and when you dropping between other files you get "dropIndex"

Expected behavior Always get parent node of the target place in dropNode object Documentation of the difference between index and dropIndex or fix its behavior

Minimal reproduction of the problem with instructions Case A

  1. Move the file inside the folder into another place between files in the current folder
  2. Check dropNdrop in the drop event object

Case B

  1. Move the file inside another folder and drop it between files
  2. Check dropNdrop in the drop event object

Please tell us about your environment:

Please don't close this issue this time without deep investigation. It really prevents implementing some dragging functionality in the scope of server data, because you cant correctly track the new position of the node.

joelcoxokc commented 3 years ago

@yigitfindikli Any update on this? This is a critical one for us, As we are unable to proceed into production with this bug.

Seems like if I drop a node on another node... The event.dropNode is the correct new parent... with dropIndex of 0

However, If I expand node A. Then drag a node that is currently not a child of A into a specific index of A Then event.dropNode is a new sibling and not the parent. Nor is the dropIndex correct. If I drop a node at index 0 then the drop index is 0. But is if drop a node at any other index. It is index 1.

PLEASE UPDATE!!!! THIS IS CRITICAL to the functionality of the tree. In my personal opinion, this makes the tree component entirely unusable, as we cannot accurately update data.

joelcoxokc commented 3 years ago

Currently, the only workaround is to do a DIFF check against my raw data and the value of the tree. This is a CRUSH on performance because I have to recursively iterate my entire data collection on every drop event.

joelcoxokc commented 3 years ago

UPDATE:

In the process of navigating a workaround, I found that the only way to even get a diff check to work is if I do the following.

I have to set validateDrop to true For no reason Then onNodeDrop I have to call event.accept() & tree.updateSerializedValue()

This will update each treeNode with the appropriate children. However, it does not update each child's parent property

So I have to iterate through every treeNode and set its parent property manually.

NOTE: This is a much easier workaround if you keep a flat array in your personal code to iterate through

Then I can do my diff check.

hannesthor commented 3 years ago

I am upvoting this.

VictoriaKalakustava commented 2 years ago

Upvoting I found a workaround for my case. I am need to restrict to create parent node. I added the following code and it works for me. But it would be nice if the bug will be fixed because seems it is not assumed to use originalEvent. But maybe it will help other dev

onDrop(event: any) {
  if (event.originalEvent.target.childNodes.length === 0) {
    event.accept();
  }
}
aymennsalhi commented 2 years ago

even it's a little late, but I just find a way to accept the dropevent or not:

first of all try to add an attribute "level" to your node data, so that your tree for example have 3 nodes level, all nodes of 1st level will have level: 1, second levels nodes will have level: 2 ....

try to do this :

onDrop(e) { console.log(e); } in here you will find that e --> originalEvent -> path[0] will have a html element (span or li) anyway, just focus on the classList

if you drop your node obove an other node so you will see "ng-star-inserted" class, but if you drop it in the droppoint (under each node) you will have a classList of ("p-treenode-droppoint" "ng-star-inserted" "p-treenode-droppoint-active")

so here you can make control like that and I hope it will resolve your issues:

onDrop(e) { if(e.dragNode.data.level === e.dropNode.data.level && (e.originalEvent.path[0].classList.value.toString()).includes('p-treenode-droppoint')) { e.accept(); this.newUpdates = true; } else { console.log('no wayyy!'); } }

francoisduchemin commented 2 years ago

I'm using the version 14.0.2 and the dropNode is incorrect unfortunately... Does anyone have a workaround that does not involve a diff of the whole tree?

joelcoxokc commented 2 years ago

I'm using the version 14.0.2 and the dropNode is incorrect unfortunately...

Does anyone have a workaround that does not involve a diff of the whole tree?

Unfortunately no. Our team just finished migrating away from Prime NG over this issue. The fact that it has been such an obvious issue without any comments from the contributors. This single issue renders the entire component unusable when trying to save the sort order to a database.

Our team doesn't have the time to deal with unmanaged and over looked open source libraries. I suggest you do the same.

francoisduchemin commented 2 years ago

Thanks for your answer! May I ask which tree library you are using instead?

mertsincan commented 1 year ago

Hi,

So sorry for the delayed response! Improvements have been made to many components recently, both in terms of performance and enhancement. Therefore, this improvement may have been developed in another issue ticket without realizing it. You can check this in the documentation. If there is no improvement on this, can you reopen the issue so we can include it in our roadmap? Please don't forget to add your feedback as a comment after reopening the issue. These will be taken into account by us and will contribute to the development of this feature. Thanks a lot for your understanding!

Best Regards,

Halynsky commented 1 year ago

It was already reopened once... So you guys again closing it without any fix or review?

joelcoxokc commented 1 year ago

Ok Bug should still be active

Here is a video demoing my issues https://www.loom.com/share/9d4d6a6bff9e4094ac1b9926f362e3f6

joelcoxokc commented 1 year ago

Ive updated the comment with a video above with a shorter video... And re-pasting it here https://www.loom.com/share/9d4d6a6bff9e4094ac1b9926f362e3f6

joelcoxokc commented 1 year ago

Another update.

Here is a video to describe what I am experiencing.

Also here is my patched code.

Our team will decide in January whether of not to continue to use Primeng over the bug that has been here for 2 years.

https://www.loom.com/share/c3bbdad4528842c4bbe36b5fe9d0706f?from_recorder=1&focus_title=1

import { Tree } from 'primeng/tree';
@Component({})
export class ExampleComponent  {
    @ViewChild(Tree)
    public tree: Tree;

    public onNodeDrop(event: { dragNode: TeamNode; dropNode: TeamNode; accept: () => void }): void {
            event.accept();
            this.tree.updateSerializedValue();

            let parentId: string | null = null;

            if (event.dropNode.children?.includes(event.dragNode)) {
                parentId = event.dropNode.data.id;
            } else if (event.dragNode.parent && event.dragNode.parent.children?.includes(event.dragNode)) {
                parentId = event.dragNode.parent.data.id;
            }

            // Do something with parent
    }
}
xtt13 commented 1 year ago

Are there any news regarding the issue? It's open way too long. @mertsincan

shailee-m-argus commented 1 year ago

@mertsincan any updates on this? it is a blocker for us

joelcoxokc commented 1 year ago

@xtt13 @shailee-m-argus

My patch code on my comment above is working for us in production.

I personally recommend finding a different UI library. This has been open for 3 years with no comments from prime.

michelebenolli commented 1 year ago

Any news on this?

joelcoxokc commented 1 year ago

Any news on this?

Prime team doesn't respond to issues anymore.

Your best bet is to watch the second video I posted and check out the code I posted. We have that code running in production.

Also get off prime asap.

xtt13 commented 1 year ago

Joel's fix helped me out on this issue. Its a workaround but ok for the moment.

I am also pretty frustrated why prime team didn't respond anymore..

nssoft-rlagrange commented 1 year ago

I'm using the version 14.0.2 and the dropNode is incorrect unfortunately... Does anyone have a workaround that does not involve a diff of the whole tree?

Unfortunately no. Our team just finished migrating away from Prime NG over this issue. The fact that it has been such an obvious issue without any comments from the contributors. This single issue renders the entire component unusable when trying to save the sort order to a database.

Our team doesn't have the time to deal with unmanaged and over looked open source libraries. I suggest you do the same.

Hi @joelcoxokc , with what library did you swap primeng for the tree component ? I have the same issue here and I'm considering switching too.

meriturva commented 2 weeks ago

We are in 2024 and we have the same issue. Any news?