Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use default LayoutBounds #178

Open
zepumph opened this issue Sep 26, 2019 · 1 comment
Open

Use default LayoutBounds #178

zepumph opened this issue Sep 26, 2019 · 1 comment

Comments

@zepumph
Copy link
Member

zepumph commented Sep 26, 2019

In GFLBScreenView.js. May be similar to phetsims/coulombs-law#111 and phetsims/gravity-force-lab#179.

@zepumph zepumph self-assigned this Sep 26, 2019
@zepumph
Copy link
Member Author

zepumph commented Oct 16, 2019

This was a bit harder than I thought it would be. This is a patch that will inform my work next time. I couldn't get the model to recognize the new layout positions (there was overlap in the drawn nodes).

I also found that you could get pretty good sizing by applying a scale of 1.33 to all view elements, but that bypassed the model-view-transform. Also in general that felt like a code smell.

There were many hard coded, "empirically determined," values that needed changing. to do that I used the following functions:

> x = x=> x/768*1024
> y = y=> y/464*618
Index: inverse-square-law-common/js/view/ISLCForceArrowNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- inverse-square-law-common/js/view/ISLCForceArrowNode.js	(revision 0bb4c47db9bfcde172afe3a8816a87006cfb13ef)
+++ inverse-square-law-common/js/view/ISLCForceArrowNode.js	(date 1571177743879)
@@ -29,8 +29,8 @@
   const forceOnObjectByOtherObjectWithUnitsPatternString = require( 'string!INVERSE_SQUARE_LAW_COMMON/forceOnObjectByOtherObjectWithUnitsPattern' );
 
   // constants
