Repository navigation
Clarifying the signature of the onMove callback #1861
Description
Custom
The doc for options in the readme describes 3 return values for onMove corresponding to the special behaviors (false, 1 and -1): https://github.com/SortableJS/Sortable/tree/88838bfa5647e278f4cf87fdd4c3e34cd24fb33c#options
Reading the source code at
Lines 1247 to 1254 in 88838bf
onMove callback).
I opened a PR at DefinitelyTyped/DefinitelyTyped#45684 to update the type definitions to change the return type to false | 1 | -1 | void, to account for this behavior (so that the typechecker does not complain when returning nothing). but the maintainers answered saying they prefer to stick to the documented API.
Is this behavior when returning nothing an official one ? Suggested answers for preventing the move to certain positions without forcing the insertion side are relying on this, either returning nothing or returning true depending on the snippets (see #1019 and #1500 for such examples).
What should be the official signature for this callback ?
false | 1 | -1(current type definition, preventing this behavior)false | 1 | -1 | voidallowing to return nothing to keep the automatic insertion position based on the directionboolean | 1 | -1 | voidallowing to return eithertrueor nothing to keep the automatic insertion position
Based on the decision, it would be great to update the readme to reflect that in the doc (I can submit a PR once I know which variant the maintainers want)
Activity
waynevanson commentedon Jul 17, 2020
We need to update the docs to reflect that the signature should return
false | 1 | -1 | void.stof commentedon Jul 17, 2020
@waynevanson the existing types at
@types/sortablejsare documentingbooleanrather than onlyfalse, makingtruea valid return value to keep the automatic behavior. Removing support fortruethere would be a BC break for typescript users.And supporting
truealso makes it convenient to write code depending on the kind of conditions you want (as you can return a boolean expression easily).So should I go for
falseorbooleanin my PR for DefinitelyTyped ? My own vote would go for allowingbooleanhere, but I don't really have a voice in the final decision, as I'm only a user of the library, not a maintainer or an active contributor.waynevanson commentedon Jul 22, 2020
@stof that's interesting point. So returning true uses the insertion point automatically generated?
If this is the case, option three looks to be the correct return value.
stof commentedon Jul 23, 2020
In the current implementation, returning anything use the automatic insertion point, except for the 3 special values (
false,1and-1). you could be returning weird things if you want, but I don't think documentinganyas the return type makes sense (it would make it too easy to hide mistakes where just some values are special). Buttruelooks like a sensible value to accept.