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

Alternative input design for FaucetNode #773

Closed
3 tasks
pixelzoom opened this issue Oct 3, 2022 · 36 comments
Closed
3 tasks

Alternative input design for FaucetNode #773

pixelzoom opened this issue Oct 3, 2022 · 36 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Oct 3, 2022

Discussed briefly with @arouinfar via Zoom. This is a prerequisite to phetsims/ph-scale#249 (support for alt input). @kathy-phet FYI.

FaucetNode is a common-code UI component used for the faucets in pH Scale and many other sims. It needs alt input design, including answers to the following:

  • Which keys are used?
  • What are the defaults for KeyboardDragListener options dragVelocity, shiftDragVelocity, hotkeyHoldInterval, etc.?
  • What does the keyboard help look like? (mockup)

Keep in mind that FaucetNode has 2 options that affect it's alt input behavior, and the keyboard help:

  • tapToDispenseEnabled: boolean -- If true (default), tapping the faucet knob momentarily opens the faucet, then closes it. If false, tapping the knob does nothing.
  • closeOnRelease: boolean - If true (default), the faucet is closed when released. If false, the faucet remains open.

Sims that use FaucetNode, for context:

  • beers-law-lab, concentration
  • energy-forms-and-changes
  • faradays-electromagnetic-lab, generator
  • fluid-pressure-and-flow
  • ph-scale, ph-scale-basics
  • scenery-phet demo (Components screen)
  • wave-interference
@pixelzoom
Copy link
Contributor Author

In addition to general design, we need input from the a11y team on how this should be implemented. Should it use AccessibleSlider or KeyboardDragListener? And what do you recommend for keyboard help. Assigning to @jessegreenberg and @terracoda.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Oct 4, 2022

I should mention that this is high priority, because pH Scale and pH Scale: Basics are included in the PhET-iO batch release https://github.com/orgs/phetsims/projects/44.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Oct 4, 2022

@arouinfar here's a proposal for the keyboard help, implemented in ph-scale master. For the general case, the "Dispense a small amount" row is optional, to match how the FaucetNode was created with tapToDispenseEnabled. The optional closeOnRelease of behavior of FaucetNode is not mentioned in the keyboard help - does it need to be?

Also note that FaucetNode currently has no alt input support. So you won't experience the behavior described here. In pH Scale, the FaucetNodes will get focus, but you can't interact with them yet via the keyboard.

Let me know what you'd like to change.

screenshot_1905

EDIT: Major changes were needed before this proposal could be reviewed. See #773 (comment).

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Oct 4, 2022

Also note that FaucetNode currently has no alt input support. ....

My mistake - FaucetNode does have some support for alt input. It's treated like a slider, using AccessibleSlider. I didn't realize this because there seems to be an assuption that the FaucetNode is pointing to the right, and sliding to the right increases flow. In pH Scale, all of the FaucetNodex point to the left, and you slide to the left to increase flow. So (for example) pressing the right arrow moves the faucet knob to the left.

After thinking about it, I sort of get it -- for a slider, sliding the thumb to the left typically (but not always) decreases the value, while sliding to the right increases the value. But it feels very confusing for the knob to be moving in the opposite direction as the arrow key that's pressed. @arouinfar does this feel confusing to you?

I don't see where AccessibleSlider was added in the git history, so assigning to @jessegreenberg and @zepumph. Do you think something should be changed about how this is behaving in pH Scale? Has this been addressed in some other sim?

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Oct 5, 2022

While AccessibleSlider was added to FaucetNode to make it behave like a slider, alt input support for the tapToDispenseEnabled feature was not added. Was that feature intentionally omitted? Is there an alt input design for FaucetNode that I can refer to?

@pixelzoom
Copy link
Contributor Author

