Investigate creating a DisplayMode for NumberPickers

[marlitas]

From the phet-io design doc: Solicit input from MK to consult a bit on the strategy, and maybe CM, since it’s a PhET-iO specific common-code feature.

Consider phetsims/sun#617 as well

Lots of design questions. Schedule a meeting next week with AR.

This would be similar to the feature ComboBox has.

[marlitas]

Relevant discussion in: phetsims/sun#884

[marlitas]

@zepumph is doing some initial investigation for us.

[zepumph]

I added my idea to the end of phetsims/sun#884 (comment). I believe @samreid and I have the same idea! If that is what works best for this sim that is fine, but I believe that it will likely add complexity to try to do this in a way that isn't in common code directly. Sorry I can't be of more help here.

[marlitas]

I took @zepumph and @samreid's patches a step forward and really want to talk about how egregious it is to see if there's a way to solve this that doesn't feel completely invasive in the common code, but also provides the sim with a straightforward implementation.

Subject: [PATCH] numberPicker opacity
---
Index: mean-share-and-balance/js/common/view/TablePlateNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/mean-share-and-balance/js/common/view/TablePlateNode.ts b/mean-share-and-balance/js/common/view/TablePlateNode.ts
--- a/mean-share-and-balance/js/common/view/TablePlateNode.ts	(revision 4e62a4ccf10f0b042540e5f3cc14724eb587a401)
+++ b/mean-share-and-balance/js/common/view/TablePlateNode.ts	(date 1716412415043)
@@ -75,7 +75,9 @@
         color: MeanShareAndBalanceColors.numberPickerColorProperty,
         valueChangedSoundPlayer: snackQuantitySoundPlayer,
         boundarySoundPlayer: snackQuantitySoundPlayer,
-        tandem: options.tandem.createTandem( 'numberPicker' )
+        tandem: options.tandem.createTandem( 'numberPicker' ),
+        informativeComponentsDisabledOpacity: 1,
+        interactiveComponentsDisabledOpacity: 0.25
       }
     );
 
Index: sun/js/NumberPicker.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/NumberPicker.ts b/sun/js/NumberPicker.ts
--- a/sun/js/NumberPicker.ts	(revision 3cd5c702159e13688afd1b0b9da468f129ccee71)
+++ b/sun/js/NumberPicker.ts	(date 1716412274069)
@@ -80,7 +80,10 @@
   decrementEnabledFunction?: ( value: number, range: Range ) => boolean;
 
   // Opacity used to indicate disabled, [0,1] exclusive
+  // If individual component opacity is provided, disabledOpacity will be ignored.
   disabledOpacity?: number;
+  interactiveComponentsDisabledOpacity?: number | null;
+  informativeComponentsDisabledOpacity?: number | null;
 
   // Sound generators for when the NumberPicker's value changes, and when it hits range extremities.
   // Use nullSoundPlayer to disable.
@@ -165,6 +168,8 @@
       incrementEnabledFunction: ( value: number, range: Range ) => ( value !== null && value !== undefined && value < range.max ),
       decrementEnabledFunction: ( value: number, range: Range ) => ( value !== null && value !== undefined && value > range.min ),
       disabledOpacity: SceneryConstants.DISABLED_OPACITY,
+      informativeComponentsDisabledOpacity: null,
+      interactiveComponentsDisabledOpacity: null,
       valueChangedSoundPlayer: generalSoftClickSoundPlayer,
       boundarySoundPlayer: generalBoundaryBoopSoundPlayer,
 
@@ -184,6 +189,10 @@
       phetioFeatured: true
     }, providedOptions );
 
+    if ( options.informativeComponentsDisabledOpacity ) {
+     options.disabledOpacity = 1;
+    }
+
     if ( !options.formatValue ) {
       options.formatValue = ( value: number ) => Utils.toFixed( value, options.decimalPlaces );
     }
@@ -259,7 +268,7 @@
     // Nodes
 
     // displays the value
-    const valueNode = new Text( '', { font: options.font, pickable: false } );
+    const valueNode = new Text( '', { font: options.font, pickable: false, disabledOpacity: options.informativeComponentsDisabledOpacity } );
 
     // compute max width of text based on the width of all possible values.
     // See https://github.com/phetsims/area-model-common/issues/5
@@ -312,7 +321,9 @@
     const strokedBackground = new Rectangle( 0, 0, backgroundWidth, backgroundHeight, backgroundCornerRadius, backgroundCornerRadius, {
       pickable: false,
       stroke: options.backgroundStroke,
-      lineWidth: options.backgroundLineWidth
+      lineWidth: options.backgroundLineWidth,
+      disabledOpacity: options.interactiveComponentsDisabledOpacity ? options.interactiveComponentsDisabledOpacity : options.disabledOpacity,
+      enabledProperty: this.enabledProperty
     } );
 
     // compute size of arrows
