SortableJS / Sortable

Reorderable drag-and-drop lists for modern browsers and touch devices. No jQuery or framework required.
https://sortablejs.github.io/Sortable/
MIT License
29.29k stars 3.69k forks source link

Sorting a list with checked checkboxes causes intentionally unchecked boxes to revert to checked #1052

Closed gavbaa closed 5 years ago

gavbaa commented 7 years ago

Example: http://jsbin.com/belacoreki/edit?html,js,output

Reproduction steps: 1) Uncheck "ID" 2) Drag the sort handle anywhere in the list.

What should happen: List is reordered with checkboxes maintaining their current state (ID unchecked)

What happens: List is reordered with the ID checkbox being re-checked.

Verified issue against master and 1.5.1 release. Regression from 1.4.2, verified does not have this issue (you can change the CDN link to https://cdnjs.cloudflare.com/ajax/libs/Sortable/1.4.2/Sortable.js to verify).

LeventeHudak commented 7 years ago

Just stumbled upon the same issue and can also validate it was working in v1.4.2.

ohepworthbell commented 7 years ago

I have also noted the issue in v1.5.1

jmt75200 commented 7 years ago

Still an issue with v1.5.1 http://jsbin.com/dukiqafeto/edit?html,js,output

But is working with v1.4.2

cbwiedel commented 7 years ago

I'm also running into the same issue.

danimalia commented 7 years ago

I'm seeing this too, using sortablejs@1.6.0 and angular-sortablejs@2.0.6. It only happens if you specify a drag handle, fwiw. I just reverted to sortablejs@1.4.2 and the issue was resolved.

gwenael74 commented 7 years ago

To solve this bug, i put in comment the line "el.checked=true" in the function "_nulling": savedInputChecked.forEach(function (el) { //el.checked = true; });

thetuningspoon commented 7 years ago

Problem for me as well

kaltar commented 6 years ago

This is a problem on anything that uses checkboxes.

lbowers commented 6 years ago

I'm experiencing this too. Took me some time to realise Sortable was the cause of the problem.

Clicking on the handle (or any control within the handle) causes any toggled checkboxes to revert to their checked state. Any quick fix, without changing the source code, until this is fixed?

ml054 commented 6 years ago

@lbowers for quick fix just use version 1.4.2 unless you need some of the features available in latest (buggy version). It helped in my case. Maybe it isn't best solution but works.

ststaynov commented 6 years ago

@RubaXa Can this code be removed from master since it causes this bug?

ianshade commented 6 years ago

To confirm it is related to handle and filter options Here is a bin without handle set and checkboxes work as expected: http://jsbin.com/vuxilaqiwi/edit?html,js,output Here with filters (checkbox in a filter) and re-checking again occurs: http://jsbin.com/rimehumawo/1/edit?html,js,output

Has anybody investigated if the proposed solution - removing el.checked = true; - is a valid solution and will not mess with anything or if there is any other solution that could make into PR?

n1474335 commented 6 years ago

Any update on this? It's causing a lot of problems in CyberChef.

n1474335 commented 6 years ago

It looks like this has been fixed here: https://github.com/RubaXa/Sortable/blob/master/Sortable.js#L1503

However the package version has not been updated so this fix will not pull through to any projects building from npm. Could @RubaXa increment the patch version please?

FYI, if you want a better fix that does not forget checked inputs on elements that are not currently being dragged, this might be a better solution:

    function _saveInputCheckedState(root) {
-       savedInputChecked.length = 0;
-
        var inputs = root.getElementsByTagName('input');
        var idx = inputs.length;

        while (idx--) {
            var el = inputs[idx];
-           el.checked && savedInputChecked.push(el);
+           var i = savedInputChecked.indexOf(el);
+           if (el.checked && i < 0) {
+               // el checked and not already in array
+               savedInputChecked.push(el);
+           } else if (el.checked === false && i >= 0) {
+               // el unchecked but in array
+               savedInputChecked.splice(i, 1);
+           }
        }
    }
kaicataldo commented 6 years ago

First off, thanks to the maintainers of this project! I've confirmed the current master branch fixes this for me. Any chance this will be released any time soon?

CaelanStewart commented 6 years ago

It's been a whole year since this was reported and a fix still hasn't made its way into a release.

Why?

ststaynov commented 6 years ago

@RubaXa any chance this could be released anytime soon? Thanks.

dominikfiala commented 6 years ago

Just stumbled upon this issue and felt the need to warn you guys: this is a deal breaker. Had to switch from vue-draggable that depends on this package to MooTools Sortables. Yep. That MooTools that is depracated and old like dinosaurs. But it works.

Tip: Do not use handler option and experiment with pointer-events: none; or onmousedown(event => event.preventDefault()) to disable drag on other elements except the handle to simulate the handle behavior.

@RubaXa Would you please consider putting someone else in charge of this repo?

PierreGuyot commented 6 years ago

Same problem as dominikfiala here, is there a chance for a fix to be released any soon?

vankeer commented 6 years ago

@dominikfiala @PierreGuyot This issue is fixed, see comment above: https://github.com/RubaXa/Sortable/issues/1052#issuecomment-369613072

Someone just needs to take over maintaining this library and create a new release. For our project, we just forked this repo and created a private release with the fix

owen-m1 commented 5 years ago

Please check the latest version

tsyrya commented 5 years ago

@owen-m1 the problem is still actual, at least for me. My current version is 1.8.1 (the latest).

owen-m1 commented 5 years ago

@tsyrya It works for me. Please reproduce in a JSBin.

tsyrya commented 5 years ago

@owen-m1 I am sorry, seems like that was my fault (I didn't update the package correctly) and seems like it is working now.

kboirel commented 4 years ago

The problem stills occur to me in the latest version. If I specify a handle, the checkbox is not working at all when I click on the handle. What should I do?

CaelanStewart commented 4 years ago

Yeah I'm seeing the same issue, still.

It also breaks TinyMCE, too.

DanPatten commented 4 years ago

I am modifying the checkboxes on the start event and on end the checkboxes are getting restored. Can we make this fix optional please?

leftright1 commented 2 years ago

Still an issue with v1.13.0

owen-m1 commented 2 years ago

@leftright1 its working for me, could you provide an example?

alexbezhan commented 1 year ago

The problem still persists when using .filter option