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

Dragged element duplication #4

Open
mgthomas99 opened this issue Aug 30, 2017 · 26 comments
Open

Dragged element duplication #4

mgthomas99 opened this issue Aug 30, 2017 · 26 comments
Labels

Comments

@mgthomas99
Copy link

mgthomas99 commented Aug 30, 2017

I would like to report a bug.

VDDL Version: 0.5.2
Vue Version: 2.3.3

When I drag-and-drop <vddl-draggable> elements within a <vddl-list> too quickly while there is text selected on the webpage, the dragged element sometimes clones itself and replaces a different element in the list.

I have created an example project here, although the bug can occur in any project.

How to reproduce:

  1. Create a <vddl-list> and fill it with an arbitrary number of <vddl-draggable> items.
  2. Press Ctrl+A to select all text on the page. (I have noticed that the bug is more likely to happen if there is a lot of text selected).
  3. Repeatedly drag-and-drop the same item from the list as quickly as possible. It will eventually clone itself by replacing another element in the list (typically the element closest to where you dropped the item).

EDIT: I have moved the link to MediaFire so my Github projects list is not cluttered.

@baixiyang
Copy link

how to resolve this problem

@baixiyang
Copy link

我从上往下拖不会有问题,但是从下往上拖就会有问题。而且这个问题有时候会发生有时候突然又正常了。。。。

@hejianxian
Copy link
Owner

@baixiyang 谢谢关注!

我观察过其他人写的基于H5的拖动库,大部分都不会出现这种问题,但是我暂时还没找到解决方法。而且最近一直在加班,没太多时间去处理这个问题,实在抱歉。

如果你有任何解决思路,请告诉我一下,谢谢!

@awulkan
Copy link
Contributor

awulkan commented Nov 17, 2017

My boss was able to find a way to recreate the bug (Please don't mind the awful design, it's only temporary).
https://imgur.com/a/jR61z

In that GIF I am dragging an inner container and dropping it at the top of the outer container. By doing so it replaces the item that was previously placed first in the outer container.

I haven't tried recreating it like this outside of our project yet though.

Update: Here's another recreational version:
https://imgur.com/a/fkAx4

It doesn't happen when I drag items downwards though, only when I drag them up. It also happens when I drag them into the first position I think.

@awulkan
Copy link
Contributor

awulkan commented Nov 17, 2017

Hey again. I decided to try it out with your example code for VDDL nested lists, and I got some weird results...