@@ -321,7 +332,9 @@
     const arrowOptions = {
       stroke: options.arrowStroke,
       lineWidth: options.arrowLineWidth,
-      pickable: false
+      pickable: false,
+      disabledOpacity: options.interactiveComponentsDisabledOpacity ? options.interactiveComponentsDisabledOpacity : options.disabledOpacity,
+      enabledProperty: this.enabledProperty
     };
 
     // increment arrow, pointing up, described clockwise from tip

[zepumph]

That is a very reasonable idea. I probably won't have time to discuss this week, but perhaps talk with @samreid about that feasibility. Something I didn't understand too much was if we wanted to make a general notion of this feature, "informationalEnabled" or something like that. Then we could benefit from keeping the actual opacities inside of sun. That seems best. I bet @samreid has some other thoughts.

[marlitas]

I liked @zepumph's suggestion above, so I went ahead and modified my patch to have an option useDisplayMode. In thinking about how this might apply to other components, this option could trigger opacity changes in only the interactive portions of a component while keeping any informational components at full opacity. Although I don't want to get too in the weeds over general design and code patterns.

Subject: [PATCH] useDisplayMode
---
Index: mean-share-and-balance/js/common/view/TablePlateNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/mean-share-and-balance/js/common/view/TablePlateNode.ts b/mean-share-and-balance/js/common/view/TablePlateNode.ts
--- a/mean-share-and-balance/js/common/view/TablePlateNode.ts	(revision 4e62a4ccf10f0b042540e5f3cc14724eb587a401)
+++ b/mean-share-and-balance/js/common/view/TablePlateNode.ts	(date 1716492811240)
@@ -75,7 +75,8 @@
         color: MeanShareAndBalanceColors.numberPickerColorProperty,
         valueChangedSoundPlayer: snackQuantitySoundPlayer,
         boundarySoundPlayer: snackQuantitySoundPlayer,
-        tandem: options.tandem.createTandem( 'numberPicker' )
+        tandem: options.tandem.createTandem( 'numberPicker' ),
+        useDisplayMode: true
       }
     );
 
Index: sun/js/NumberPicker.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/NumberPicker.ts b/sun/js/NumberPicker.ts
--- a/sun/js/NumberPicker.ts	(revision 3cd5c702159e13688afd1b0b9da468f129ccee71)
+++ b/sun/js/NumberPicker.ts	(date 1716492789813)
@@ -80,8 +80,13 @@
   decrementEnabledFunction?: ( value: number, range: Range ) => boolean;
 
   // Opacity used to indicate disabled, [0,1] exclusive
+  // If individual component opacity is provided, disabledOpacity will be ignored.
   disabledOpacity?: number;
 
+  // When component is disabled we do not want the disabledOpacity to apply to informational components. This creates
+  // a "displayMode" where the information can be clearly seen, but interactive components are disabled.
+  useDisplayMode?: boolean;
+
   // Sound generators for when the NumberPicker's value changes, and when it hits range extremities.
   // Use nullSoundPlayer to disable.
   valueChangedSoundPlayer?: TSoundPlayer;
@@ -165,6 +170,7 @@
       incrementEnabledFunction: ( value: number, range: Range ) => ( value !== null && value !== undefined && value < range.max ),
       decrementEnabledFunction: ( value: number, range: Range ) => ( value !== null && value !== undefined && value > range.min ),
       disabledOpacity: SceneryConstants.DISABLED_OPACITY,
+      useDisplayMode: false,
       valueChangedSoundPlayer: generalSoftClickSoundPlayer,
       boundarySoundPlayer: generalBoundaryBoopSoundPlayer,
 
@@ -184,6 +190,12 @@
       phetioFeatured: true
     }, providedOptions );
 
+    let interactiveComponentsDisabledOpacity = options.disabledOpacity;
+    if ( options.useDisplayMode ) {
+     options.disabledOpacity = 1;
+     interactiveComponentsDisabledOpacity = SceneryConstants.DISABLED_OPACITY;
+    }
+
     if ( !options.formatValue ) {
       options.formatValue = ( value: number ) => Utils.toFixed( value, options.decimalPlaces );
     }
@@ -312,7 +324,9 @@
     const strokedBackground = new Rectangle( 0, 0, backgroundWidth, backgroundHeight, backgroundCornerRadius, backgroundCornerRadius, {
       pickable: false,
       stroke: options.backgroundStroke,
-      lineWidth: options.backgroundLineWidth
+      lineWidth: options.backgroundLineWidth,
+      disabledOpacity: interactiveComponentsDisabledOpacity,
+      enabledProperty: this.enabledProperty
     } );
 
     // compute size of arrows
