Possible to snag draggables while ComboBox is open

[jessegreenberg]

Found in phetsims/molarity#214, and also confirmed in beers-law-lab. Steps:

1. Use touch to open the solute menu
2. Drag finger along options
3. Without lifting up, move finger over a slider to grab it
4. Move finger back to menu and lift up to make a selection

Fixing this would likely mean changing the ComboBox listeners to be attached to a Pointer so that touch snag on other draggables is disabled.

@ariel-phet is this something that needs to be addressed? @pixelzoom may also have thoughts since he recently did lots of work on ComboBox.

[pixelzoom]

You'll need to get @jonathanolson's help on this one. I don't understand what's going on here. And is this a general problem? Can I snag something else while interacting with something?

[jessegreenberg]

Here is a diff with the change I was thinking of, minimally tested

diff --git a/js/ComboBox.js b/js/ComboBox.js
index 4df01af5..381fd7b2 100644
--- a/js/ComboBox.js
+++ b/js/ComboBox.js
@@ -52,7 +52,7 @@ define( require => {
       // See https://github.com/phetsims/sun/issues/542
       assert && assert( listParent.maxWidth === null,
         'ComboBox is responsible for scaling listBox. Setting maxWidth for listParent may result in buggy behavior.' );
-
+
       options = merge( {

         align: 'left', // see ALIGN_VALUES
diff --git a/js/ComboBoxListItemNode.js b/js/ComboBoxListItemNode.js
index a6d19625..32874d91 100644
--- a/js/ComboBoxListItemNode.js
+++ b/js/ComboBoxListItemNode.js
@@ -93,6 +93,14 @@ define( require => {
       // a11y focus highlight is fitted to this Node's bounds, so that it doesn't overlap other items in the list box
       this.focusHighlight = Shape.bounds( this.localBounds );

+      // "Attach" this listener to the Pointer on down event so that other things outside the ComboBox cannot be
+      // picked up with touch snag if down pointer remains down outside the ComboBoxListBox
+      const pointerListener = {
+        up( event ) {
+          event.pointer.removeInputListener( pointerListener );
+        }
+      };
+
       // Show highlight when pointer is over this item.
       // Change fill instead of visibility so that we don't end up with vertical pointer gaps in the list
       this.addInputListener( {
@@ -100,7 +108,11 @@ define( require => {
         focus() { highlightRectangle.fill = options.highlightFill; },

         exit() { highlightRectangle.fill = null; },
-        blur() { highlightRectangle.fill = null; }
+        blur() { highlightRectangle.fill = null; },
+
+        down( event ) {
+          event.pointer.addInputListener( pointerListener, true );
+        }
       } );
     }
   }
diff --git a/js/SliderThumb.js b/js/SliderThumb.js
index d5fb1288..a4641d12 100644
--- a/js/SliderThumb.js
+++ b/js/SliderThumb.js
@@ -71,7 +71,9 @@ define( require => {
     // highlight thumb on pointer over
     this.addInputListener( new ButtonListener( {
       over: function( event ) {
-        self.fill = options.fillHighlighted;
+        if ( !event.pointer.isAttached() ) {
+          self.fill = options.fillHighlighted;
+        }
       },
       up: function( event ) {
         self.fill = options.fill;

But even better, there is a TODO in ComboboxListBox

//TODO sun#462 replace fireEmitter and selectionListener with a standard scenery listener

This should also resolve the problem because PressListener/FireListener also attach to the Pointer. #462

[ariel-phet]

@jessegreenberg in my opinion, this behavior could be left as is.

My reason is that to cause the behavior you have to interact in a fairly nonstandard manner. I don't think most people are going to press the ComboBox and keep pointer down with touch. The natural interaction is to tap to open, then tap on the selection (I didn't even know you could keep pointer down).

If it is easy to fix with no major consequences, feel free, so I will leave it to your discretion

[jessegreenberg]

OK thanks. This is going to be resolved naturally by #462, so I will close this issue and we can continue there. I made a note there linking this issue to that one.

But understood it is not a very high priority, and we can proceed with publishing Molarity without addressing this issue.