From df2d4b813717d24b1fe3ca419fc03d6190fb40af Mon Sep 17 00:00:00 2001 From: pixelzoom Date: Tue, 7 Dec 2021 15:11:06 -0700 Subject: [PATCH] clean up how ruler visibility is handled, https://github.com/phetsims/geometric-optics/issues/263 --- js/common/view/GeometricOpticsRulerNode.ts | 4 +- js/common/view/RulersToolbox.ts | 67 +++++++++++----------- 2 files changed, 36 insertions(+), 35 deletions(-) diff --git a/js/common/view/GeometricOpticsRulerNode.ts b/js/common/view/GeometricOpticsRulerNode.ts index 1318d7bb..69fe6531 100644 --- a/js/common/view/GeometricOpticsRulerNode.ts +++ b/js/common/view/GeometricOpticsRulerNode.ts @@ -33,7 +33,7 @@ const MINIMUM_VISIBLE_LENGTH = GeometricOpticsConstants.RULER_MINIMUM_VISIBLE_LE class GeometricOpticsRulerNode extends Node { - private readonly ruler: GeometricOpticsRuler; + readonly ruler: GeometricOpticsRuler; private toolboxBounds: Bounds2; private readonly dragListener: DragListener; @@ -131,7 +131,7 @@ class GeometricOpticsRulerNode extends Node { // Return ruler to toolbox if the pointer is within the toolbox. assert && assert( event.pointer.point instanceof Vector2 ); if ( this.toolboxBounds.containsPoint( this.globalToParentPoint( event.pointer.point as Vector2 ) ) ) { - this.visible = false; + ruler.visibleProperty.value = false; } } } ); diff --git a/js/common/view/RulersToolbox.ts b/js/common/view/RulersToolbox.ts index 90990d08..55fff5b4 100644 --- a/js/common/view/RulersToolbox.ts +++ b/js/common/view/RulersToolbox.ts @@ -16,6 +16,9 @@ import Panel from '../../../../sun/js/Panel.js'; import Tandem from '../../../../tandem/js/Tandem.js'; import geometricOptics from '../../geometricOptics.js'; import GeometricOpticsRulerNode from './GeometricOpticsRulerNode.js'; +import { RulerOrientation } from '../model/GeometricOpticsRuler.js'; +import DerivedProperty from '../../../../axon/js/DerivedProperty.js'; +import IProperty from '../../../../axon/js/IProperty.js'; class RulersToolbox extends Panel { @@ -46,9 +49,11 @@ class RulersToolbox extends Panel { tandem: Tandem.REQUIRED }, options ); - // create two icons for rulers: A vertical and a Horizontal ruler - const horizontalRulerIconNode = createRulerIcon( false, false ); - const verticalRulerIconNode = createRulerIcon( true, true ); + // create icons for the 2 rulers + // @ts-ignore DerivedProperty.not has incorrect param type + const horizontalRulerIconNode = createRulerIcon( 'horizontal', DerivedProperty.not( horizontalRulerNode.visibleProperty ) ); + // @ts-ignore DerivedProperty.not has incorrect param type + const verticalRulerIconNode = createRulerIcon( 'vertical', DerivedProperty.not( verticalRulerNode.visibleProperty ) ); // increase touchArea and mouseArea for both rulers horizontalRulerIconNode.touchArea = horizontalRulerIconNode.localBounds.dilatedXY( @@ -76,26 +81,18 @@ class RulersToolbox extends Panel { */ const addForwardingListener = ( iconNode: Node, rulerNode: GeometricOpticsRulerNode ): void => { - // ruler node and icon node have opposite visibilities - rulerNode.visibleProperty.link( ( visible: boolean ) => { - iconNode.visible = !visible; - } ); - iconNode.addInputListener( DragListener.createForwardingListener( ( event: SceneryEvent ) => { + assert && assert( !rulerNode.ruler.visibleProperty.value ); - // we can add a ruler only if the ruler Node is not visible - if ( !rulerNode.visible ) { - - // set the visibility of ruler Node to true - rulerNode.visible = true; + // Make the ruler visible. + rulerNode.ruler.visibleProperty.value = true; - // position the center of the rulerNode to the cursor - assert && assert( event.pointer.point ); // {Vector2|null} - rulerNode.center = this.globalToParentPoint( event.pointer.point! ); + // Position the center of the rulerNode at the pointer. + assert && assert( event.pointer.point ); // {Vector2|null} + rulerNode.center = this.globalToParentPoint( event.pointer.point! ); - // forward events - rulerNode.startDrag( event ); - } + // Forward events to the RulerNode. + rulerNode.startDrag( event ); } ) ); }; @@ -112,14 +109,27 @@ class RulersToolbox extends Panel { /** * Returns a small ruler icon - * @param isVertical - is the ruler icon along the vertical axis - * @param tickMarksOnBottom + * @param orientation + * @param visibleProperty */ -function createRulerIcon( isVertical: boolean, tickMarksOnBottom: boolean ): RulerNode { +function createRulerIcon( orientation: RulerOrientation, visibleProperty: IProperty ): RulerNode { const rulerWidth = 400; const rulerHeight = 0.35 * rulerWidth; + const options = { + + // RulerNode options + backgroundLineWidth: 3, + minorTicksPerMajorTick: 5, + majorTickHeight: ( 0.6 * rulerHeight ) / 2, + minorTickHeight: ( 0.4 * rulerHeight ) / 2, + majorTickLineWidth: 5, + minorTickLineWidth: 2, + cursor: 'pointer', + visibleProperty: visibleProperty + }; + const numberOfMajorTicks = 5; const majorTickLabels = [ '' ]; for ( let i = 1; i < numberOfMajorTicks; i++ ) { @@ -128,20 +138,11 @@ function createRulerIcon( isVertical: boolean, tickMarksOnBottom: boolean ): Rul const majorTickWidth = rulerWidth / ( majorTickLabels.length - 1 ); const units = ''; // empty string for units - const rulerIconNode = new RulerNode( rulerWidth, rulerHeight, majorTickWidth, majorTickLabels, units, { - backgroundLineWidth: 3, - tickMarksOnBottom: tickMarksOnBottom, - minorTicksPerMajorTick: 5, - majorTickHeight: ( 0.6 * rulerHeight ) / 2, - minorTickHeight: ( 0.4 * rulerHeight ) / 2, - majorTickLineWidth: 5, - minorTickLineWidth: 2, - cursor: 'pointer' - } ); + const rulerIconNode = new RulerNode( rulerWidth, rulerHeight, majorTickWidth, majorTickLabels, units, options ); rulerIconNode.scale( 0.12 ); // rotate to create vertical ruler - if ( isVertical ) { + if ( orientation === 'vertical' ) { rulerIconNode.rotate( -Math.PI / 2 ); }