Index off by 1 and sortupdate fired if no changes are made.

[mfeherpataky]

Firstly, let me thank you for your time putting the code up and maintaining it! The roadmap and the next release do look very promising!

For the last two days I am evaluating the project and have come to a conclusion that there is something amiss when 'sortupdate' is dispatched.

Sortupdate fires off - per comments in the code - when either parent list has changed (makes sense) and the index of the item has changed (also makes sense). Attached to the sortupdate is this super helpful detail object.

I was tipped off when 'sortupdate' was dispatched when no changes to the list was made and the results in the detail object seemed to be off-by-1.

function index (element, elementList) {
    if (!(element instanceof Element) || !(elementList instanceof NodeList || elementList instanceof HTMLCollection || elementList instanceof Array)) {
        throw new Error('You must provide an element and a list of elements.');
    }
    return Array.from(elementList).indexOf(element);
}

The index function is called few times across the code - firstly at dragstart to get the initial listItem index and then at drop. At drop it gets both element and the elementList, yet this time elementList has a placeholder sitting in it... So the listItem index will be off-by-one.

Or am I missing something and just completely missed the point?
Thanks!

[lukasoppermann]

Hey, @mfeherpataky, thanks for reporting this. I did notice it recently as well. I am planning an update to the events #361 and am hoping to solve this in there as well.

However if you want to send a quick PR that would be great. I think you probably need to add a _filter to create a list with only relevant items. Otherwise I will look into it to see if your observation allows for a quick Bugfix 👍.

[mfeherpataky]

Thanks for a quick reply @lukasoppermann.

I quickly did try the filter approach, but ended up with sortupdate being triggered only on parentList change - the index function is called in few places.

In any case in the next days I will get it working, so if there is a relatively clean fix with no unexpected side-effects I will send a PR your way. Thanks!

[mfeherpataky]

hi @lukasoppermann, as you suggested passing _filter to each _index where the elementsList may still contain placeholder after the reposition was done - gets the job done.

I would be happy to contribute to the project. However, what is the setup? I have not worked with TypeScript before, and there it no info on the expected setup in the Contribution Guide. Slight syntax change is not a problem, and comes naturally to anyone using JSDoc, yet I am having difficulty getting TS files off the ground with errors left and right when trying to build with tsconfig.json.

I assume the JS files are built from TS ones, and you wish to keep the code base in TS, right? How do I go about it?

[lukasoppermann]

Hey @mfeherpataky,
yes, the .js files are build from the ts files.

So I assume you cloned the repo and ran npm i? What errors do you get?
Typescript is actually not that different to JS.

[mfeherpataky]

@lukasoppermann all is fine - had to switch to Visual Studio as my default IDE complains to heavens about TypeScript - will look into it when I have more time.

Once last question - I did add some test and updated some, as the changes were test-breaking. Now, how do I rebuild the .js tests? The .ts ones are passing, the .js ones not.

[lukasoppermann]

Great! 👍

Concerning your question: Are you running an up to date version? No js files should be tested.