search

phetsims / arithmetic

"Arithmetic" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/arithmetic
GNU General Public License v3.0
5 stars 5 forks source link

Consider converting to default layoutBounds before PhET-iO publication #197

Closed pixelzoom closed 6 months ago

pixelzoom commented 1 year ago

This sim currently uses non-default layoutBounds. In ArithmeticView.js:

    super( { layoutBounds: new Bounds2( 0, 0, 768, 504 ) } );

Per https://github.com/phetsims/phet-io/issues/1939... layoutBounds should not be changed for published PhET-iO sims because doing so may disrupt client customizations. So evaluate whether to change layoutBounds before publishing a PhET-iO version.

If you decide not to change layoutBounds, document why in ArithmeticView.js, and refer to this issue.

marlitas commented 7 months ago

hmmm... I am not sure if this should be part of the region and culture work. It seems like non-default layoutBounds is odd in general, so I'm inclined to try the default bounds out and see what happens...

AgustinVallejo commented 7 months ago

Same as in https://github.com/phetsims/balancing-act/issues/109, removed the custom layoutBounds. Assigning back to review

marlitas commented 6 months ago

Hmmm. Okay this does change a lot of the sizing of stuff which I think will need to be readjusted to match the published version... I'm not sure if it's worth the time to do that or if that's within the scope of this publication. I don't think I fully understand the pros and cons of switching over to the default layout bounds from a maintenance and user perspective...

@pixelzoom can you provide more context on any compelling reasons for why this work should be prioritized? Or is this only worth it for PhET-iO?

marlitas commented 6 months ago

From Dev Meeting today it was decided that we would like to close all these issues as won't do. Before doing so we will need to revert the commits.

pixelzoom commented 6 months ago

To avoid having to revisit this topic in the future, and to make it clear to future maintainers, I recommend a code comment wherever non-default layoutBounds are used.

For sims that have already been published with PhET-iO, something like this in BLLConstants.ts:

  // While these layoutBounds differ from the default, PhET-iO customizations may rely on these bounds.
  // So do not change. See https://github.com/phetsims/beers-law-lab/issues/289.
  LAYOUT_BOUNDS: new Bounds2( 0, 0, 1100, 700 ),

Or for non-PhET-iO sims, something as simple as this in RAPConstants.ts:

  SCREEN_VIEW_LAYOUT_BOUNDS: new Bounds2( 0, 0, 835, 504 ), //TODO https://github.com/phetsims/reactants-products-and-leftovers/issues/75
AgustinVallejo commented 6 months ago

Changes reverted. Feel free to review and close @marlitas

marlitas commented 6 months ago

Sounds good let's close!