Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[iOS] function pickers are not working #29

Closed
pixelzoom opened this issue May 11, 2016 · 14 comments
Closed

[iOS] function pickers are not working #29

pixelzoom opened this issue May 11, 2016 · 14 comments

Comments

@pixelzoom
Copy link
Contributor

Reported by @kathy-phet via email:

The pickers for changing the function values (e.g. from +1 to +2) do not work on the iPad I have. It always just tries to move the function -- as opposed to changing the value in the function. So I cannot get anything but the "1"s right now on iPad. (latest dev version)

The "pickers" referred to are instances of scenery-phet.NumberPicker.

The version that @kathy-phet is testing is presumably 1.0.0-dev.26. I received no info on her iPad model, iOS version or browser version.

I reproduced the problem on iPad2 with iO 9.3.1 and mobile Safari.

Steps to reproduce:

  1. Start 1.0.0-dev.26: http://www.colorado.edu/physics/phet/dev/html/function-builder/1.0.0-dev.26/function-builder_en.html
  2. Go to the "Equations" screen
  3. In the functions carousel (horizontal carousel at bottom center), click on one of the pickers.

On desktop (Safari, Chrome, Firefox), the pickers work as desired: Clicking a picker increments/decrements the value, but does not result in moving the function. The function does not move or pop out of the carousel unless you click on the function somewhere other than the picker.

On mobile Safari, "clicking" on a picker doesn't increment/decrement the value, and instead pops the function out of the carousel. If you hold the pointer down, the picker value will change.

Here's the relevant code in function-builder.MathFunctionNode:

     // picker for changing operand value
      var picker = new FBNumberPicker( functionInstance.operandProperty, functionInstance.operandRange, {
        color: functionInstance.viewOptions.pickerColor,
        font: FBConstants.EQUATIONS_FUNCTION_PICKER_FONT,
        arrowLineWidth: 0.5,
        skipZero: !functionInstance.zeroOperandValid
      } );

      // prevent clicking on the picker from starting a drag sequence for the function node
      picker.addInputListener( {
        down: function( event, trail ) {
          event.handle(); // don't propagate event to parent
        }
      } );

Is it possible that event.handle is not working as expected for touch?

Assigning to @jonathanolson with high priority, since this affects ability to use the sim.

@pixelzoom
Copy link
Contributor Author

This is a showstopper.
Assigning to @ariel-phet to prioritize and assign.
Mentioning @jonathanolson, since he will undoubtedly be involved in addressing this.

@pixelzoom pixelzoom changed the title function pickers are not working on iOS [iOS] function pickers are not working May 11, 2016
@jonathanolson
Copy link
Contributor

It seems touch-specific (I can also reproduce in "device mode" on Chrome), and event.handle shouldn't* have any differences between mouse and touch.

It looks likely that touch-to-snag is enabled on the function bodies themselves, so that after the touch-down triggers the picker, any movement afterwards triggers the drag on the function body.

@jonathanolson
Copy link
Contributor

(Will be investigating how to fix).

@jonathanolson
Copy link
Contributor

It does seem that allowTouchSnag: false in MovableNode prevents the bad behavior. Can this be turned off, or have an option that controls which nodes can be touch snagged?

Additionally, the 'trail' parameter to the event listener should be removed, as Scenery just calls eventListener( event ) without a second parameter.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented May 17, 2016

The "movables" in the sim are cards and functions. Since interaction with cards and functions is similar, I'm fairly certain that we should have touch snag for all or nothing. And it would seem odd to have touch snag for some functions but not others.

So... options:

The first 2 options are the "all or nothing" extremes:

(1) Try to fix the problem so that we can have touchSnag:true for all cards and functions. Since interaction with this sim really lends itself to quick swiping (snagging) motions, this is obviously the preferred choice.

(2) Set touchSnag:false for all cards and functions. This is the easiest workaround, and ensures that interaction with cards and functions will behave the same. The sim may feel a little different than other PhET sims on touch devices. And it will be unfortunate that we won't be taking advantage of touchSnag in a sim that is particularly suited to it.

The other options are compromises:

(3) Set touchSnag:true for cards, but touchSnag:false for all function. Interaction with cards and functions will differ, but at least all functions will be similar.

(4) Set touchSnag:true for cards and all function that don't have pickers, touchSnag:false for functions with pickers. Interaction of cards and majority of functions will be similar, functions with pickers will be different.

Since this issue is not specific to this sim, my recommendation is option (1), pursue a fix. I don't like either of the compromises (3 & 4) and would probably just set touchSnag:false (2) for the entire sim if a fix can't be identified.

@ariel-phet your input?

@pixelzoom
Copy link
Contributor Author

Additionally, the 'trail' parameter to the event listener should be removed, as Scenery just calls eventListener( event ) without a second parameter.

@jonathanolson Are you referring to this in MathFunctionNode? Please point me to the description of the input listener API.

    picker.addInputListener( {
        down: function( event, trail ) {
          event.handle(); // don't propagate event to parent
        }
      } );

@pixelzoom
Copy link
Contributor Author

input listeners documentation: http://phetsims.github.io/scenery/doc/user-input

@jonathanolson
Copy link
Contributor

The "snag" is triggered by a 'touchenter' event, from Input's enterEvents. The 'enter' event is actually sent to each individual thing that is now under the pointer (which wasn't before, which since it is a "new" pointer, is everything), and doesn't bubble (so handle() won't do anything in touchenter).

Additionally, the 'enter' events happen before the 'down' event, so it's actually snagging the function node before manipulating the picker.

I'll try to figure out a good general solution, as there doesn't look like an easy patch to make.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented May 18, 2016

Discussed options in #29 (comment) with @ariel-phet. @jonathanolson should continue to pursue a general fix. But as a fallback, @ariel-phet would like to try option (3) in a dev version.

@pixelzoom
Copy link
Contributor Author

@ariel-phet here's the dev version that you requested:
http://www.colorado.edu/physics/phet/dev/html/function-builder/1.0.0-dev.31/function-builder_en.html

Cards have allowTouchSnag:true, functions have allowTouchSnag:false.

@pixelzoom pixelzoom assigned ariel-phet and unassigned pixelzoom May 18, 2016
@pixelzoom
Copy link
Contributor Author

Re the above dev version... It doesn't feel too bad to me, probably a reasonable workaround until a general solution can be implemented in scenery. @ariel-phet if you agree, please change the priority of this issue to what you feel is appropriate.

@ariel-phet
Copy link

Tried on iPad, does not really seem to affect usability. If I deliberately try to swipe across a function card I can see this action does not work, but it is very natural to grab the function cards since they are so large compared to my finger. I feel very comfortable with this solution.

@ariel-phet
Copy link

Given testing and comments above, changing this to priority 5, as we have other pressing issues.

@pixelzoom
Copy link
Contributor Author

The general issue has been moved to phetsims/scenery#547, with same priority (deferred) that @ariel-phet has specified here.

Issue #49 reminds us to re-enable this feature in function-builder when scenery is fixed.

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants