home screen buttons don't behave correctly on touch devices

[pixelzoom]

This was discovered during dev testing of ph-scale, and reported in phetsims/ph-scale#160.

The home screen is broken in master on touch devices for all sims. When one of the screen buttons is touched, the button does not get larger, it goes immediately to the associated screen. This has been broken since at least 3/23/20, when ph-scale 1.0.0-dev.15 was published for dev testing.

Seems to be working fine on mouse devices.

Steps to reproduce:

1. Run any multi-screen sim on iPad
2. Touch any home screen button other than the button for the first screen.
3. Instead of the button getting larger, the sim will go immediately to the screen associated with the button.

This is a pre-requisite for ph-scale, which has (had?) a deadline of "dev version to client by 3/30/20".

[KatieWoe]

This is also happening in phetsims/qa#488

[chrisklus]

I figured out why this is happening - the cause of the bug was present before the HomeScreenButton refactor in #601, but it wasn't a bug then because each HomeScreenButton consisted of a small button and a large button, so the input listener behavior was slightly different between then and now.

Aside from the usual input listeners, this additional one handles the case described in the comment:

// If you touch an unselected button, it become selected. If then without lifting your finger you swipe over to the
// next button, that one becomes selected instead.
this.addInputListener( {
  over: event => {
    if ( event.pointer instanceof Touch ) {
      homeScreenModel.selectedScreenProperty.value = screen;
    }
  }
} );

On touch devices, that over listener fires first, which changes the button to be the 'selected' button. Then in the same touch, the following "normal" (?) input listener fires as well:

// if the button is already selected, then set the sim's screen to be its corresponding screen. otherwise,
// make the button selected
const buttonDown = () => {
  if ( isSelectedProperty.value ) {
    homeScreenModel.screenProperty.value = screen;
  }
  else {
    homeScreenModel.selectedScreenProperty.value = screen;
  }
};

Since the button was already selected in the same touch, it takes the first path and sets the screen away from the home screen instead of making the button bigger. I checked out some old SHAs and found that this "double hit" behavior was present then as well, but the difference is that in the previous block of code looked like this:

var buttonDown = large ?
                 function() {
                   sim.showHomeScreenProperty.value = false;
                   highlightedScreenIndexProperty.value = -1;
                 } :
                 function() {
                   sim.screenIndexProperty.value = index;
                 };

Instead of checking the value of a Property, it checks if itself is a small or large button, which are static booleans. Since both touch events go to the small button, it takes the second path instead, and the only consequence is that sim.screenIndexProperty.value = index is set twice in a row.

To fix, I think we need a different pattern for accomplishing the behavior in the first code block. I'm imagining a touch-drag listener, where homeScreenModel.selectedScreenProperty.value = screen; runs when a touch event has dragged over a button that the drag didn't start in. This would eliminate the double hit behavior. I think I could get this working, but I don't know the simplest/most desirable pattern off the top of my head. I'm reaching out on slack for help to implement.

[chrisklus]

@samreid and @jonathanolson looked at this with me and came up with a working solution. We decided to guard touchdown events to eliminate the "double hit" behavior. Thanks!

Assigning to @pixelzoom to confirm the behavior is as expected and close if so.

[pixelzoom]

I did not review the code changes. But I did confirm that it's now working correctly on touch and mouse devices. Closing.