Self code review for common view directory, see: #258

js/balance-point/view/BalancePointControls.ts

@@ -72,7 +72,6 @@ export default class BalancePointControls extends MeanShareAndBalanceControls {
     const numberOfDataPointsProperty = model.selectedSceneModelProperty.value.targetNumberOfBallsProperty;
     const options = optionize<BalancePointControlsOptions, SelfOptions, MeanShareAndBalanceControlsOptions>()( {
       controlsPDOMOrder: [ meanFulcrumRadioButtonGroup, checkboxGroup ],
-      soccerContext: true,
       numberSpinnerOptions: {
         arrowsSoundPlayer: new NumberSpinnerSoundPlayer( numberOfDataPointsProperty ),
         tandem: providedOptions.tandem.createTandem( 'numberOfBallsSpinner' )

js/common/SnackStacker.ts

@@ -7,7 +7,7 @@
  */
 
 import meanShareAndBalance from '../meanShareAndBalance.js';
-import { Image, Node } from '../../../scenery/js/imports.js';
+import { Image } from '../../../scenery/js/imports.js';
 import { SnackType } from './view/SharingScreenView.js';
 import MeanShareAndBalanceConstants from './MeanShareAndBalanceConstants.js';
 import Plate from './model/Plate.js';
@@ -41,34 +41,6 @@ class SnackStacker {
     }
   }
 
-  /**
-   * Position the graphical node that is being used to depict all or part of a snack.  This method is used to position
-   * the nodes that are *not* represented as images in the view.  The position is set in
-   */
-  public static setSnackGraphicPosition( snackNode: Node, snackType: SnackType, positionInStack: number ): void {
-
-    assert && assert( snackType === 'candyBars' || snackType === 'apples', 'unknown snack type' );
-
-    if ( snackType === 'candyBars' ) {
-
-      // The candy bar graphic Nodes are stacked in a single column with a little space between each.
-      snackNode.centerX = MeanShareAndBalanceConstants.CANDY_BAR_WIDTH / 2;
-      snackNode.centerY = -( MeanShareAndBalanceConstants.NOTEPAD_PLATE_DIMENSION.height +
-                             MeanShareAndBalanceConstants.CANDY_BAR_HEIGHT / 2 +
-                             positionInStack * ( MeanShareAndBalanceConstants.CANDY_BAR_HEIGHT +
-                             MeanShareAndBalanceConstants.NOTEPAD_CANDY_BAR_VERTICAL_SPACING ) );
-    }
-    else {
-
-      // The apples are stacked in two columns with some space between them in both the x and y dimensions.
-      snackNode.centerX = positionInStack % 2 === 0 ?
-                          MeanShareAndBalanceConstants.NOTEPAD_PLATE_DIMENSION.width / 2 - snackNode.width / 2 - HORIZONTAL_SPACE_BETWEEN_APPLES / 2 :
-                          MeanShareAndBalanceConstants.NOTEPAD_PLATE_DIMENSION.width / 2 + snackNode.width / 2 + HORIZONTAL_SPACE_BETWEEN_APPLES / 2;
-      snackNode.bottom = -Math.floor( positionInStack / 2 ) * ( snackNode.width + VERTICAL_SPACE_BETWEEN_APPLES ) -
-                         VERTICAL_SPACE_BETWEEN_APPLES;
-    }
-  }
-
   /**
    * Get the position for a candy bar model element that is stacked on a plate.  The position is based on where the
    * plate is in model space and the candy bar's position in the stack.

js/common/view/LabTableNode.ts

@@ -1,7 +1,7 @@
 // Copyright 2022-2024, University of Colorado Boulder
 
 /**
- * Shows the table at the bottom of the screen
+ * Shows the table at the bottom of the screen. A rectangle extends the bottom of the table in narrow aspect ratios.
  *
  * @author Marla Schulz (PhET Interactive Simulations)
  * @author Sam Reid (PhET Interactive Simulations)

js/common/view/MeanCalculationPanel.ts

@@ -35,8 +35,8 @@ export type MeanDisplayType = 'decimal' | 'mixedFraction' | 'remainder';
 
 type SelfOptions = {
 
-  // One of the items in this dialog can be displayed as either a decimal number or a mixed fraction, and this option
-  // controls which.
+  // One of the items in this dialog can be displayed as either a remainder, decimal number, and/or mixed fraction.
+  // This option controls which.
   calculatedMeanDisplayMode?: MeanDisplayType;
   zeroDataMessageProperty?: LocalizedStringProperty | null;
 };
@@ -215,7 +215,8 @@ export default class MeanCalculationPanel extends Panel {
         vinculumLineWidth: VINCULUM_LINE_WIDTH
       } );
 
-      // The value can be represented as either a decimal & mixed fraction, mixed fraction, or a whole number with a remainder.
+      // The value can be represented as either a decimal & mixed fraction, mixed fraction,
+      // or a whole number with a remainder.
       let valueRepresentation: Node | null = null;
       let decimalRepresentationText: Node | null = null;
       let decimalRepresentation: Node | null = null;
@@ -238,7 +239,7 @@ export default class MeanCalculationPanel extends Panel {
           vinculumLineWidth: VINCULUM_LINE_WIDTH
         } );
 
-
+        // Update the equal sign based on whether the mean is an integer or not.
         const equalSign = mean > Math.floor( mean ) ? '≈' : '=';
         const decimalTextPatternStringProperty = new PatternStringProperty(
           MeanShareAndBalanceStrings.meanEqualSignPatternStringProperty,

js/common/view/MeanPredictionSlider.ts

@@ -81,8 +81,6 @@ export default class MeanPredictionSlider extends AccessibleSlider( Node, 0 ) {
     const dragListener = new DragListener( {
       positionProperty: predictMeanPositionProperty,
       transform: modelViewTransform,
-
-      // phet-io
       tandem: options.tandem.createTandem( 'dragListener' )
     } );
 
@@ -117,6 +115,11 @@ export default class MeanPredictionSlider extends AccessibleSlider( Node, 0 ) {
     this.predictMeanHandle.touchArea = this.predictMeanHandle.localBounds.dilated( MeanShareAndBalanceConstants.TOUCH_AREA_DILATION );
   }
 
+  /**
+   * Update the length of the line based on the provided start and end x values.
+   * @param lineStart
+   * @param lineEnd
+   */
   public updateLine( lineStart: number, lineEnd: number ): void {
     const x1 = lineStart - LINE_X_MARGIN;
     const x2 = lineEnd + LINE_X_MARGIN * 2;

js/common/view/MeanShareAndBalanceCheckboxGroup.ts

@@ -1,7 +1,7 @@
 // Copyright 2022-2024, University of Colorado Boulder
 
 /**
- * A vertical checkbox group that controls visibility for predictMean, Mean, tickMarks, and cupWaterLevel.
+ * A vertical checkbox group that controls visibility for various sim features.
  *
  * @author Marla Schulz (PhET Interactive Simulations)
  */
@@ -45,8 +45,6 @@ export default class MeanShareAndBalanceCheckboxGroup extends VerticalCheckboxGr
         } ),
         property: options.predictMeanVisibleProperty,
         options: { accessibleName: MeanShareAndBalanceStrings.predictMeanStringProperty },
-
-        // phet-io
         tandemName: 'predictMeanCheckbox'
       } );
     }