@arouinfar @terracoda This is another case (similar to #610) where a .md file describing the design would be helpful. So let's make sure that happens as part of this issue.

@pixelzoom pixelzoom self-assigned this Oct 5, 2022
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Oct 5, 2022

Since FaucetNode is currently implemented as AccessibleSlider, what I proposed for keyboard help in #773 (comment) is incomplete. So it made sense to use the common-code help for "Slider Controls" help (SliderControlsKeyboardHelpSection), with a more specific title. For example, see the help for the Macro screen below.

What's probematic about this is that I haven't been able to figure out how to append additional lines to this help, so that we can provide help for the optional tapToDispenseEnabled feature.

screenshot_1915

@jessegreenberg
Copy link
Contributor

I recall testing AccessibleSlider in FaucetNode many years ago in the PhET team a11y sprint. I don't think it ever saw through full design beyond that.

The first thing I noticed when trying it out was closeOnRelease is very annoying when using a keyboard. We might want to ignore or change that feature somehow for alt input.

I don't know how important it is to control the flow rate, but a much simpler and more accessible interaction would be to make this behave like a toggle button. Enter/spacebar to toggle the flow of liquid.

@terracoda
Copy link

I agree with @jessegreenberg, I don't think the pull-faucet maps well to a slider and is more like a toggle.

Waves Intro uses a toggle button to release drops of liquid.

@zepumph
Copy link
Member

zepumph commented Oct 5, 2022

I think it would be worth spending a bit of time implementing with a toggle button interaction as an investigation before we make the decision. Most likely we won't know if it will be an acceptable simplification until we feel it out.

@jessegreenberg
Copy link
Contributor

I changed FaucetNode to use tapToDispense for alt input so a bit is let out every enter/spacebar press. It seemed like a nice place to start and feel it out since that function is already available. But it could also be switched to an on/off toggle without a timer.

@arouinfar @terracoda over to you to try it out and comment on next steps!

@jessegreenberg jessegreenberg removed their assignment Oct 5, 2022
@arouinfar
Copy link

arouinfar commented Oct 5, 2022

There is a lot of nuance around the way the FaucetNode behaves in different sims. Sometimes it's important for users to set the flow rate of the FaucetNode. Sometimes it's important for the faucet to closeOnRelease because users need to actively control the flow of water. Sometimes it's important to dispense a precise amount of water when clicking on the faucet to allow for finer adjustment of the volume. Essentially, there are both slider-like and button-like qualities to the interaction.

I think the tapToDispense behavior @jessegreenberg implemented in pH Scale generally works, but won't scale to all sims. Even so, if the user wants to completely fill or drain the beaker, it could require >20 key presses which is a bit annoying. Could we have modifier keys to add smaller/larger amounts? If so, this would work wonderfully.

The FaucetNode in energy-forms-and-changes and the Water Tower screen of fluid-pressure-and-flow are closeOnRelease: false. In these sims, it is critical to be able to set the flow rate, and once it's set, let it keep going while the user interacts with other parts of the sim. This sort of interaction doesn't map well to the behavior currently in pH Scale.

The first thing I noticed when trying it out was closeOnRelease is very annoying when using a keyboard. We might want to ignore or change that feature somehow for alt input.

@jessegreenberg would it be possible to trigger the closeOnRelease when the user tabs away from the FaucetNode? I'm imagining a flow something like this (assuming we are dealing with a slider):

  1. Tab to FaucetNode
  2. Use arrow key to open faucet. The FaucetNode stays open while it still has focus (unless you reach max volume and it becomes disabled).
  3. Subsequent arrow keys in the same direction open up the faucet further to increase the flow rate.
  4. Tab to the next item and the FaucetNode closes.

@zepumph
Copy link
Member

zepumph commented Oct 6, 2022

Subset of phetsims/scenery#1298

@pixelzoom pixelzoom removed their assignment Oct 7, 2022
@pixelzoom
Copy link
Contributor Author

I'm going to unassign myself for now. It feels like there are already too many people involved, and I'm happy to use (in ph-scale) whatever the designers/a11y team come up with. I'll be on vacation for a couple of week, feel free to ping me if you need my input.

@terracoda
Copy link

Are learners expected to move their attention to some where else while the faucet is on?

@arouinfar
Copy link

Are learners expected to move their attention to some where else while the faucet is on?

In general, yes, but the degree varies.

When the student turns on the faucet in pH Scale, their attention could shift to the beaker volume or the pH readout. To reach a particular volume, they will need to actively control the faucet while watching the volume indicator.
Screen Shot 2022-10-12 at 9 33 34 AM

In Energy Forms and Changes, the student will set the flow rate of the faucet, which does not automatically shut itself off (like pH Scale). Students may first set the flow rate and then attend to the energy throughout the system. Or, they may change the flow rate and watch what happens to the generator (or other parts of the system) while they are actively changing the flow rate.
image

@zepumph
Copy link
Member

zepumph commented Nov 25, 2022

It isn't clear to me that this is in fact high priority. @pixelzoom is this not longer pressing because ph-scale is proceeding without alt-input?

@pixelzoom
Copy link
Contributor Author

According to @kathy-phet, @jessegreenberg is the lead for implementing FaucetNode alt-input behavior.

@jessegreenberg
Copy link
Contributor

Here are notes from a design meeting today with @terracoda @pixelzoom @arouinfar and @jessegreenberg.

Feature list:

  • FaucetNode will always be composed with AccessibleSlider. Flow rate is important to control so we always need to control the value with arrow keys. By using AccessibleSlider, all the usual keyboard step functionality works too (page up/home/end/etc.)
  • If tapToDispenseEnabled: true, an additional listener will be added to make it behave like a button. It will dispense for a short amount of time (just like tapToDispenseEnabled: true for mouse/touch)
  • If closeOnRelease: true, the faucet will auto close on the blur event. Otherwise, the user will manually turn off the faucet.
  • The 0 key can be pressed to quickly turn off the faucet. It will be useful for people that aren't aware of the "home" command.
  • The focus highlight will surround the knob. A group focus highlight will surround the shooter and extend out to show how far the knob can be dragged. Same for interactive highlight.
    image

Potential challenge:

  • Some faucets point left. We need the orientation of the faucet to match the movement from the arrow keys. AccessibleValueHandler has options to support this, but how it gets used/exposed by FaucetNode is the tricky part.

Additional discussion:

  • We considered adding key bindings to 1, 2, 3 keys to quickly control the flow rate. We decided against it because it is already easy to explore the range of flow rate values with the arrow keys. And adding additional keys will take up more space in a keyboard help dialog.
  • We talked about commands to quickly set resultant values from flow rate like volume or energy. Commands like this could be done in a subclass or in sim specific code, but shouldn't be added to this general component.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Jan 25, 2024

This patch is working pretty well, but I want to try to simplify it by mixing AccessibleSlider into the shooter instead. I think it will make the highlight code more simple.

Subject: [PATCH] Remove opt out of Interactive Highlights, see https://github.com/phetsims/friction/issues/201
---
Index: js/FaucetNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/FaucetNode.ts b/js/FaucetNode.ts
--- a/js/FaucetNode.ts	(revision c1225ba4f2366945af8da3be87bf904a4161738b)
+++ b/js/FaucetNode.ts	(date 1706205640166)
@@ -26,7 +26,7 @@
 import Bounds2 from '../../dot/js/Bounds2.js';
 import LinearFunction from '../../dot/js/LinearFunction.js';
 import InstanceRegistry from '../../phet-core/js/documentation/InstanceRegistry.js';
-import { Circle, DragListener, Image, InteractiveHighlighting, Node, NodeOptions, Rectangle } from '../../scenery/js/imports.js';
+import { Circle, DragListener, HighlightPath, Image, InteractiveHighlighting, KeyboardListener, Node, NodeOptions, Rectangle } from '../../scenery/js/imports.js';
 import EventType from '../../tandem/js/EventType.js';
 import Tandem from '../../tandem/js/Tandem.js';
 import IOType from '../../tandem/js/types/IOType.js';
@@ -46,6 +46,9 @@
 import optionize from '../../phet-core/js/optionize.js';
 import StrictOmit from '../../phet-core/js/types/StrictOmit.js';
 import { TimerListener } from '../../axon/js/Timer.js';
+import AccessibleSlider, { AccessibleSliderOptions } from '../../sun/js/accessibility/AccessibleSlider.js';
+import { Shape } from '../../kite/js/imports.js';
+import DynamicProperty from '../../axon/js/DynamicProperty.js';
 
 // constants
 const DEBUG_ORIGIN = false; // when true, draws a red dot at the origin (bottom-center of the spout)
@@ -66,6 +69,7 @@
   tapToDispenseInterval?: number; // tap-to-dispense feature: amount of time that fluid is dispensed, in milliseconds
   closeOnRelease?: boolean; // when true, releasing the shooter closes the faucet
   interactiveProperty?: TReadOnlyProperty<boolean>; // when the faucet is interactive, the flow rate control is visible, see issue #67
+  flipSliderInput?: boolean; // when true, the keyboard input for the flow rate is flipped - this is useful if the faucet is flipped horizontally
 
   // Overcome a flickering problems, see https://github.com/phetsims/wave-interference/issues/187
   rasterizeHorizontalPipeNode?: boolean;
@@ -73,10 +77,13 @@
   // options for the nested type ShooterNode
   shooterOptions?: ShooterNodeOptions;
 };
