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

Update for Vue 2 #17

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Update for Vue 2 #17

wants to merge 13 commits into from

Conversation

MunGell
Copy link

@MunGell MunGell commented Oct 17, 2016

Hi Jeff,

I've taken the liberty of updating your code to work with Vue v2.
This PR is basically an opinionated version of #13 with fixed tests and dependencies.

I wouldn't expect this to be merged, but may be it will be useful for you when you will be updating this module to v2.

@irelandpaul
Copy link

I pulled this request and it seems to be working on Vue 2. Nice work @MunGell

@d13r
Copy link

d13r commented Nov 3, 2016

Thanks for this @MunGell, it works well.

To get it working correctly I also had to add v-bind:key to my template:

<tr v-for="(action, index) in actions" v-bind:key="action.id">
    ...
</tr>

Otherwise Vue isn't aware of the HTML change and it gets out of sync with the model.

It is mentioned in the Vue 2 docs, but took me a while to find it, so it may be helpful to add it to the Vue Sortable docs / examples too.

@kartsims
Copy link

kartsims commented Nov 4, 2016

Works great for me too, will use your version until this PR is merged, thanks @MunGell 🙌

@crabmusket
Copy link

Easy mode - add this to package.json:

    "vue-sortable": "git://github.com/MunGell/vue-sortable#77c26994",

@wlx200510
Copy link

It works well , I advice you add the v-bind need for arr sort sync, which is better for using.

@rap2hpoutre
Copy link

Is there a reason why it should not be merged?

@suth
Copy link

suth commented Dec 8, 2016

Thanks @MunGell, only issue I've seen is that the Sortable instance is no longer available on the VM

@MunGell
Copy link
Author

MunGell commented Dec 8, 2016

@suth yes, there is no vm variable in vue.js v2.
I didn't find a way to keep this variable accessible, if you have any suggestions - they are very welcome.

@suth
Copy link

suth commented Dec 8, 2016

I haven't really worked with custom directives, but this seems to work:

inserted: function (el, binding, vnode) {
    var sortable = new Sortable(el, binding.value || {});

    if (binding.arg) {
        if (!vnode.context.sortable) {
            vnode.context.sortable = {}
        }

        //  Throw an error if the given ID is not unique
        if (vnode.context.sortable[binding.arg]) {
            console.warn('[vue-sortable] cannot set already defined sortable id: \'' + binding.arg + '\'')
        } else {
            vnode.context.sortable[binding.arg] = sortable
        }
    }
}

@MunGell
Copy link
Author

MunGell commented Dec 9, 2016

Hi @suth

Thank you for taking time for the research!
I will take a look at your solution and will try to incorporate it into this PR

Thanks

@garygreen
Copy link

garygreen commented Jan 23, 2017

This doesn't seem to work for me - although the items are sortable, the actual order of the array inside VM doesn't get changed. Any ideas @davejamesmiller @MunGell ?

Vue.version
"2.1.10"

@suth
Copy link

suth commented Jan 26, 2017

@garygreen listen for the onUpdate event and modify the array. Not sure if it's the best method but how I'm doing it for now.

onUpdateCallback(evt) {
  this.items.splice(evt.newIndex, 0, this.items.splice(evt.oldIndex, 1)[0])
}

@bradlis7 bradlis7 mentioned this pull request Feb 18, 2017
@ctf0
Copy link

ctf0 commented Jul 24, 2017

@MunGell why not re-release the package under a different name ex.vue-sortable-2 or something and now we can support both vuejs versions.

@MunGell
Copy link
Author

MunGell commented Jul 24, 2017

Hi @ctf0

How would it be different from https://github.com/SortableJS/Vue.Draggable ?

@ctf0
Copy link

ctf0 commented Jul 24, 2017

it uses a directive which is much easier to incorporate into any project without much work, ex.

<ul v-sortable="{ onUpdate: onUpdate }">
    <li v-for="item in list" :key="item.id">
        <p>{{item.title}}</p>
        <button class="delete" @click.prevent="deleteItem(item)"></button>
    </li>
</ul>

thats all i had to do beside making an ajax to get the list & remove an item, can Vue.Draggable be that easy ? specially that it pretty much does the same.

@anteriovieira
Copy link

@sagalbot any progress for this pull request?

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

Successfully merging this pull request may close these issues.