@@ -58,12 +56,9 @@ export default class MeanShareAndBalanceCheckboxGroup extends VerticalCheckboxGr
         } ),
         property: options.tickMarksVisibleProperty,
         options: { accessibleName: MeanShareAndBalanceStrings.tickMarksStringProperty },
-
-        // phet-io
         tandemName: 'tickMarksCheckbox'
       } );
     }
-
     if ( options.totalVisibleProperty ) {
       checkboxItems.push( {
         createNode: () => new Text( MeanShareAndBalanceStrings.totalStringProperty, {
@@ -72,8 +67,6 @@ export default class MeanShareAndBalanceCheckboxGroup extends VerticalCheckboxGr
         } ),
         property: options.totalVisibleProperty,
         options: { accessibleName: MeanShareAndBalanceStrings.totalStringProperty },
-
-        // phet-io
         tandemName: 'totalCheckbox'
       } );
     }

js/common/view/MeanShareAndBalanceControls.ts

@@ -27,8 +27,11 @@ import WithRequired from '../../../../phet-core/js/types/WithRequired.js';
 
 type SelfOptions = {
   numberSpinnerOptions: WithRequired<NumberSpinnerOptions, 'tandem'>;
+
+  // Define the traversal order of controls.
   controlsPDOMOrder: Node[];
-  soccerContext?: boolean;
+
+  // When provided the info button will be displayed and will toggle the visibility of the info panel.
   infoPanelVisibleProperty?: Property<boolean> | null;
 };
 