@@ -321,7 +335,9 @@
     const arrowOptions = {
       stroke: options.arrowStroke,
       lineWidth: options.arrowLineWidth,
-      pickable: false
+      pickable: false,
+      disabledOpacity: interactiveComponentsDisabledOpacity,
+      enabledProperty: this.enabledProperty
     };
 
     // increment arrow, pointing up, described clockwise from tip

[samreid]

Patch from collaboration with @marlitas

Subject: [PATCH] useDisplayMode
---
Index: sun/js/NumberPicker.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/NumberPicker.ts b/sun/js/NumberPicker.ts
--- a/sun/js/NumberPicker.ts	(revision 3cd5c702159e13688afd1b0b9da468f129ccee71)
+++ b/sun/js/NumberPicker.ts	(date 1716496693158)
@@ -57,10 +57,12 @@
   mouseAreaYDilation?: number;
   backgroundStroke?: TColor;
   backgroundLineWidth?: number;
+  backgroundDisabledOpacity?: number;
   arrowHeight?: number;
   arrowYSpacing?: number;
   arrowStroke?: TColor;
   arrowLineWidth?: number;
+  arrowDisabledOpacity?: number;
   valueMaxWidth?: number | null; // If non-null, it will cap the value's maxWidth to this value
 
   /**
@@ -156,10 +158,12 @@
       mouseAreaYDilation: 5,
       backgroundStroke: 'gray',
       backgroundLineWidth: 0.5,
+      backgroundDisabledOpacity: 1,
       arrowHeight: 6,
       arrowYSpacing: 3,
       arrowStroke: 'black',
       arrowLineWidth: 0.25,
+      arrowDisabledOpacity: 1,
       valueMaxWidth: null,
       onInput: _.noop,
       incrementEnabledFunction: ( value: number, range: Range ) => ( value !== null && value !== undefined && value < range.max ),
@@ -312,7 +316,9 @@
     const strokedBackground = new Rectangle( 0, 0, backgroundWidth, backgroundHeight, backgroundCornerRadius, backgroundCornerRadius, {
       pickable: false,
       stroke: options.backgroundStroke,
-      lineWidth: options.backgroundLineWidth
+      lineWidth: options.backgroundLineWidth,
+      disabledOpacity: options.backgroundDisabledOpacity,
+      enabledProperty: this.enabledProperty
     } );
 
     // compute size of arrows
@@ -321,6 +327,8 @@
     const arrowOptions = {
       stroke: options.arrowStroke,
       lineWidth: options.arrowLineWidth,
+      disabledOpacity: options.arrowDisabledOpacity,
+      enabledProperty: this.enabledProperty,
       pickable: false
     };
 
@@ -514,6 +522,7 @@
       decrementEnabledProperty.dispose();
       this.incrementArrow.dispose();
       this.decrementArrow.dispose();
+      strokedBackground.dispose();
 
       if ( valueProperty.hasListener( valueObserver ) ) {
         valueProperty.unlink( valueObserver );
Index: mean-share-and-balance/js/common/view/TablePlateNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/mean-share-and-balance/js/common/view/TablePlateNode.ts b/mean-share-and-balance/js/common/view/TablePlateNode.ts
--- a/mean-share-and-balance/js/common/view/TablePlateNode.ts	(revision 4e62a4ccf10f0b042540e5f3cc14724eb587a401)
+++ b/mean-share-and-balance/js/common/view/TablePlateNode.ts	(date 1716496791501)
@@ -8,7 +8,7 @@
  * @author John Blanco (PhET Interactive Simulations)
  */
 
-import { Image, Node, NodeOptions } from '../../../../scenery/js/imports.js';
+import { Image, Node, NodeOptions, SceneryConstants } from '../../../../scenery/js/imports.js';
 import meanShareAndBalance from '../../meanShareAndBalance.js';
 import Plate from '../model/Plate.js';
 import NumberPicker from '../../../../sun/js/NumberPicker.js';
@@ -75,7 +75,11 @@
         color: MeanShareAndBalanceColors.numberPickerColorProperty,
         valueChangedSoundPlayer: snackQuantitySoundPlayer,
         boundarySoundPlayer: snackQuantitySoundPlayer,
-        tandem: options.tandem.createTandem( 'numberPicker' )
+        tandem: options.tandem.createTandem( 'numberPicker' ),
+
+        disabledOpacity: 1,
+        arrowDisabledOpacity: SceneryConstants.DISABLED_OPACITY,
+        backgroundDisabledOpacity: SceneryConstants.DISABLED_OPACITY
       }
     );
 

[marlitas]

I implemented the patch above with some added documentation in TablePlateNode, and also checked some sims to make sure their implementation wasn't impacted. Sending over to @samreid for review.

[marlitas]

This is also ready for @amanda-phet's design review.

[samreid]

Code review looks good. Nice work.

[amanda-phet]

enabledProperty for number pickers now displays a full opacity number. Looks great to me.