-type ParentOptions = NodeOptions;
-export type FaucetNodeOptions = SelfOptions & ParentOptions;
+
+type ParentOptions = AccessibleSliderOptions & NodeOptions;
 
-export default class FaucetNode extends Node {
+// TODO: How do I add valueProperty to this?
+export type FaucetNodeOptions = SelfOptions & StrictOmit<ParentOptions, 'interactiveHighlightEnabled'>;
+
+export default class FaucetNode extends AccessibleSlider( Node, 0 ) {
 
   private readonly disposeFaucetNode: () => void;
 
@@ -94,11 +101,19 @@
       closeOnRelease: true,
       interactiveProperty: new Property( true ),
       rasterizeHorizontalPipeNode: false,
+      flipSliderInput: false,
+
+      // AccessibleSliderOptions
+      enabledRangeProperty: new phet.axon.Property( new phet.dot.Range( 0, maxFlowRate ) ),
 
       // ParentOptions
       scale: 1,
       enabledProperty: enabledProperty,
 
+      // AccessibleSlider is composed with InteractiveHighlighting, but the InteractiveHighlight should surround
+      // the shooter.
+      interactiveHighlightEnabled: false,
+
       // phet-io
       tandem: Tandem.REQUIRED,
       tandemNameSuffix: 'FaucetNode',
@@ -109,6 +124,14 @@
 
     assert && assert( ( 1000 * options.tapToDispenseAmount / options.tapToDispenseInterval ) <= maxFlowRate );
 
+    // An intermediate Property is given to the AccessibleSlider to flip the input but not the flow rate.
+    options.valueProperty = options.flipSliderInput ?
+                            new DynamicProperty( new Property( flowRateProperty ), {
+                              bidirectional: true,
+                              map: ( propertyValue: number ) => maxFlowRate - propertyValue,
+                              inverseMap: ( propertyValue: number ) => maxFlowRate - propertyValue
+                            } ) : flowRateProperty;
+
     // shooter
     const shooterNode = new ShooterNode( enabledProperty, options.shooterOptions );
 
@@ -273,8 +296,51 @@
     } );
     shooterNode.addInputListener( dragListener );
 