@@ -51,7 +54,6 @@ export default class MeanShareAndBalanceControls extends Node {
   ) {
 
     const options = optionize<MeanShareAndBalanceControlsOptions, SelfOptions, NodeOptions>()( {
-      soccerContext: false,
       infoPanelVisibleProperty: null,
       isDisposable: false
     }, providedOptions );
@@ -112,6 +114,9 @@ export default class MeanShareAndBalanceControls extends Node {
   }
 }
 
+/**
+ * We are overriding the sound player for the info button to play the dialog open and close sounds.
+ */
 class InfoButtonSoundPlayer implements TSoundPlayer {
   public constructor( private readonly dialogVisibleProperty: Property<boolean> ) {
   }

js/common/view/MeanShareAndBalanceScreenView.ts

@@ -38,15 +38,12 @@ export default class MeanShareAndBalanceScreenView extends ScreenView {
 
     const controlsWidthOffset = ( MeanShareAndBalanceConstants.CONTROLS_PREFERRED_WIDTH +
                                   MeanShareAndBalanceConstants.CONTROLS_HORIZONTAL_MARGIN ) / 2;
-    this.playAreaCenterX = this.layoutBounds.centerX - controlsWidthOffset;
 
+    this.playAreaCenterX = this.layoutBounds.centerX - controlsWidthOffset;
     this.notepad = notepadNode;
-
     this.questionBar = new QuestionBar( this.layoutBounds, this.visibleBoundsProperty, {
       questionString: questionBarStringProperty,
       barFill: questionBarColor,
-
-      // phet-io
       tandem: options.tandem.createTandem( 'questionBar' )
     } );
 
@@ -58,8 +55,6 @@ export default class MeanShareAndBalanceScreenView extends ScreenView {
       },
       right: this.layoutBounds.maxX - MeanShareAndBalanceConstants.SCREEN_VIEW_X_MARGIN,
       bottom: this.layoutBounds.maxY - MeanShareAndBalanceConstants.SCREEN_VIEW_Y_MARGIN,
-
-      // phet-io
       tandem: options.tandem.createTandem( 'resetAllButton' )
     } );
 
