Have to touch twice to dismiss "Left|right side of the balance is full" dialogs

[phet-steele]

@pixelzoom when using TOUCH input, I have to consistently touch the screen twice to dismiss the Oops! dialog when a plate is full (from #74, not the dialog for hitting max integer). This is regardless of touching the close button, the dialog, or the barrier rectangle.

Only takes one mouse click to dismiss.

Seen on macOS 10.13.4 Chrome, using the iPad emmulator in the dev tools, and iOS 11.2.5 iPad Air 2 on master at https://github.com/phetsims/equality-explorer/commit/45034868d5a491d53550ebebec59e557c0f981aa. For #91.

[pixelzoom]

@phet-steele is that also the case for the Oops! dialog when the maxInteger limit is exceeded? (Run with ?maxInteger=1 to get there quickly.)

[phet-steele]

@phet-steele is that also the case for the Oops! dialog when the maxInteger limit is exceeded?

(from #74, not the dialog for hitting max integer).

No, maybe I wasn't explicit enough in saying that dialog was fine. Sorry.

[pixelzoom]

You were plenty explicit. I skimmed too quickly (in a meeting, multi-tasking poorly).

[phet-steele]

@pixelzoom I tried disabling touchSnag on the terms and it fixed the issue for me.

[pixelzoom]

Hmm, curious. Also curious that it doesn't occur for all Oops! dialogs, since they are all identical dialogs with different message strings.

[pixelzoom]

I discussed with @phet-steele and I now understand why this occurs only for the "Left|right side of the balance is full" dialogs. This condition occurs at the start of a drag sequence, in TermDragListener start, and results in a call to interrupt. TermDragListener is a subtype of SimpleDragHandler, with allowTouchSnag: true.

Here's the relevant piece of TermDragListener start:

214         else if ( this.oppositePlate.isFull() ) {

              // opposite plate is full, cannot create inverse term, show 'Oops' message
              var thisIsLeft = ( this.termCreator.positiveLocation.x < this.equivalentTermCreator.positiveLocation.x );
              var message = thisIsLeft ? rightSideOfBalanceIsFullString : leftSideOfBalanceIsFullString;
219           var oopsDialog = new OopsDialog( message );
              oopsDialog.show();

              // interrupt this drag sequence, since we can't take term off the plate
223           this.interrupt();
              return; // generally bad form to return from the middle of a function, but this is a workaround
            }

If allowTouchSnag: true is removed from TermDragListener, then the problem goes away. So I suspect that there's something related to allowTouchSnag that is not being properly cleaned up by interrupt. The first click on the dialog is likely cleaning up something to do with allowTouchSnag, and the second click is closing the dialog.

Inspecting interrupt, it's not obvious (to me) what the problem might be, so maybe it's deeper in scenery. I'll need to enlist @jonathanolson's help on this one.

[pixelzoom]

Steps to reproduce on a touch device:

1. Start the sim with ?ea&showGrid&rows=1&columns=1 so that there's 1 cell on each plate. This makes it easy to fill a plate.
2. Go to the Variables screen
3. Drag an 'x' from the left toolbox to the left plate.
4. Drag a '-x' from the right toolbox to the right plate.
5. Turn on the lock (below the scale)
6. Try to drag the 'x' off of the left plate. You'll see the "Right side of balance is full" dialog.
7. It will require 2 clicks to close the dialog.

[jonathanolson]

Hmm, I'm getting an assertion failure (Assertion failed: lock is on, equivalentTerm expected) when trying to reproduce.

Also, I see checks for this.interrupted in your start() function. Are you expending the endDrag of the interrupt drag listener to call start?

It would be good to collaborate on this to resolve the issue, let me know when you're available?

[jonathanolson]

Also since we didn't have touch fuzzing until an hour ago, I can't be 100% sure this assertion failure I'm hitting wouldn't have happened before (in which case I'll want to investigate what is causing it Scenery-side).

[pixelzoom]

start function checks this.interrupted because it calls startOpposite (abstract, implemented by subtypes) which may interrupt the drag cycle.

I recently factored out big chunks of TermDragListener into subtypes CombineTermsDragListener and SeparateTermsDragListener, possible that I may have broken something.

Available tomorrow (5/24), catch me on Slack.

[pixelzoom]

I indeed have broken interrupt handling in TermDragListener. Fixed in the above commit. Verified that steps to reproduce this issue work, as shown in https://github.com/phetsims/equality-explorer/issues/97#issuecomment-387583148.

[pixelzoom]

Discussed with @jonathanolson via Slack. I'll try to summarize here.

This is a general issue with SimpleDragHandler, and probably with the new DragListener. With allowTouchSnag: true, touch deals with 2 events when a drag starts: 'enter' and 'down'. If we call interrupt during processing of the 'enter', then start will still be called when processing the down. Logic needs to be added that basically says "if the drag cycle was interrupted when processing the 'enter', don't bother with the 'down'".

In the specific case of TermDragListener, this is resulting in start being called twice, and two OopsDialogs being opened. And then it requires 2 clicks to close the 2 OopsDialogs.

@jonathanolson will devise a solution for SimpleDragHandler and DragListener.

[jonathanolson]

Implemented the solution for SimpleDragHandler and DragListener. Can you verify the above commit fixes the issue for this sim?

[pixelzoom]

@phet-steele Can you please verify?

[phet-steele]

Verified; I only have to touch once now.