phetsims / projectile-data-lab

"Projectile Data Lab" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 0 forks source link

Unnecessary `{}` argument for providedOptions #218

Closed pixelzoom closed 5 months ago

pixelzoom commented 5 months ago

For code review #32 ...

There are quite a few places where an unnecessary {} is provides as the argument for providedOptions, which is either optional or should be optional.

    const histogramSoundIconToggleNode = new ToggleNode<boolean, Node>( new DerivedProperty( [
...
    } ], {} );
    const chartCanvasNode = new ChartCanvasNode( chartTransform, [ histogramPainter ], {} );
    const launchIconToggleNode = new ToggleNode<'single' | 'continuous', Image>( launchModeProperty, [ {
...
43    } ], {} );

    const launchButtonToggleNode = new ToggleNode<boolean, Node>( new DerivedProperty( [
 ...
60    } ], {} );

   const launchButtonWithAutogenerateToggleNode = new ToggleNode<boolean, Node>( ...
68    } ], {} );
    const fieldOverlayBack = new FieldOverlayNode( this.modelViewTransform, {} );
    const noDataLabel = new PDLText( ProjectileDataLabStrings.noDataStringProperty, {} );
    this.launcherNode = new CustomLauncherNode(
...
      {}
    );
    this.launcherNode = new CustomLauncherNode(
...
      {}
    );
pixelzoom commented 5 months ago

In https://github.com/phetsims/projectile-data-lab/commit/4a4af06cd50ee169929f9d3f5a5375dadd786a45, I addressed all of the cases expect for MeasuresScreenView.ts and SourcesScreenView.ts, in case you want to apply the patch provided in https://github.com/phetsims/projectile-data-lab/issues/217.

For FieldOverlayNode, I made the isLeftSide option required, because I think it makes the call site in PDLScreenView clearer:

    const fieldOverlayBack = new FieldOverlayNode( this.modelViewTransform, { isLeftSide: false } );
    const fieldOverlayFront = new FieldOverlayNode( this.modelViewTransform, { isLeftSide: true } );

In https://github.com/phetsims/projectile-data-lab/commit/b99f117c93675a1030bd38b2a550d16778580ea0, I fixed an incorrect options pattern for CustomLauncherNodeOptions, and made providedOptions optional. So you should be able to simply remove the {} from MeasuresScreenView.ts and SourcesScreenView.ts.

Over to @samreid or @matthew-blackman to review and complete work.

matthew-blackman commented 5 months ago

I removed the remaining instances of empty providedOptions. Thanks for the review feedback and fix @pixelzoom! Closing.