search

phetsims / graphing-quadratics

"Graphing Quadratics" is an educational simulation in HTML5, by PhET Interactive Simulations.
MIT License
1 stars 4 forks source link

Use default layoutBounds #181

Closed pixelzoom closed 1 year ago

pixelzoom commented 1 year ago

Related to https://github.com/phetsims/joist/issues/542 ... This sim uses non-standard layoutBounds, because it is related to graph-lines, which was a Java port.

In GQConstants:

  // Custom layout bounds because this sim is part of the Graphing Lines family, which was ported to Java.
  // We used Java's layoutBounds in order to avoid changing sizes, fonts, etc. during the port.
  // The bounds with was modified to match the default HTML5 layoutBounds aspect ratio.
  // See https://github.com/phetsims/graphing-quadratics/issues/121
  SCREEN_VIEW_LAYOUT_BOUNDS: new Bounds2( 0, 0, 1160, 700 ),

Someday it should be converted to standard (default) layoutBounds, as specified in ScreenView.ts:

const DEFAULT_LAYOUT_BOUNDS = new Bounds2( 0, 0, 1024, 618 );
pixelzoom commented 1 year ago

Another thing to consider if we ever did this... Changing the layoutBounds will require changing the position of everything on the screen. For anything that has a postionProperty, it's value will need to change. That will create serious problems for PhET-iO clients who have set custom values of positionProperty, and it's unlikely to be something that can be addressed by migration rules.

pixelzoom commented 1 year ago

graphing-quadratics has already been published for PhET-iO. So we can't change the layoutBounds at this point. Noted in GLConstants.ts. Closing.

phet-dev commented 1 year ago

Reopening because there is a TODO marked for this issue.

phet-dev commented 1 year ago

Reopening because there is a TODO marked for this issue.

pixelzoom commented 1 year ago

I deleted this vestigial TODO in the main branch yesterday, see https://github.com/phetsims/graphing-quadratics/commit/6e89b0f6e13205b0883ebd5adc0e4c8499a8129f. I suspect that the automated task is still looking at master. I've asked the "master => main" team to have a look.

pixelzoom commented 1 year ago

In Slack#developer, @zepumph said:

Lol! Sorry, the process was broken and while I was debugging it, I ran it on stale shas. It is fixed now. Sorry for the bother.