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

Nested dragging may cause unexpected DOM change #30

Closed
nivekz opened this issue Nov 15, 2016 · 20 comments
Closed

Nested dragging may cause unexpected DOM change #30

nivekz opened this issue Nov 15, 2016 · 20 comments

Comments

@nivekz
Copy link

nivekz commented Nov 15, 2016

Our app has two-level nested draggable items and has list defined/mapped.

When dragging an item in the inner list, sometimes the DOM may update incorrectly, and when it happens, the item being dragged will jump to the beginning of the list. The vue data, however, is correct/as expected. So it seems like the DOM is updated inconsistently w.r.t. the event, whereas the vue data is fine.

We tried to make a reproducible demo: https://jsfiddle.net/6sha4vv6/1/

Dragging Wildcard Courses x to the place of Multiple Courses x and release can cause the former to jump to the first position.

@David-Desmaisons
Copy link
Member

Thanks for reporting and providing a fiddle. I will investigate

@Muffinman
Copy link

I'm also experiencing this. Any solution / work-around?

@David-Desmaisons
Copy link
Member

@Muffinman , from quick analysis the problem arises because in case of nested component sortable.js raise an event with wrong oldindex information. See here: https://jsfiddle.net/dede89/8f0yz0m5/

@RubaXa I am opening an issue for Sortable with the simplified scenario

NB: I was not abble to identify a work-arround for this one.

@David-Desmaisons
Copy link
Member

@RubaXa It seems to be exactly the same as SortableJS/Sortable#363. Is this issue corrected in current sortbale npm package?

@Muffinman
Copy link

Muffinman commented Nov 22, 2016

Looking at the latest stable npm package (v1.4.2) It seems that evt.preventDefault() is only called in _onTapStart() if you specify options.filter when initialising Sortable.

EDIT: It only calls evt.preventDefault() if the currently tapped element should be filtered - I guess it may be possible to init the parent sortable so that the child elements are filtered and the child sortable so that the parent elements are filtered.

@Muffinman
Copy link

I couldn't get it to work with filters, but it does seem like this issue is fixed in Sortable 1.5.0-rc2 (I've tested and it works).

Hoping they roll out an npm tag soon!

@David-Desmaisons
Copy link
Member

@Muffinman I did also try to play arround with filter and draggable option without success. I will update vue.draggable as soon as Sortable package will be released

@muchacho-diesel
Copy link

@David-Desmaisons I have the same issue, this is my example code: https://jsfiddle.net/4tp3hz87/13/

I found a temporary workaround for this issue, I used the Sortable version 1.5.0-rc1 in the package.json at Vue.Sortable plugin.

@nivekz
Copy link
Author

nivekz commented Nov 25, 2016

I have a related question when I looked into the source code of Vue.Draggable for this issue. Why does Vue.Draggable seem to manipulate DOM (as opposed to just maintaining the state of Vue data and letting Sortable handle DOM exclusively)?

I made an alternative component that is very stupid but seems to work correctly with nested dragging (i.e., the issue here does not occur). The main idea (and the only logic) of my prototype is:

  mounted() {
    Sortable.create(this.$el, {
      onUpdate: (e) => {
        this.list.splice(e.newIndex, 0, this.list.splice(e.oldIndex, 1)[0]);
        this.$emit('reorder', {
          oldIndex: e.oldIndex,
          newIndex: e.newIndex,
        });
      },
    });
  },

So in other words, why does Vue.Draggable choose a heavier approach than this strawman?

@David-Desmaisons
Copy link
Member

Good question!
The approach you mention wasd the first approach of this library.

The thing is after the drop operation Sortable.js can leave the DOM in a state diferrent from what vue expect and this resulted in nasty bugs in case of use of components, v-if directives, etc...

So the current approach is to "cancel" the dom modification after the drag operation and let vue update the dom based on changes on vm.

This allows also to use vue.js animation in a drag context.

@jefflam
Copy link

jefflam commented Nov 29, 2016

@David-Desmaisons this may be a little off-topic to this thread on nested sorting having issues, but on the point of your approach of "cancelling the dom mod" after drag, how do I then acutally then detect the real 'onEnd' scenario in the DOM manipulation?

The current scenario I have is I have 2 lists wit 2 IDs and 2 items that can be dragged from one list to another.

When I drag one item from one list to another, I call the 'onEnd' event to try to detect which list ID the item is dropped on, so that I can make an API call to update the position change on the database.

However due to the DOM cancellation, during the 'onEnd' event, I end up detecting the original list ID.

Any suggestions? I can also move this to a new topic if that's easier. Thanks!

Edit: In this case, I don't see how I am able to detect the ID of the list when the user drops the item (ends the drag). I am able to detect it using the onMove event, however I am not sure if that's the base practice. What I would have to do then is to maintain the ID of the list in component state as the user drags it around, and only onEnd event do I read the component state and send it to the server.

Edit 2: I take the a previous edit back -- it seems like even on calling the onMove or onUpdate event, the DOM manipulation is cancelled hence returning the DOM state to the original state resulting in the inability for me to properly detect the state of the DOM/position of the objects after the end of the drag...

@David-Desmaisons
Copy link
Member

Hello @jefflam , I will be nice to open a new issue for tracability reason. To undestand better your scenario, could you exlain why you are needing to tack dom changes vs viewmodel changes?

@jefflam
Copy link

jefflam commented Nov 29, 2016

Will create a new issue and reply to you in that thread, thanks!

@kisglaci
Copy link

kisglaci commented Dec 12, 2016

I'm experiencing the same issue here: https://jsfiddle.net/oos7u19g/ if it helps;

@kisglaci
Copy link

Does somebody know if this bug's solved already? I've tried v2.4.0 but it's still buggy.

@David-Desmaisons
Copy link
Member

Hello @kisglaci , to solve this, vue,draggable should update to use Sortable 1.5.0-rc2. Current version is not using this release. Temporary by-pass is to use Vue.Draggble v2.4.0 with Sortable 1.5.0-rc2 (changing the depeendy by hand)

@kisglaci
Copy link

Sortable has only 1.5.0-rc1 currently, not? I've checked the package and it seems to be the case: https://github.com/RubaXa/Sortable/tree/1.5.0-rc1

@David-Desmaisons
Copy link
Member

Sortbale is, but Vue.Draggble v2.4.0 is using 1.4.2

@kisglaci
Copy link

Solved by forking the package and applying the fix. Just in case if somebody need to add it to npm you can use (using sortble 1.5.0-rc1):
"vuedraggable": "https://github.com/kisglaci/Vue.Draggable/archive/v2.4.1.tar.gz"

@David-Desmaisons
Copy link
Member

David-Desmaisons commented Dec 12, 2016

I just published vue,.draggable 2.5.0-rc-0 referencing Sortbale 1.5.0-rc1 which contains a fix for this issue,

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

No branches or pull requests

6 participants