Releasing 2 chocolate bars above/below the same stack can result in improper stacking

[Nancy-Salpepi]

Test device
iPad 9th generation

Operating System
iPadOS 17.5.1

Browser
Safari

Problem description
For phetsims/qa#1105, on the Distribute Screen if I release 2 bars at the same time above or below a stack, they will stack in the same spot, leaving a space in the stack. Touching the "floating" bar will fix the problem.

Steps to reproduce

1. On the Distribute screen add at least 2 more people
2. Grab a bar with one finger and a second bar with another finger
3. Place both bar high above (or below) the same stack and release at the same time

Visuals

RPReplay_Final1720724329.mov

[jbphet]

I can duplicate this, but it requires a bit more specific sequence:

1. Grab a bar from a stack where you're not going to drop it
2. Grab a bar from a stack where you are going to drop it
3. Drop the first one (which will create a gap), then the second (which will go to the same location as the first one)

[jbphet]

I've committed a proposed fix for this issue. The problem was with the positioning that occurred in the item-added listener for the snacks on the plate, because it wasn't taking into account the possibility that a previously added snack could be in the process of being dragged. I regression tested a number of different scenarios and didn't see any problems.

This isn't a big delta in terms of the number of lines changed, but it is a different way of deciding where the candy bar should go, so should be reviewed carefully. I a little concerned that there was some history for why it was being done this way before.

Assigning to @marlitas for review.

[marlitas]

The only scenario I can think of that could potentially be buggy is if this listener fires and another candy bar has it's draggingProperty set to false, has been added to the observable array, but it's own listener has not fired yet. I'm not even sure that would be possible... Both candy bars would have to come from different plates, be dropped on the same plate at the exact same time, and the observable array listeners would need to get scrambled up in a manner that I don't believe javascript allows.

I don't have an easy way to test this scenario... but I also don't think it's possible. I'll send to @jbphet to confirm, but otherwise I think this change looks good and seems robust.

[marlitas]

Apparently my brain was quite fuzzy when I was looking at this. This creates problems in the state wrapper right away. I'm going to revert the above commit. @jbphet and I should check in.

[jbphet]

Mea culpa for not checking the state wrapper - I always forget that. I've redone the change so that it will handle the phet-io-instigated situation where the add-item-listener is being fired after several things have been added, and not as each is added. I've tested it locally for the typical mouse-driven usage, the problematic multi-touch cases, and in the state wrapper, and it seems to be working correctly. I think it's ready for review again.

[marlitas]

This is looking good. I went through the code, did some testing and this seems like a strong way to address the problem. Thanks for your work on this @jbphet! I'll send this over to @Nancy-Salpepi to confirm all looks well on main. Feel free to close.

[Nancy-Salpepi]

Looks great on main! Closing.