SortableJS / Vue.Draggable

Vue drag-and-drop component based on Sortable.js
https://sortablejs.github.io/Vue.Draggable/
MIT License
20.06k stars 2.89k forks source link

Best way to use with nested elements and Vuex #701

Open range-of-motion opened 5 years ago

range-of-motion commented 5 years ago

Code Demo

Step by step scenario

  1. Open the demo
  2. Open your browser's console
  3. Re-arrange a nested element inside it's parent
  4. Nothing shows up in the console, right?
  5. Re-arrange a "root" element
  6. Now the console shows that the elements have changed, right?

Actual Solution

Cry.

Expected Solution

That re-arranging nested elements works?

David-Desmaisons commented 5 years ago

It is working check the exaample:

https://sortablejs.github.io/Vue.Draggable/#/nested-with-vmodel

https://sortablejs.github.io/Vue.Draggable/#/nested-example

range-of-motion commented 5 years ago

Right, but mine doesn't. And (as far as I know) it's an exact copy of your v-model example.

I created an isolated repo and demo for you, if you could help me figure out what the problem is I would greatly appreciate that 🙂

David-Desmaisons commented 5 years ago

@range-of-motion nested dragging are complex cases especially ehen integarted with vuex store. Unfortunatelly I don't have the time to review all the code you wrote and look for a problem. Main lines: -Use computed and v-model to commit changes when dra has ended. I would advice you to try to stick to the provide examples and make incremental changes.

range-of-motion commented 5 years ago

Thanks for the quick responses–you're being very helpful.

Vuex isn't the cause of this bug. Merely changing the order of nested elements doesn't emit the update emitter.

I think your example also has this issue I'm describing.

donnysim commented 5 years ago

Kind of anti VueX but the example persists nested changes directly to VueX store object without the need of the computed setter, the event does not reach root component with vModel to persist the changes and trigger the setter. The setter is only triggered for root changes. This kind of leave a very difficult situation on figuring out how to correctly persist the change without modifying the instance on VueX if we want to be VueX safe, where everything is changed using mutations, keeping mutation log etc..

For events, I'm not sure if this is possible to solve elegantly because of component nesting, but mostly it would need to pass the events all the way up the chain to the first child, which then would emit them to the main component, but some events need to emit specific values? For example the input event should only emit it's shallow copied current value without changing the actual prop value passed to it, where the parent then replaces it in it's shallow copied value, and then emits it back to parent and so on? resulting in always achieving a new object and thus always executing the computed setter to update the VueX store without modifying the actual object.

David-Desmaisons commented 5 years ago

@range-of-motion If you can reproduce on the example, could you detail, which example, step by step scenario and actual vs expected?

range-of-motion commented 5 years ago

@David-Desmaisons alright, so it's very simple. I forked your repository and added a console.log statement to prove my point.

You can clone it yourself and run it locally, but I created a recording for you and everyone else. Re-ordering parent elements works fine, as you can see in the console–there is an event that's being emitted. But re-ordering nested elements doesn't emit an event.

That's the bug.

nesting_bug_vue_draggable

range-of-motion commented 5 years ago

@David-Desmaisons anything else you need from me?

David-Desmaisons commented 5 years ago

With the context, I understand the question. I agree with @donnysim that the current solution is a Vuex antipatern has it mutates directly store data. My point of view: this is a Vuex design question on how to design a store data and action to represent nested data. Once the solution found, use v-model binding with draggable and use computed properties to get the list and perform the corresponding actions. At the end, utimatelly, it is not a vue,draggable issue. I guess stack-overflow would be the perfect place to discuss this kind of design and pro and cons of possible solution.

TLA020 commented 4 years ago

Did you find a solution for this problem, as it's a vue-dragable issue.

Maybe we should emit an event with the new created array, where we can trigger a action inside our vuex store to "commit" this new array to the store.

Directly changing the state is considered badpractice. (anti pattern) and will not work in strict mode.

donovanhare commented 4 years ago

Would also be interested in a solution - I don't think the example page should be promoting an example that does not work as intended.

esvit commented 4 years ago

I have faced with the same problem. Created a small component as work around for the issue. Maybe it will be useful for someone https://codepen.io/esvit/pen/abzZjax

FE-Mars commented 4 years ago

I have faced with the same problem.

agcty commented 4 years ago

I also have this problem and it's really annoying.

David-Desmaisons commented 4 years ago

