Add code review comments, see: #109

js/common/view/QuantumMeasurementHistogram.ts

@@ -157,7 +157,7 @@ class QuantumMeasurementHistogram extends Node {
 
       const fractionFont = new PhetFont( { size: 15, weight: 'bold' } );
 
-      // Assume no one will every have the patience to shoot more than 1000 photons.
+      // Assume no one will ever have the patience to shoot more than 1000 photons.
       const fractionTermsRange = new Range( 0, 999 );
 
       leftNumberDisplay = new NumberDisplay(

js/common/view/SceneSelectorRadioButtonGroup.ts

@@ -30,6 +30,7 @@ class SceneSelectorRadioButtonGroup<T extends string> extends RectangularRadioBu
                       valueToStringMap: Map<T, LocalizedStringProperty>,
                       providedOptions: SceneSelectorRadioButtonGroupOptions ) {
 
+    // REVIEW: This assertion is odd... why are you not able to have multiple radio buttons if the Property supports it?
     assert && assert( valueToStringMap.size === 2, 'SceneSelectorRadioButtonGroup requires exactly two items' );
 
     const options = optionize<SceneSelectorRadioButtonGroupOptions, SelfOptions, RectangularRadioButtonGroupOptions>()( {

js/photons/view/ExpectationValueControl.ts

@@ -2,7 +2,7 @@
 
 /**
  * ExpectationValueControl is a view element that allows the user to control the visibility of the expectation value
- * line in the normalized outcome vector graph.  It also displays, under the right conditions, the expectation value.
+ * line in the normalized outcome vector graph. It also displays, under the right conditions, the expectation value.
  *
  * @author John Blanco, PhET Interactive Simulations
  */

js/photons/view/ExpectationValueVectorControl.ts

@@ -2,7 +2,7 @@
 
 /**
  * ExpectationValueVectorControl is a view element that allows the user to control the visibility of the expectation
- * value vector in another view component.  It is basically just a checkbox with an arrow that represents the vector.
+ * value vector in another view component. It is basically just a checkbox with an arrow that represents the vector.
  *
  * @author John Blanco, PhET Interactive Simulations
  */

js/photons/view/FlatPolarizationAngleIndicator.ts

@@ -123,6 +123,8 @@ export default class FlatPolarizationAngleIndicator extends Node {
       children: [ thetaNode, angleReadoutText ],
       centerX: horizontalAxis.centerX + 40,
       centerY: verticalAxis.top,
+
+      // REVIEW: use DerivedProperty.valueNotEqualsConstant
       visibleProperty: new DerivedProperty( [ polarizationAngleProperty ], angle => angle !== null )
     } );
 

js/photons/view/LaserNode.ts

@@ -1,7 +1,7 @@
 // Copyright 2024-2025, University of Colorado Boulder
 
 /**
- * LaserNode represents the laser, which emits photons, in the view.  It allows the user to set the rate at which
+ * LaserNode represents the laser, which emits photons, in the view. It allows the user to set the rate at which
  * photons are produced, or to produce them one at a time.
  *
  * @author John Blanco, PhET Interactive Simulations
@@ -75,7 +75,7 @@ export default class LaserNode extends Node {
 
     const nodeChildren: Node[] = [ laserPointerNode, caption ];
 
-    // If the laser is in many-photon mode, we need a slider to control the rate of photon emission.  And a label for
+    // If the laser is in many-photon mode, we need a slider to control the rate of photon emission. And a label for
     // the rate.
     if ( model.emissionMode === 'manyPhotons' ) {
       const emissionRateSlider = new HSlider( model.emissionRateProperty, model.emissionRateProperty.range, {

js/photons/view/MirrorNode.ts

@@ -29,6 +29,8 @@ export default class MirrorNode extends Node {
                       modelViewTransform: ModelViewTransform2,
                       providedOptions?: MirrorNodeOptions ) {
 
+    // REVIEW: This seems like it could break if the photons change size at all. Is there any way to calculate this offset
+    // based on your photon dimensions?
     // Define an offset for positioning the mirror.  This is needed because the photons reflect based on their center
     // positions, and if the mirror isn't offset a bit, the photons can appear to go partially through the mirror.
     // The value is in screen coordinates and is empirically determined.  This only works for a mirror that is oriented

js/photons/view/NormalizedOutcomeVectorGraph.ts

@@ -104,6 +104,8 @@ export default class NormalizedOutcomeVectorGraph extends Node {
       [ normalizedOutcomeValueProperty ],
       value => -value
     );
+
+    // REVIEW: Suggested to rename showNumericValueProperty to numericValueVisibleProperty
     const showNumericValueProperty = DerivedProperty.and( [ showVectorProperty, displayNumericValueProperty ] );
     const normalizedOutcomeValueDisplay = new NumberDisplay(
       invertedNormalizedOutcomeValueProperty,

js/photons/view/ObliquePolarizationAngleIndicator.ts

@@ -63,7 +63,7 @@ const UNIT_LENGTH = AXIS_LENGTH * 0.75;
 const PROJECTION_ANGLE_IN_DEGREES = 45;
 const PROJECTION_ANGLE_IN_RADIANS = Utils.toRadians( PROJECTION_ANGLE_IN_DEGREES );
 
-// Define a function that projects a 3D point into the 2D plane of the screen.  This uses what is called a "cabinet
+// Define a function that projects a 3D point into the 2D plane of the screen. This uses what is called a "cabinet
 // projection" in engineering drawing, which is a specific type of oblique projection.
 const project3Dto2D = ( x: number, y: number, z: number ): Vector2 => {
   return new Vector2(
@@ -77,7 +77,7 @@ export default class ObliquePolarizationAngleIndicator extends Node {
   public constructor( polarizationAngleProperty: TReadOnlyProperty<number | null>,
                       providedOptions?: PolarizationPlaneRepresentationOptions ) {
 
-    // Create the Y axis line with an arrow head.  This is pointing directly to the right.  We have to do this as a
+    // Create the Y axis line with an arrow head. This is pointing directly to the right.  We have to do this as a
     // separate arrow head and line because the line has a dashed pattern, which doesn't work with ArrowNode.
     const yAxisArrowHead = new ArrowNode( 0.9 * AXIS_LENGTH, 0, AXIS_LENGTH, 0, AXIS_OPTIONS );
     const yAxisLine = new Line( 0, 0, AXIS_LENGTH, 0, {
@@ -128,6 +128,9 @@ export default class ObliquePolarizationAngleIndicator extends Node {
     } );
 
     // Create a Property for the fill used for the unit circle, since it changes when the photons are unpolarized.
+    //REVIEW: Technically `QuantumMeasurementColors.photonBaseColorProperty` should be part of what the DerivedProperty
+    // is listening to. I think the only scenario where this would be buggy is in the Color Editor, but could also cause
+    // a bug in PhET-iO if we gave clients the option to adjust colorProperties.
     const unitCircleFillProperty = new DerivedProperty(
       [ polarizationAngleProperty ],
       polarizationAngle => polarizationAngle === null ?

js/photons/view/PhotonDetectionProbabilityPanel.ts

@@ -49,14 +49,14 @@ export default class PhotonDetectionProbabilityPanel extends AccordionBox {
       expandedProperty: expandedProperty
     }, providedOptions );
 
-    // Calculate the probability of a photon being detected as horizontally polarized.  A null value indicates that the
+    // Calculate the probability of a photon being detected as horizontally polarized. A null value indicates that the
     // probability is unknown.
     const probabilityOfHorizontalProperty = new DerivedProperty(
       [ polarizationAngleProperty ],
       polarizationAngle => polarizationAngle === null ? null : Math.cos( Utils.toRadians( polarizationAngle ) ) ** 2
     );
 
-    // Calculate the probability of a photon being detected as vertically polarized.  A null value indicates that the
+    // Calculate the probability of a photon being detected as vertically polarized. A null value indicates that the
     // probability is unknown.
     const probabilityOfVerticalProperty = new DerivedProperty(
       [ probabilityOfHorizontalProperty ],

js/photons/view/PhotonDetectorNode.ts

@@ -151,6 +151,7 @@ export default class PhotonDetectorNode extends Node {
       } );
     }
     else {
+      //REVIEW: Can this if/elseif/else block be reduced to just an if/else? Why do you need to handle an unsupported detection direction?
       assert && assert( false, `unsupported detection direction: ${model.detectionDirection}` );
       label = new RichText( '' );
     }

js/photons/view/PhotonPolarizationAngleControl.ts

@@ -135,6 +135,8 @@ export default class PhotonPolarizationAngleControl extends Panel {
     // Create a slider to control the custom angle of polarization.  It is only visible when the custom preset value
     // is selected.
     const customAngleSlider = new HSlider( photonSource.customPolarizationAngleProperty, new Range( 0, 90 ), {
+
+      // REVIEW: Use DerivedProperty.valueEqualsConstant
       visibleProperty: new DerivedProperty(
         [ photonSource.presetPolarizationDirectionProperty ],
         value => value === 'custom'
@@ -184,7 +186,7 @@ export default class PhotonPolarizationAngleControl extends Panel {
       tandem: providedOptions.tandem.createTandem( 'polarizationIndicator' )
     } );
 
-    // Put the left portion of the panel into and HBox with the polarization indicator.
+    // Put the left portion of the panel into an HBox with the polarization indicator.
     const content = new HBox( {
       children: [ leftPortionOfPanel, polarizationIndicator ]
     } );

js/photons/view/PhotonSprites.ts

@@ -1,16 +1,16 @@
 // Copyright 2024-2025, University of Colorado Boulder
 
 /**
- * PhotonSprites is a class that can be used to perform high-performance rendering of a set of photons.  It uses
- * scenery's Sprites feature, which uses renderer:'webgl', with a fallback of 'canvas'.  The photons support multiple
- * position states to allow for rendering the probability of a photon being in different positions.  To support this,
+ * PhotonSprites is a class that can be used to perform high-performance rendering of a set of photons. It uses
+ * scenery's Sprites feature, which uses renderer:'webgl', with a fallback of 'canvas'. The photons support multiple
+ * position states to allow for rendering the probability of a photon being in different positions. To support this,
  * the photons are rendered with a solid exterior and an interior that varies in opacity based on the probability of the
  * photon being in that position.
  *  *
  * Understanding this implementation requires an understanding of the scenery Sprites API. In a nutshell: Sprites has an
  * array of Sprite and an array of SpriteInstance. The array of Sprite is the complete unique set of images used to
  * render all SpriteInstances. Each SpriteInstance has a reference to a Sprite (which determines what it looks like) and
- * a Matrix3 (which determines how it's transformed).  At each model step, the positions of the PhotonInstance instances
+ * a Matrix3 (which determines how it's transformed). At each model step, the positions of the PhotonInstance instances
  * are updated by adjusting their matrix, and then invalidatePaint is called to re-render the sprites.
  *
  * @author John Blanco (PhET Interactive Simulations)
@@ -50,7 +50,7 @@ class PhotonSprites extends Sprites {
     const spriteInstances: SpriteInstance[] = [];
 
     // Invoke the superclass constructor with no sprites because creating some of the sprites is an asynchronous
-    // process.  The sprites are added below.
+    // process. The sprites are added below.
     super( {
       sprites: [],
       spriteInstances: spriteInstances,

js/photons/view/PhotonTestingArea.ts

@@ -51,6 +51,8 @@ class PhotonTestingArea extends Node {
 
     const particleBehaviorModeRadioButtonGroupTandem = providedOptions.tandem.createTandem( 'particleBehaviorModeRadioButtonGroup' );
     const particleBehaviorModeRadioButtonGroup = new AquaRadioButtonGroup<SystemType>(
+
+      // REVIEW: Why do you need this type assertion?
       model.particleBehaviorModeProperty as PhetioProperty<SystemType>,
       SystemTypeValues.map( behaviorMode => {