+    const keyboardListener = new KeyboardListener( {
+      keys: [ 'enter', 'space', '0' ],
+      callback: ( event, keysPressed ) => {
+        if ( options.tapToDispenseEnabled && [ 'enter', 'space' ].includes( keysPressed ) ) {
+
+          // stop the previous timeout before running a new dispsn
+          if ( tapToDispenseIsRunning ) {
+            endTapToDispense();
+          }
+
+          tapToDispenseIsArmed = true;
+          startTapToDispense();
+        }
+
+        if ( keysPressed === '0' ) {
+          flowRateProperty.set( 0 );
+        }
+      }
+    } );
+    this.addInputListener( keyboardListener );
+
+    // if close on release is true, the faucet will turn off when focus moves somewhere else
+    if ( options.closeOnRelease ) {
+      this.addInputListener( {
+        blur: () => {
+          flowRateProperty.set( 0 );
+        }
+      } );
+    }
+
+    // The highlights surrounds the knob and shooter, ShooterNode provides a custom shapes.
+    const localHighlightBounds = this.globalToLocalBounds( shooterNode.localToGlobalBounds( shooterNode.focusHighlightBounds ) );
+    const localGroupHighlightBounds = this.globalToLocalBounds( shooterNode.localToGlobalBounds( shooterNode.groupFocusHighlightBounds ) );
+
+    this.focusHighlight = new HighlightPath( Shape.bounds( localHighlightBounds ) );
+    this.groupFocusHighlight = new HighlightPath( Shape.bounds( localGroupHighlightBounds ), {
+      outerStroke: HighlightPath.OUTER_LIGHT_GROUP_FOCUS_COLOR,
+      innerStroke: HighlightPath.INNER_LIGHT_GROUP_FOCUS_COLOR,
+      outerLineWidth: HighlightPath.GROUP_OUTER_LINE_WIDTH,
+      innerLineWidth: HighlightPath.GROUP_INNER_LINE_WIDTH
+    } );
+
     const flowRateObserver = ( flowRate: number ) => {
       shooterNode.left = bodyNode.left + offsetToFlowRate.inverse( flowRate );
+      ( this.focusHighlight as HighlightPath ).right = shooterNode.right + 5;
     };
     flowRateProperty.link( flowRateObserver );
 
@@ -355,6 +421,12 @@
 
   private readonly disposeShooterNode: () => void;
 