@kanslooz

Directly changing the state is considered badpractice. (anti pattern) and will not work in strict mode.

I fully agree.

Did you find a solution for this problem, as it's a vue-dragable issue.

I fully disagree: there is nothing in the design of this component that results in mutating any store objects.

As a matter of fact, it only depends on the way you design your Vuex store to handle nested structure that exceeds by far the scope of this framework.

I will remove the example from the live demos and close this issue.

codewp commented 4 years ago

I have the same issue with nested Vuex store and it causes [vuex] do not mutate Vuex store state outside mutation handlers. if you add strict: true, to your store configuration that is explained here.

This issue causes the below line of Vuedraggable.

var spliceList = function spliceList(list) {
        return list.splice.apply(list, _toConsumableArray(_arguments));
};

Splicing the list causes this issue. Is it possible to use an event instead of a mutating list inside VueDraggable? or any other way?

@David-Desmaisons Please note that closing the issue will not solve the issue. Also, I should say that I used your nested example by adding strict: true to the Vuex store object to enable error reports.

Please check the attached gif. Record_2020_01_21_17_44_18_286

Thanks.

Gerbenodk commented 4 years ago

@codewp I have the same issue..

mzinsmeister commented 4 years ago

I think at least the official example should show a way to do it without directly modifying the store.

lynnerang commented 4 years ago

I also have this issue. Is it even specific to Vuex, or just an issue of updating nested components up to a first child? I’m finding that one of my emitted events to update is being cancelled out/overwritten by the other (the “add” and “remove” events triggered in the origin and destination) and one never even reaches the first child to update it whether that is then committed to the store or not. Works fine so long as I’m dragging within the same list/level, or dragging anywhere within another first child’s nested components tree (since there aren’t competing updates to the same first child).

jd0e commented 4 years ago

I also have the same issue. The issue is related to Vuedraggable, not vuex itself. Vuex is just reporting an error, when Vuedraggable is modifying the state directly. Could we have a working example for vuex, please?

elmercd commented 4 years ago

I also have this issue. My workaround for this:

TeodorDimitrov89 commented 4 years ago

@elmercd You saved my day. Big thank you for you code. I managed to implement it for my needs. Thank you

krkaa commented 4 years ago

@elmercd Thank you. It's really working implementation

embryCODE commented 3 years ago

@elmercd Thanks so much. This was several hours for me yesterday. Couldn't figure out why the input event wasn't being fired when nested arrays were rearranged. Your solution is great.

carloslema commented 3 years ago

Team, I've been trying to figure this out for about two weeks and a few sleepless nights.

How can I get the "index path" from a nested element? Something like Root.content.item[1].children.item[2]

The initial concept came from: https://sortablejs.github.io/Vue.Draggable/#/nested-example

And the VUEX integration came from another answer in here, which has been super helpful in getting my CMS project running.

this is what I am talking about:

<div
  v-for="(content, index) in realValue"
  **:key="index" <---- PARENT INDEX**
  :class="[{ active: focusStatus && sectionId == content.id && columnId == id && widgetId == index }, content.class]"
  @mouseenter="onFocus(content.id, id, index)"
  @mouseleave="offFocus(content.id, id, index)"
  :style="[sectionStyle(content.format), styleInline]"
  >
  .
  .
  .
   <nested-draggable
    :child="true"
    v-bind="dragOptions"
    v-if="content.children"
    :list="content.children"
    :class="[content.class, sectionColor(content.format)]"
    :style="sectionStyle(content.format)"
    @change="onChange"
    **:id="index" <---- NESTED INDEX**
    />

The problem shows up when trying to delete a nested node, or copy/paste a nested object (dragging a new one works fine).

Data flow: Firebase -> Vuex(state) —> Draggable/Nested Component @onChange -> Vuex(state) and then @save2FB saves the full JSON obj in Vuex(state) back to Firebase. I have all of this working without any errors.

To avoid collision, every time I drag a new element or move an existing element/nested element the node.ids get refreshed sequential node.ids — so using UUID for node.ids might not be an option—but might solve my problem, since then I can use findIndex to trace back to root.

Any ideas are welcome.

Take a look at this example: https://ryshu.github.io/jsonpath-picker/

JSON path picker

PS. I'll be trying JSONPath tomorrow, and taking a hard look at JSON path picker to see how @ryshu did it.

xs47968224 commented 3 years ago

@elmercd nice~