-  const ARROW_LENGTH = 8; // empirically determined
-  const TEXT_OFFSET = 10; // empirically determined to make sure text does not go out of bounds
+  const ARROW_LENGTH = 10.66; // empirically determined
+  const TEXT_OFFSET = 13.3; // empirically determined to make sure text does not go out of bounds
 
   /**
    * @param {Range} arrowForceRange - the range in force magnitude
@@ -55,13 +55,13 @@
       forceReadoutDecimalPlaces: 12, // number of decimal places in force readout
 
       // arrow node arguments
-      forceArrowHeight: 150,
+      forceArrowHeight: 200,
 
       // arrow node options
-      maxArrowWidth: 15, // max width of the arrow when when redrawn, in view coordinates - used in mapping function
+      maxArrowWidth: 20, // max width of the arrow when when redrawn, in view coordinates - used in mapping function
       minArrowWidth: 0, // Some ISLC sims support an object value of zero, setting this to zero supports this case.
-      headHeight: 8,
-      headWidth: 8,
+      headHeight: 10.6,
+      headWidth: 10.6,
       tailWidth: 3,
       arrowStroke: null,
       arrowFill: '#fff',
@@ -121,8 +121,8 @@
       fill: options.arrowLabelFill,
       stroke: options.labelStroke,
       lineWidth: options.arrowNodeLineWidth,
-      maxWidth: 300, // empirically determined through testing with long strings
-      y: -20,
+      maxWidth: 400, // empirically determined through testing with long strings
+      y: -26,
       tandem: tandem.createTandem( 'forceText' ),
       phetioDocumentation: 'This text updates from the model as the force changes, and cannot be edited.',
       phetioComponentOptions: {
@@ -131,7 +131,7 @@
     } );
 
     // @private - tip and tail set in redrawArrow
-    this.arrow = new ArrowNode( 0, -options.forceArrowHeight, 200, -options.forceArrowHeight, _.extend( {
+    this.arrow = new ArrowNode( 0, -options.forceArrowHeight, 266, -options.forceArrowHeight, _.extend( {
       lineWidth: options.arrowNodeLineWidth,
       stroke: options.arrowStroke,
       fill: options.arrowFill,
@@ -141,7 +141,7 @@
     Node.call( this, options );
 
     // @private
-    this.arrowTextBackground = new Rectangle( 0, 0, 1000, 1000, { fill: options.backgroundFill, opacity: .3 } );
+    this.arrowTextBackground = new Rectangle( 0, 0, 1333, 1331, { fill: options.backgroundFill, opacity: .3 } );
     this.addChild( this.arrowTextBackground );
 
     this.addChild( this.arrowText );
Index: inverse-square-law-common/js/view/ISLCObjectNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- inverse-square-law-common/js/view/ISLCObjectNode.js	(revision 0bb4c47db9bfcde172afe3a8816a87006cfb13ef)
+++ inverse-square-law-common/js/view/ISLCObjectNode.js	(date 1571177769097)
@@ -42,7 +42,7 @@
   const POSITIVE_FILL = new Color( '#f66' );
   const ZERO_FILL = new Color( 'gray' );
 
-  const LABEL_MAX_WIDTH = 50; // empirically determined through testing with long strings
+  const LABEL_MAX_WIDTH = 66.6; // empirically determined through testing with long strings
   const LABEL_CENTER_X = 0;
 
   /**
@@ -72,7 +72,7 @@
       arrowColor: '#66f', // color of vertical line
       y: 250,
 
-      forceArrowHeight: 150, // height of arrow in view coordinates
+      forceArrowHeight: 200, // length of arrow in view coordinates
 
       // phet-io
       tandem: Tandem.required,
@@ -118,12 +118,9 @@
 
     assert && assert( config.label, 'required param' );
     assert && assert( config.otherObjectLabel, 'required param' );
-    assert && assert( alertManager instanceof ISLCAlertManager );
+    assert && assert( alertManager instanceof ISLCAlertManager );assert && assert( !config.containerTagName, 'ISLCObjectNode sets its own containerTagName' );
 
-    Node.call( this, {
-      containerTagName: 'div',
-      tandem: config.tandem
-    } );
+    Node.call( this, merge( { containerTagName: 'div' }, config ) );
 
     this.accessibleName = PositionDescriber.getObjectLabelPositionText( config.label );
 
Index: inverse-square-law-common/js/view/ISLCCheckboxPanel.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- inverse-square-law-common/js/view/ISLCCheckboxPanel.js	(revision 0bb4c47db9bfcde172afe3a8816a87006cfb13ef)
+++ inverse-square-law-common/js/view/ISLCCheckboxPanel.js	(date 1571178015715)
@@ -33,15 +33,15 @@
 
         checkboxGroupOptions: {
           checkboxOptions: {
-            spacing: 10,
-            padding: 8,
-            boxWidth: 16
+            spacing: 13,
+            padding: 10.6,
+            boxWidth: 21.3
           }
         },
         fill: '#FDF498',
-        xMargin: 10,
-        yMargin: 10,
-        minWidth: 170,
+        xMargin: 13,
+        yMargin: 13,
+        minWidth: 226,
 
         // phet-io
         tandem: Tandem.required
@@ -52,8 +52,8 @@
         assert && assert( item.tandem );
         item.node = new Text( item.label, {
           tandem: item.tandem.createTandem( 'labelNode' ),
-          font: new PhetFont( 14 ),
-          maxWidth: 125
+          font: new PhetFont( 18.66 ),
+          maxWidth: 166
         } );
         return item;
       } );
Index: gravity-force-lab-basics/js/gravity-force-lab-basics/view/GFLBMassNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- gravity-force-lab-basics/js/gravity-force-lab-basics/view/GFLBMassNode.js	(revision 8192f36c8490e77fad5ecba2a766bc24809e5282)
+++ gravity-force-lab-basics/js/gravity-force-lab-basics/view/GFLBMassNode.js	(date 1571177593355)
@@ -16,7 +16,7 @@
   const Tandem = require( 'TANDEM/Tandem' );
 
   // constants
-  const MASS_NODE_Y_POSITION = 215;
+  const MASS_NODE_Y_POSITION = 286;
 
   class GFLBMassNode extends MassNode {
 
@@ -37,7 +37,7 @@
           forceReadoutDecimalPlaces: 1,
           arrowFill: 'black',
           arrowLabelFill: 'black',
-          maxArrowWidth: 400,
+          maxArrowWidth: 533,
           forceThresholdPercent: 7 * Math.pow( 10, -4 ),
           backgroundFill: GFLBConstants.BACKGROUND_COLOR_PROPERTY
         },
@@ -53,7 +53,9 @@
         tandem: Tandem.required,
 
         // a11y recompute the PDOM descriptions when show distance is toggled
-        additionalA11yDependencies: [ model.showDistanceProperty ]
+        additionalA11yDependencies: [ model.showDistanceProperty ],
+
+        scale: 1.33
       }, options );
 
       super( model, mass, layoutBounds, modelViewTransform, alertManager, forceDescriber, positionDescriber, options );
Index: gravity-force-lab-basics/js/gravity-force-lab-basics/view/GFLBScreenView.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- gravity-force-lab-basics/js/gravity-force-lab-basics/view/GFLBScreenView.js	(revision 8192f36c8490e77fad5ecba2a766bc24809e5282)
+++ gravity-force-lab-basics/js/gravity-force-lab-basics/view/GFLBScreenView.js	(date 1571178051226)
@@ -11,7 +11,6 @@
 
   // modules
   const AccessiblePeer = require( 'SCENERY/accessibility/AccessiblePeer' );
-  const Bounds2 = require( 'DOT/Bounds2' );
   const CheckboxSoundGenerator = require( 'TAMBO/sound-generators/CheckboxSoundGenerator' );
   const Color = require( 'SCENERY/util/Color' );
   const DefaultDirection = require( 'INVERSE_SQUARE_LAW_COMMON/view/DefaultDirection' );
@@ -50,8 +49,8 @@
   const Vector2 = require( 'DOT/Vector2' );
 
   // constants
-  const MASS_CONTROLS_Y_POSITION = 385;
-  const PANEL_SPACING = 50;
+  const MASS_CONTROLS_Y_POSITION = 512;
+  const PANEL_SPACING = 67;
   const SHOW_GRID = ISLCQueryParameters.showGrid;
   const SHOW_DRAG_BOUNDS = ISLCQueryParameters.showDragBounds;
   const OBJECT_ONE = ISLCObjectEnum.OBJECT_ONE;
@@ -98,7 +97,6 @@
       const alertManager = new GFLBAlertManager( model, massDescriber, forceDescriber );
 
       super( {
-        layoutBounds: new Bounds2( 0, 0, 768, 464 ),
         screenSummaryContent: new GravityForceLabScreenSummaryNode( model, massDescriber, forceDescriber, positionDescriber, {
           mainDescriptionContent: screenSummaryMainDescriptionString,
           secondaryDescriptionContent: screenSummarySecondaryDescriptionString,
@@ -113,7 +111,7 @@
       const modelViewTransform = ModelViewTransform2.createSinglePointScaleInvertedYMapping(
         Vector2.ZERO,
         new Vector2( this.layoutBounds.width / 2, this.layoutBounds.height / 2 ),
-        .05
+        .04
       );
 
       // add the mass nodes to the view
@@ -124,7 +122,7 @@
           otherObjectLabel: mass2LabelString,
           defaultDirection: DefaultDirection.LEFT,
           arrowColor: '#66F',
-          forceArrowHeight: 125,
+          forceArrowHeight: 166.5,
           tandem: tandem.createTandem( 'mass1Node' )
         } );
 
@@ -135,7 +133,7 @@
           otherObjectLabel: mass1LabelString,
           defaultDirection: DefaultDirection.RIGHT,
           arrowColor: '#F66',
-          forceArrowHeight: 175,
+          forceArrowHeight: 233,
           tandem: tandem.createTandem( 'mass2Node' )
         } );
 
@@ -295,7 +293,7 @@
       // arrow that shows distance between the two masses
       const distanceArrowNode = new DistanceArrowNode( model, modelViewTransform, {
         tandem: tandem.createTandem( 'distanceArrowNode' ),
-        y: 145
+        y: 193
       } );
       model.showDistanceProperty.linkAttribute( distanceArrowNode, 'visible' );
       massPositionsNode.addChild( distanceArrowNode );
@@ -308,9 +306,10 @@
           mass2Node.reset();
           this.forceSoundGenerator.reset();
         },
-        right: this.layoutBounds.maxX - 10,
-        bottom: this.layoutBounds.maxY - 10,
-        tandem: tandem.createTandem( 'resetAllButton' )
+        right: this.layoutBounds.maxX - 13.3,
+        bottom: this.layoutBounds.maxY - 13.3,
+        tandem: tandem.createTandem( 'resetAllButton' ),
+        scale: 1.33
       } );
 
       soundManager.addSoundGenerator( new ResetAllSoundGenerator( model.resetInProgressProperty, {
@@ -331,14 +330,14 @@
       ];
 
       // layout the view elements
-      parameterControlPanel.right = this.layoutBounds.width - 15;
+      parameterControlPanel.right = this.layoutBounds.width - 20;
       parameterControlPanel.bottom = MASS_CONTROLS_Y_POSITION;
 
       massControlBox.right = parameterControlPanel.left - PANEL_SPACING;
       massControlBox.top = parameterControlPanel.top;
 
       resetAllButton.right = parameterControlPanel.right;
-      resetAllButton.top = parameterControlPanel.bottom + 13.5;
+      resetAllButton.top = parameterControlPanel.bottom + 18;
 
       //------------------------------------------------
       // debugging
Index: gravity-force-lab-basics/js/gravity-force-lab-basics/view/GFLBMassControl.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- gravity-force-lab-basics/js/gravity-force-lab-basics/view/GFLBMassControl.js	(revision 8192f36c8490e77fad5ecba2a766bc24809e5282)
+++ gravity-force-lab-basics/js/gravity-force-lab-basics/view/GFLBMassControl.js	(date 1571177943724)
@@ -26,8 +26,8 @@
   const billionKgString = require( 'string!GRAVITY_FORCE_LAB_BASICS/billionKg' );
 
   // constants
-  const MIN_PANEL_WIDTH = 150;
-  const MAX_TEXT_WIDTH = 120; // i18n
+  const MIN_PANEL_WIDTH = 200;
+  const MAX_TEXT_WIDTH = 160; // i18n
   const BILLION_MULTIPLIER = GFLBConstants.BILLION_MULTIPLIER;
 
   class GFLBMassControl extends Panel {
@@ -50,14 +50,14 @@
       }, options );
 
       const titleText = new Text( titleString, {
-        font: new PhetFont( 18 ),
+        font: new PhetFont( 24 ),
         maxWidth: MAX_TEXT_WIDTH,
         tandem: tandem.createTandem( 'titleText' )
       } );
 
       const numberPicker = new NumberPicker( valueProperty, new Property( massRange ), {
         font: new PhetFont( 20 ),
-        scale: 1.5,
+        scale: 2,
         tandem: tandem.createTandem( 'numberPicker' ),
         upFunction: mass => mass + BILLION_MULTIPLIER,
         downFunction: mass => mass - BILLION_MULTIPLIER,
@@ -75,27 +75,27 @@
         a11yCreateAriaValueText: () => massDescriber.getMassAndUnit( thisObjectEnum )
       } );
       const numberPickerLabel = new Text( billionKgString, {
-        font: new PhetFont( { size: 14 } ),
+        font: new PhetFont( { size: 18.66 } ),
         maxWidth: MAX_TEXT_WIDTH,
         tandem: tandem.createTandem( 'numberPickerLabel' )
       } );
 
       const numberPickerHBox = new HBox( {
         children: [ numberPicker, numberPickerLabel ],
-        spacing: 10
+        spacing: 13
       } );
 
       const panelVBox = new VBox( {
         children: [ titleText, numberPickerHBox ],
-        spacing: 10
+        spacing: 13
       } );
 
       titleText.on( 'text', () => { titleText.centerX = panelVBox.centerX; } );
 
       super( panelVBox, {
         fill: '#f1f1f2',
-        xMargin: 15,
-        yMargin: 10,
+        xMargin: 26,
+        yMargin: 13,
         minWidth: MIN_PANEL_WIDTH,
         resize: false,
         align: 'center',

@zepumph zepumph removed their assignment Oct 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant