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

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

Closed
gavbaa opened this issue Mar 1, 2017 · 30 comments

Comments

@gavbaa
Copy link

gavbaa commented Mar 1, 2017

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
Copy link

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

@ohepworthbell
Copy link
Contributor

I have also noted the issue in v1.5.1

@jmt75200
Copy link

jmt75200 commented May 1, 2017

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

But is working with v1.4.2

@cbwiedel
Copy link

I'm also running into the same issue.

@danimalia
Copy link

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
Copy link

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
Copy link

Problem for me as well

@kaltar
Copy link

kaltar commented Sep 25, 2017

This is a problem on anything that uses checkboxes.

@lbowers
Copy link

lbowers commented Sep 29, 2017

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
Copy link

ml054 commented Sep 29, 2017

@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
Copy link

ststaynov commented Jan 24, 2018

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

@ianshade
Copy link

ianshade commented Feb 5, 2018

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
Copy link

n1474335 commented Mar 1, 2018

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

@n1474335
Copy link

n1474335 commented Mar 1, 2018

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
Copy link

kaicataldo commented Apr 4, 2018

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
Copy link

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

Why?

@ststaynov
Copy link

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

@dominikfiala
Copy link

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
Copy link

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

@vankeer
Copy link

vankeer commented Jul 16, 2018

@dominikfiala @PierreGuyot This issue is fixed, see comment above: #1052 (comment)

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
Copy link
Member

owen-m1 commented Jan 5, 2019

Please check the latest version

@tsyrya
Copy link

tsyrya commented Jan 21, 2019

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

@owen-m1
Copy link
Member

owen-m1 commented Jan 21, 2019

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

@tsyrya
Copy link

tsyrya commented Jan 21, 2019

@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
Copy link

kboirel commented Feb 19, 2020

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
Copy link

Yeah I'm seeing the same issue, still.

It also breaks TinyMCE, too.

@DanPatten
Copy link

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
Copy link

Still an issue with v1.13.0

@owen-m1
Copy link
Member

owen-m1 commented Oct 10, 2021

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

@alexbezhan
Copy link

The problem still persists when using .filter option

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