@@ -71,9 +66,15 @@ export default class MeanShareAndBalanceScreenView extends ScreenView {
    * Resets the view.
    */
   public reset(): void {
-    // May be used for future screens
+    // Overwritten in subclasses.
   }
 
+  /**
+   * Set the traversal order for the screen.
+   * @param notepadInteractionNodes
+   * @param tableInteractionNodes
+   * @param controlNodes
+   */
   protected msabSetPDOMOrder( notepadInteractionNodes: Node[], tableInteractionNodes: Node[], controlNodes: Node[] ): void {
     this.pdomPlayAreaNode.setPDOMOrder( [
       ...notepadInteractionNodes,

js/common/view/NotepadNode.ts

@@ -41,6 +41,9 @@ const NOTEPAD_RING_BOTTOM = 33.5;
 const PAPER_PAGE_SIZE = new Dimension2( 720, 240 );
 const LABEL_MARGIN = 15;
 const TOTAL_MARGIN = 5;
+const PAPER_STACK_HEIGHT = 4;
+const STACK_OFFSET = 3;
+const NUMBER_OF_RINGS = 8;
 
 export default class NotepadNode extends Node {
 
@@ -66,12 +69,10 @@ export default class NotepadNode extends Node {
     const paperStackNode = new Node();
     const paperWidth = PAPER_PAGE_SIZE.width;
     const paperHeight = PAPER_PAGE_SIZE.height;
-    const paperStackHeight = 4;
-    const stackOffset = 3;
 
-    for ( let i = paperStackHeight; i > 0; i-- ) {
-      const xOffset = i * -stackOffset;
-      const yOffset = i * stackOffset;
+    for ( let i = PAPER_STACK_HEIGHT; i > 0; i-- ) {
+      const xOffset = i * -STACK_OFFSET;
+      const yOffset = i * STACK_OFFSET;
       const paper = new Rectangle( xOffset, yOffset, paperWidth, paperHeight, {
         fill: MeanShareAndBalanceColors.notepadColorProperty,
         stroke: 'black',
@@ -82,9 +83,8 @@ export default class NotepadNode extends Node {
 
     // Create the set of notebook ring images.
     const ringsNode = new Node();
-    const numberOfRings = 8;
-    _.times( numberOfRings, ( i: number ) => {
-      const x = i * ( ( paperWidth - 20 ) / numberOfRings ) + 30;
+    _.times( NUMBER_OF_RINGS, ( i: number ) => {
+      const x = i * ( ( paperWidth - 20 ) / NUMBER_OF_RINGS ) + 30;
       const image = new Image( notepadRing_svg, { x: x, bottom: NOTEPAD_RING_BOTTOM, maxHeight: 55 } );
       ringsNode.addChild( image );
     } );
@@ -97,10 +97,6 @@ export default class NotepadNode extends Node {
 
     // Make a copy of the paper stack bounds available to subclasses for positioning of child nodes.
     this.paperStackBounds = paperStackNode.bounds.copy();
-
-    // Make the rings node available to subclasses for layering adjustments.
-    this.ringsNode = ringsNode;
-
     this.readoutNode = null;
 
     if ( options.readoutPatternStringProperty ) {
@@ -129,6 +125,9 @@ export default class NotepadNode extends Node {
       this.addChild( readoutAlignBox );
       this.readoutNode = readoutAlignBox;
     }
+
+    // Make the rings node available to subclasses for layering adjustments.
+    this.ringsNode = ringsNode;
   }
 }
 

js/common/view/SharingControls.ts

@@ -28,6 +28,8 @@ import numberOfPlatesV6_mp3 from '../../../sounds/numberOfPlatesV6_mp3.js';
 
 type SelfOptions = {
   showSyncButton?: boolean;
+
+  // When the predictMeanVisibleProperty is provided the predict mean tool can be toggled on and off.
   predictMeanVisibleProperty?: Property<boolean> | null;
   vBoxOptions?: StrictOmit<VBoxOptions, 'children' | 'align'>;
 };

js/common/view/SharingScreenView.ts

@@ -2,10 +2,10 @@
 /**
  * The base screen view for the Distribute Screen and the Fair Share Screen.
  * Both screens have:
- * - A table with people, each of whom have a plate with a snack on them.
+ * - A table with people, each of whom have a plate with snack(s) on them.
  * - A notepad that also shows plates and snacks
- * - A mean calculation dialog that shows the mean calculation of the snacks in the play area
- * - A control panel that allows the user to change the number of people
+ * - A mean calculation info panel that shows the mean calculation of the snacks in the play area
+ * - A number spinner that allows the user to change the number of people
  *
  * @author Marla Schulz (PhET Interactive Simulations)
  *
@@ -54,7 +54,6 @@ export default class SharingScreenView<T extends Snack> extends MeanShareAndBala
 
   // Layers upon which other nodes will be placed.
   protected notepadSnackLayerNode: Node;
-  private readonly peopleLayerNode: Node;
 
   // Various nodes used to depict visual elements in the view.
   protected readonly tablePlateNodes: Node[];
@@ -100,7 +99,7 @@ export default class SharingScreenView<T extends Snack> extends MeanShareAndBala
       ...model.plates.map( plate => plate.tableSnackNumberProperty )
     ];
 
-    // Create the dialog that will show the various ways to calculate the mean.
+    // Create the info panel that will show the various ways to calculate the mean.
     const meanCalculationPanel = new MeanCalculationPanel(
       calculationDependencies,
       () => model.getActivePlates().map( plate => plate.tableSnackNumberProperty.value ),
@@ -143,7 +142,6 @@ export default class SharingScreenView<T extends Snack> extends MeanShareAndBala
     } );
 
     this.tablePlateNodes = tablePlateNodes;
-    this.peopleLayerNode = peopleLayerNode;
     this.notepadSnackLayerNode = notepadSnackLayerNode;
 
     // Don't include the questionBar in the usable bounds.
@@ -166,7 +164,7 @@ export default class SharingScreenView<T extends Snack> extends MeanShareAndBala
       this.interruptSubtreeInput();
     } );
 
-    // Add all the children.  This is done all at once so that the layering of apparent and easily adjusted.
+    // Add all the children.  This is done all at once so that the z-order is apparent and easily adjusted.
     const children = [
       notepadNode,
       peopleLayerNode,