+  // Custom bounds that define highlight shapes for the ShooterNode. The FaucetNode uses these as its own
+  // highlight to indicate that the Shooter is the interactive part (but the FaucetNode is composed with
+  // InteractiveHighlighting)
+  public readonly focusHighlightBounds: Bounds2;
+  public readonly groupFocusHighlightBounds: Bounds2;
+
   public constructor( enabledProperty: TReadOnlyProperty<boolean>, providedOptions?: ShooterNodeOptions ) {
 
     const options = optionize<ShooterNodeOptions, ShooterNodeSelfOptions, NodeOptions>()( {
@@ -407,6 +479,21 @@
     knobNode.centerY = flangeNode.centerY;
     knobDisabledNode.translation = knobNode.translation;
 
+    // custom focus highlights that are relative to components of this Node
+    this.focusHighlightBounds = new Bounds2( flangeNode.left, knobNode.top, knobNode.right, knobNode.bottom ).dilated( 5 );
+
+    // the group focus highlight bounds surrounds the whole shooter and extends out to show the range of the knob
+    this.groupFocusHighlightBounds = new Bounds2(
+      shaftNode.left,
+      knobNode.top - 10,
+      knobNode.right + shaftNode.width / 2 + 15,
+      knobNode.bottom + 10
+    );
+
+    // This ShooterNode receives input for Interactive Highlighting so the interactiveHighlight is set here, even
+    // though focus highlights are set on the FaucetNode.
+    this.interactiveHighlight = Shape.bounds( this.focusHighlightBounds );
+
     const enabledObserver = ( enabled: boolean ) => {
       // the entire shooter is draggable, but encourage dragging by the knob by changing its cursor
       this.pickable = enabled;
Index: js/demo/components/demoFaucetNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/demo/components/demoFaucetNode.ts b/js/demo/components/demoFaucetNode.ts
--- a/js/demo/components/demoFaucetNode.ts	(revision c1225ba4f2366945af8da3be87bf904a4161738b)
+++ b/js/demo/components/demoFaucetNode.ts	(date 1706204847037)
@@ -17,6 +17,7 @@
 import Utils from '../../../../dot/js/Utils.js';
 import PickRequired from '../../../../phet-core/js/types/PickRequired.js';
 import Panel from '../../../../sun/js/Panel.js';
+import PickOptional from '../../../../phet-core/js/types/PickOptional.js';
 
 const MAX_FLOW_RATE = 1;
 const FONT = new PhetFont( 14 );
@@ -55,9 +56,16 @@
     closeOnRelease: false
   } );
 
+  const panel5 = new FaucetDemoPanel( panelNumber++, MAX_FLOW_RATE, {
+    tapToDispenseEnabled: true,
+    closeOnRelease: true,
+    flipSliderInput: true
+  } );
+
   const panelsBox = new HBox( {
-    children: [ panel1, panel2, panel3, panel4 ],
+    children: [ panel1, panel2, panel3, panel4, panel5 ],
     spacing: 15,
+    maxWidth: layoutBounds.width - 20,
     resize: false
   } );
 
@@ -71,7 +79,7 @@
 
 class FaucetDemoPanel extends Panel {
 
-  public constructor( panelNumber: number, maxFlowRate: number, faucetNodeOptions: PickRequired<FaucetNodeOptions, 'tapToDispenseEnabled' | 'closeOnRelease'> ) {
+  public constructor( panelNumber: number, maxFlowRate: number, faucetNodeOptions: PickRequired<FaucetNodeOptions, 'tapToDispenseEnabled' | 'closeOnRelease'> & PickOptional<FaucetNodeOptions, 'flipSliderInput'> ) {
 
     const titleText = new Text( `Example ${panelNumber}`, {
       font: new PhetFont( {
@@ -99,6 +107,10 @@
         }
       }, faucetNodeOptions ) );
 
+    if ( faucetNodeOptions.flipSliderInput ) {
+      faucetNode.setScaleMagnitude( -0.7, 0.7 );
+    }
+
     const flowRateStringProperty = new DerivedProperty( [ flowRateProperty ],
       flowRate => `flowRate=${Utils.toFixed( flowRate, 2 )}` );
     const flowRateDisplay = new Text( flowRateStringProperty, {
@@ -111,7 +123,7 @@
     } );
 
     const content = new VBox( {
-      align: 'left',
+      align: faucetNodeOptions.flipSliderInput ? 'right' : 'left',
       spacing: 10,
       children: [
         titleText,

This is what that looks like. Its not as good because you have to forward everything to the ShooterNode but still have to create a public Bounds for the highlight Shape. Im going to clean up and commit the first one.

Subject: [PATCH] Remove opt out of Interactive Highlights, see https://github.com/phetsims/friction/issues/201
---
Index: js/FaucetNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/FaucetNode.ts b/js/FaucetNode.ts
--- a/js/FaucetNode.ts	(revision d3535d8c7b40a4d83e8ae17201976166ca50db4a)
+++ b/js/FaucetNode.ts	(date 1706311123568)
@@ -26,7 +26,7 @@
 import Bounds2 from '../../dot/js/Bounds2.js';
 import LinearFunction from '../../dot/js/LinearFunction.js';
 import InstanceRegistry from '../../phet-core/js/documentation/InstanceRegistry.js';
-import { Circle, DragListener, Image, InteractiveHighlighting, Node, NodeOptions, Rectangle } from '../../scenery/js/imports.js';
+import { Circle, DragListener, HighlightPath, Image, KeyboardListener, Node, NodeOptions, Rectangle } from '../../scenery/js/imports.js';
 import EventType from '../../tandem/js/EventType.js';
 import Tandem from '../../tandem/js/Tandem.js';
 import IOType from '../../tandem/js/types/IOType.js';
@@ -43,9 +43,12 @@
 import faucetVerticalPipe_png from '../images/faucetVerticalPipe_png.js';
 import sceneryPhet from './sceneryPhet.js';
 import TReadOnlyProperty from '../../axon/js/TReadOnlyProperty.js';
-import optionize from '../../phet-core/js/optionize.js';
+import optionize, { combineOptions } from '../../phet-core/js/optionize.js';
 import StrictOmit from '../../phet-core/js/types/StrictOmit.js';
 import { TimerListener } from '../../axon/js/Timer.js';
+import { Shape } from '../../kite/js/imports.js';
+import DynamicProperty from '../../axon/js/DynamicProperty.js';
+import AccessibleSlider, { AccessibleSliderOptions } from '../../sun/js/accessibility/AccessibleSlider.js';
 
 // constants
 const DEBUG_ORIGIN = false; // when true, draws a red dot at the origin (bottom-center of the spout)
@@ -66,6 +69,7 @@
   tapToDispenseInterval?: number; // tap-to-dispense feature: amount of time that fluid is dispensed, in milliseconds
   closeOnRelease?: boolean; // when true, releasing the shooter closes the faucet
   interactiveProperty?: TReadOnlyProperty<boolean>; // when the faucet is interactive, the flow rate control is visible, see issue #67
+  flipSliderInput?: boolean; // when true, the keyboard input for the flow rate is flipped - this is useful if the faucet is flipped horizontally
 
   // Overcome a flickering problems, see https://github.com/phetsims/wave-interference/issues/187
   rasterizeHorizontalPipeNode?: boolean;
@@ -94,6 +98,7 @@
       closeOnRelease: true,
       interactiveProperty: new Property( true ),
       rasterizeHorizontalPipeNode: false,
+      flipSliderInput: false,
 
       // ParentOptions
       scale: 1,
@@ -109,8 +114,23 @@
 
     assert && assert( ( 1000 * options.tapToDispenseAmount / options.tapToDispenseInterval ) <= maxFlowRate );
 
+    const shooterHandleTapToDispense = () => {
+
+      // stop the previous timeout before running a new dispsn
+      if ( tapToDispenseIsRunning ) {
+        endTapToDispense();
+      }
+
+      tapToDispenseIsArmed = true;
+      startTapToDispense();
+    };
+
     // shooter
-    const shooterNode = new ShooterNode( enabledProperty, options.shooterOptions );
+    const shooterNode = new ShooterNode( flowRateProperty, maxFlowRate, enabledProperty, shooterHandleTapToDispense, combineOptions<ShooterNodeOptions>( {
+      flipSliderInput: options.flipSliderInput,
+      closeOnRelease: options.closeOnRelease,
+      tapToDispenseEnabled: options.tapToDispenseEnabled
+    }, options.shooterOptions ) );
 
     // track that the shooter moves in
     const trackNode = new Image( faucetTrack_png );
@@ -302,6 +322,9 @@
       tandemName: 'flowRateProperty'
     } );
 
+    const localHighlightBounds = this.globalToLocalBounds( shooterNode.localToGlobalBounds( shooterNode.groupFocusHighlightBounds ) );
+    this.groupFocusHighlight = new HighlightPath( Shape.bounds( localHighlightBounds ) );
+
     this.disposeFaucetNode = () => {
 
       // Properties
@@ -345,24 +368,32 @@
   touchAreaYDilation?: number;
   mouseAreaXDilation?: number;
   mouseAreaYDilation?: number;
+
+  flipSliderInput?: boolean;
+  closeOnRelease: boolean;
+  tapToDispenseEnabled: boolean;
 };
-type ShooterNodeOptions = ShooterNodeSelfOptions; // no NodeOptions are included
+type ShooterNodeParentOptions = NodeOptions & AccessibleSliderOptions;
+type ShooterNodeOptions = ShooterNodeSelfOptions & StrictOmit<ShooterNodeParentOptions, 'enabledRangeProperty'>;
 
 /**
  * The 'shooter' is the interactive part of the faucet.
  */
-class ShooterNode extends InteractiveHighlighting( Node ) {
+class ShooterNode extends AccessibleSlider( Node, 0 ) {
 
   private readonly disposeShooterNode: () => void;
+  public readonly groupFocusHighlightBounds: Bounds2;
 
-  public constructor( enabledProperty: TReadOnlyProperty<boolean>, providedOptions?: ShooterNodeOptions ) {
+  public constructor( flowRateProperty: Property<number>, maxFlowRate: number, enabledProperty: TReadOnlyProperty<boolean>, handleTapToDispense: () => void, providedOptions?: ShooterNodeOptions ) {
 
-    const options = optionize<ShooterNodeOptions, ShooterNodeSelfOptions, NodeOptions>()( {
+    const options = optionize<ShooterNodeOptions, ShooterNodeSelfOptions, ShooterNodeParentOptions>()( {
       knobScale: 0.6,
       touchAreaXDilation: 0,
       touchAreaYDilation: 0,
       mouseAreaXDilation: 0,
-      mouseAreaYDilation: 0
+      mouseAreaYDilation: 0,
+      flipSliderInput: false,
+      enabledRangeProperty: new phet.axon.Property( new phet.dot.Range( 0, maxFlowRate ) )
     }, providedOptions );
 
     // knob
@@ -386,16 +417,24 @@
     // stop
     const stopNode = new Image( faucetStop_png );
 
-    super( {
-      children: [
-        shaftNode,
-        stopNode,
-        flangeNode,
-        flangeDisabledNode,
-        knobNode,
-        knobDisabledNode
-      ]
-    } );
+    // An intermediate Property is given to the AccessibleSlider to flip the input but not the flow rate.
+    options.valueProperty = options.flipSliderInput ?
+                            new DynamicProperty( new Property( flowRateProperty ), {
+                              bidirectional: true,
+                              map: ( propertyValue: number ) => maxFlowRate - propertyValue,
+                              inverseMap: ( propertyValue: number ) => maxFlowRate - propertyValue
+                            } ) : flowRateProperty;
+
+    options.children = [
+      shaftNode,
+      stopNode,
+      flangeNode,
+      flangeDisabledNode,
+      knobNode,
+      knobDisabledNode
+    ];
+
+    super( options );
 
     // layout, relative to shaft
     stopNode.x = shaftNode.x + 13;
@@ -407,6 +446,12 @@
     knobNode.centerY = flangeNode.centerY;
     knobDisabledNode.translation = knobNode.translation;
 
+    // custom focus highlights that are relative to components of this Node
+    this.focusHighlight = new HighlightPath( Shape.bounds( new Bounds2( flangeNode.left, knobNode.top, knobNode.right, knobNode.bottom ).dilated( 5 ) ) );
+
+    // Define a custom highlight shape that will be set on the parent Faucet - Can't set it on this Node because then it would translate with the knob
+    this.groupFocusHighlightBounds = new Bounds2( shaftNode.left, knobNode.top - 10, knobNode.right + shaftNode.width / 2 + 18, knobNode.bottom + 10 );
+
     const enabledObserver = ( enabled: boolean ) => {
       // the entire shooter is draggable, but encourage dragging by the knob by changing its cursor
       this.pickable = enabled;
@@ -418,6 +463,29 @@
     };
     enabledProperty.link( enabledObserver );
 
+    const keyboardListener = new KeyboardListener( {
+      keys: [ 'enter', 'space', '0' ],
+      callback: ( event, keysPressed ) => {
+        if ( options.tapToDispenseEnabled && [ 'enter', 'space' ].includes( keysPressed ) ) {
+          handleTapToDispense();
+        }
+
+        if ( keysPressed === '0' ) {
+          flowRateProperty.set( 0 );
+        }
+      }
+    } );
+    this.addInputListener( keyboardListener );
+
+    // if close on release is true, the faucet will turn off when focus moves somewhere else
+    if ( options.closeOnRelease ) {
+      this.addInputListener( {
+        blur: () => {
+          flowRateProperty.set( 0 );
+        }
+      } );
+    }
+
     this.disposeShooterNode = () => {
       if ( enabledProperty.hasListener( enabledObserver ) ) {
         enabledProperty.unlink( enabledObserver );
Index: js/demo/components/demoFaucetNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/demo/components/demoFaucetNode.ts b/js/demo/components/demoFaucetNode.ts
--- a/js/demo/components/demoFaucetNode.ts	(revision d3535d8c7b40a4d83e8ae17201976166ca50db4a)
+++ b/js/demo/components/demoFaucetNode.ts	(date 1706298986474)
@@ -17,6 +17,7 @@
 import Utils from '../../../../dot/js/Utils.js';
 import PickRequired from '../../../../phet-core/js/types/PickRequired.js';
 import Panel from '../../../../sun/js/Panel.js';
+import PickOptional from '../../../../phet-core/js/types/PickOptional.js';
 
 const MAX_FLOW_RATE = 1;
 const FONT = new PhetFont( 14 );
@@ -55,9 +56,16 @@
     closeOnRelease: false
   } );
 
+  const panel5 = new FaucetDemoPanel( panelNumber++, MAX_FLOW_RATE, {
+    tapToDispenseEnabled: true,
+    closeOnRelease: true,
+    flipSliderInput: true
+  } );
+
   const panelsBox = new HBox( {
-    children: [ panel1, panel2, panel3, panel4 ],
+    children: [ panel1, panel2, panel3, panel4, panel5 ],
     spacing: 15,
+    maxWidth: layoutBounds.width - 20,
     resize: false
   } );
 
@@ -71,7 +79,7 @@
 
 class FaucetDemoPanel extends Panel {
 
-  public constructor( panelNumber: number, maxFlowRate: number, faucetNodeOptions: PickRequired<FaucetNodeOptions, 'tapToDispenseEnabled' | 'closeOnRelease'> ) {
+  public constructor( panelNumber: number, maxFlowRate: number, faucetNodeOptions: PickRequired<FaucetNodeOptions, 'tapToDispenseEnabled' | 'closeOnRelease'> & PickOptional<FaucetNodeOptions, 'flipSliderInput'> ) {
 
     const titleText = new Text( `Example ${panelNumber}`, {
       font: new PhetFont( {
@@ -99,6 +107,10 @@
         }
       }, faucetNodeOptions ) );
 
+    if ( faucetNodeOptions.flipSliderInput ) {
+      faucetNode.setScaleMagnitude( -0.7, 0.7 );
+    }
+
     const flowRateStringProperty = new DerivedProperty( [ flowRateProperty ],
       flowRate => `flowRate=${Utils.toFixed( flowRate, 2 )}` );
     const flowRateDisplay = new Text( flowRateStringProperty, {
@@ -111,7 +123,7 @@
     } );
 
     const content = new VBox( {
-      align: 'left',
+      align: faucetNodeOptions.flipSliderInput ? 'right' : 'left',
       spacing: 10,
       children: [
         titleText,

@arouinfar arouinfar removed their assignment Jan 26, 2024
@jessegreenberg
Copy link
Contributor

OK, #773 (comment) was added in the above commits. @pixelzoom would you mind reviewing? A couple of questions in particular -

  • For these group bounds, is there a better way to find the width? The width of these bounds is arbitrary right now.
    // the group focus highlight bounds surrounds the whole shooter and extends out to show the range of the knob
    this.groupFocusHighlightBounds = new Bounds2(
      shaftNode.left,
      knobNode.top - HIGHLIGHT_DILATION * 2,
      knobNode.right + shaftNode.width / 2 + 15,
      knobNode.bottom + HIGHLIGHT_DILATION * 2
    );
  • To support "reversed" input for left facing FaucetNodes, I added an option to AccessibleValueHandler called reverseAlternativeInput. Does that seem reasonable? The DynamicProperty will flip arrow keys AND home/end/page keys. I think this is correct because on my laptop, home/end are accessed with function + arrow keys. But on larger keyboards home and end have their own keys and it might be strange to press "home" to set the max flow rate.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jan 29, 2024

I tested in scenery-phet demo, and in FEL. Behavior looks great - well done! I'll assign to @arouinfar to take this for a test-drive too, to make sure I didn't misunderstand anything about the design requirements.

To answer @jessegreenberg's questions...

For these group bounds, is there a better way to find the width? ...

I don't see a better way to do this. I think it's probably fine.

To support "reversed" input for left facing FaucetNodes, I added an option to AccessibleValueHandler called reverseAlternativeInput. Does that seem reasonable? ...

That seems perfect for AccessibleValueHandler.

For FaucetNode, it would be nice to have something that's more specific to a faucet API -- like option direction: 'left' | 'right' -- and have it set reverseAlternativeInput accordingly. But since it's currently the client's responsibility to change the direction (e.g. faucetNode.setScaleMagnitude( -0.7, 0.7 ) in demoFaucetNode) I think we can forgo such an option for now.

@pixelzoom pixelzoom assigned arouinfar and unassigned pixelzoom Jan 29, 2024
@arouinfar
Copy link

arouinfar commented Jan 30, 2024

I tested in the scenery-phet demo and FEL, and everything is looking great @jessegreenberg! The demo for Example 5 (left/reverse orientation) was also helpful to confirm that reversing the input directions feels correct.

The only question I have is if the keyboard step sizes (including shift and page) are adjustable on a sim-by-sim basis. The demo uses 1% (shift), 5% (regular), and 10% (page) which seems like a reasonable default, but I could imagine different step sizes being preferable in some sims.

@arouinfar arouinfar removed their assignment Jan 30, 2024
@terracoda
Copy link

Hi folks, I agree the faucet node works great with the keyboard. The continuous flashing of the lightbulb might be a problem - i.e. might cause seizures. You might already know this.

Additionally, I am confused by the Alt Input on the magnet on the other screens. I'll open an issue for that. The Alt input design has been changed and is not consistent with the existing sim, Faraday's Law. This causes inconsistency across sims which can be confusing.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jan 30, 2024

@arouinfar asked:

The only question I have is if the keyboard step sizes (including shift and page) are adjustable on a sim-by-sim basis. ...

The answer is yes. FaucetNode extends AccessibleSlider, so it has the same options that you're used to seeing for Slider: keyboardStep, shiftKeyboardStep, and pageKeyboardStep. In a7f3f28, I've added a demonstration of those options to the scenery-phet demo. The values used are:

        keyboardStep: 1,
        shiftKeyboardStep: 0.1,
        pageKeyboardStep: 2

Back to @jessegreenberg. If there's nothing else to be done for FaucetNode, I think it's safe to close this issue.

@jessegreenberg
Copy link
Contributor

Great, thanks for reviewing everyone! Closing this issue and we will add the keyboard help content in #839.

@phet-dev
Copy link
Contributor

Reopening because there is a TODO marked for this issue.

@jessegreenberg
Copy link
Contributor

These TODOs were in ph-scale related to the keyboard help content so I pointed them to #839.

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

7 participants