In this GIF I first manage to recreate the bug in my own local dev environment, running your example code (I haven't changed anything). But when I switch over to your website and do the same drag and drop actions, it works.
https://imgur.com/a/DPvcX

I have tried this over and over many times now and I randomly get the bug in the dev environment, but never on your website (http://hejx.space/vddl-demo/#/nested).

Update: It gets weirder.
So I built the project and ran it from the dist folder with http-server (https://www.npmjs.com/package/http-server) and I could get the bug to trigger ONLY ONCE after I opened a new browser tab and navigated to the localhost page. If I then hit F5 to refresh the page, the bug dissapeared and could not be recreated. So it only showed up on the first visit to the page.

I will take a look at the code tomorrow I think, but so far I can't put my finger on what's causing it.

@awulkan
Copy link
Contributor

awulkan commented Nov 18, 2017

I have made some progress.

I believe it's this line that is removing the wrong item from the list:

this.wrapper.splice(this.index, 1);

I did some logging like this: https://imgur.com/a/2j9KW

I start with a list like this (which you find in "container 1" here http://hejx.space/vddl-demo/#/nested):

0: {type: "item", id: "1"}
1: {type: "item", id: "2"}
2: {type: "item", id: "3"}

And then drag item 3 up between item 1 and 2, I get this logged to the console:

wrapper.splice: 2

wrapper before:
0: {type: "item", id: "1"}
1: {type: "item", id: "3"}
2: {type: "item", id: "2"}
3: {type: "item", id: "3"}

wrapper after:
0: {type: "item", id: "1"}
1: {type: "item", id: "3"}
2: {type: "item", id: "3"}

This indicates that we are splicing index 2 from the list, which is 2: {type: "item", id: "2"}.

The reason why it works when dragging down, but not up is because if I drag down, then an item will be added to the list below the current item's index, which means that this.index will still be valid.

Example:

0: {type: "item", id: "1"}
1: {type: "item", id: "2"}
2: {type: "item", id: "3"}

Logged after dragging down item 1 down between item 2 and 3:

wrapper.splice: 0

wrapper before:
0: {type: "item", id: "1"}
1: {type: "item", id: "2"}
2: {type: "item", id: "1"}
3: {type: "item", id: "3"}

wrapper after:
0: {type: "item", id: "1"}
1: {type: "item", id: "2"}
2: {type: "item", id: "3"}

In this case the index to remove is 0, meaning 0: {type: "item", id: "1"}, which is correct.

However if I drag an item up, then the array will grow by 1 item (the duplicate), which pushes the start point of the drag event down one index, which causes it to delete the wrong item.

For example if I change the code to: this.wrapper.splice(this.index + 1, 1); I can now drag upwards without problem! However, now the bug appears when I drag downwards instead.

@hejianxian What do you think? Got any ideas?

@gjactat
Copy link

gjactat commented Nov 18, 2017

Hello,
The bug seems to be related to the version of Vue. I've built a small project on CodeSandbox with the exact code of your "horizontal" demo (https://codesandbox.io/s/j387n4j3y3). Note that CodeSandbox makes it easy to change the version of packages dependencies, thus comparing the ok version with the ko version.
The bug occurs with a dependency of Vue 2.5.0 but doesn't occur with Vue 2.4.2 (which is the version of your Gihtub package.json project).

For the record, the bug occurs when you drop an item on a previous index. The item is then duplicated on both the source and target of the drag'n'drop action, instead of swapped. The target item is then lost.

This doesn't solve the problem but it gives us a hint. Something has probably changed since version 2.4.2 that broke your component logic. Maybe related to reactivity or handlers execution order ?

VueJS changelog
Edit : To narrow down things, I've notices that it breaks when switching from Vue 2.4.4 to Vue 2.5.0. Nothing suprising here since version 2.5.0 changes a lot of things.

Great project by the way !

@awulkan
Copy link
Contributor

awulkan commented Nov 19, 2017

@gjactat I tested it a bit and it appears that 2.5.0 is the breakpoint for when things started go wrong.

@gjactat
Copy link

gjactat commented Nov 19, 2017

Yes... Version 2.5.0 brings major changes !

@awulkan
Copy link
Contributor

awulkan commented Nov 19, 2017

Also, if you look at my comment I explain where the wrong element is being removed from the list. Maybe I can install Vue 2.4.4 and log the same stuff to see the difference.

@gjactat
Copy link

gjactat commented Nov 19, 2017

If you can downgrade to Vue 2.4.4, you can fix your issue.
I personnaly can't downgrade :-/

@awulkan
Copy link
Contributor

awulkan commented Nov 19, 2017

I am just running the example project that you can find in this repo when I'm testing now.

@gjactat
Copy link

gjactat commented Nov 19, 2017

I need to kick a new sandbox with vddl sources instead of package dependency to investigate deeper ( breakpoints and traces....). We're getting closer !

@awulkan
Copy link
Contributor

awulkan commented Nov 19, 2017

Ok, so I have installed the older version now and I can see that the correct index is set to this.index when logging.

Starting list:

0: {type: "item", id: "1"}
1: {type: "item", id: "2"}
2: {type: "item", id: "3"}

And then drag item 3 up between item 1 and 2, I get this logged to the console:

wrapper.splice: 3

wrapper before:
0: {type: "item", id: "1"}
1: {type: "item", id: "3"}
2: {type: "item", id: "2"}
3: {type: "item", id: "3"}

wrapper after:
0: {type: "item", id: "1"}
1: {type: "item", id: "3"}
2: {type: "item", id: "2"}

Notice how this.index is 3 this time.

Logging in this file:

this.wrapper.splice(this.index, 1);

I got to sleep now. It's 1:50 here... Good luck.

@awulkan
Copy link
Contributor

awulkan commented Nov 19, 2017

I noticed that the Vuex example still suffers from this problem.
http://hejx.space/vddl-demo/#/vuex

Since it adds and deletes items through actions it wasn't solved by my pull request, but the solution should probably be similar now that we know the causation.

@baixiyang
Copy link

i think the reason is vue has not update the index after drop(when drop up ,one item insert before.vue should update the index to index +1,but not)
this.$nextTick(function () {
this$1.wrapper.splice(this$1.wrapper.indexOf(this.draggable), 1);
});
so just find the true index instead of this.index

@baixiyang
Copy link

this way cannot suit string and number list @hejianxian ,we need stricter way to get the true index。

@awulkan
Copy link
Contributor

awulkan commented Nov 20, 2017

Yes, the index is passed before the list is updated for the Vuex example, we need a way to make sure that the real index is accessible.

@hejianxian
Copy link
Owner

@awulkan I fixed it.

But I think the more serious problem is: press Ctrl+A to select all text on the page, and then, how to stop the text from being dragged while dragging.

@awulkan
Copy link
Contributor

awulkan commented Nov 20, 2017

@hejianxian Nice!

Are you talking about the nested list when it's disabled? Well, I know that you can add user-select: none; in the css on the vddl-list to prevent selection of text. It worked for me in the project I'm working on.

@hejianxian
Copy link
Owner

😭 Ctrl+A can select all text on the page.

when dragging element:

🐽 This is a big bug.

@baixiyang
Copy link

duplication element is the biggest bug i think. it will happen certainly, when drag up ward. if the bug exit , no one will use this component at all。

@awulkan
Copy link
Contributor

awulkan commented Nov 21, 2017

@baixiyang The duplication bug should be solved. Where are you experiencing it? Have you updated to the latest version?

@baixiyang
Copy link

change line 'this.splice(this.index,1)' to 'this.splice(this.wrapper.indexOf(this.draggable), 1)' is useful in my environment. this is just an adivse.

@arnoson
Copy link

arnoson commented Feb 6, 2018

I had a case where I had to access the final index (without the duplicated item in the list) inside the drop callback of a vddl-list and have a little workaround (although I think there might be a more elegant solution, maybe there is another callback for my problem?)

handleDrop(data) {
	let { index, list, item } = data
	list.splice(index, 0, item)

	const hasDuplicate = list.filter(el => el.id === item.id).length === 2
	const oldIndex = list.findIndex(el => el.id === item.id)

	if (hasDuplicate && oldIndex < index)
		index -= 1
	}

	// do something with the final index
},	

@huangqiang8438
Copy link

出现这个的问题是判断拖拽是缺少一个判断,判断当前拖拽的是向上移动还是向下移动

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

No branches or pull requests

7 participants