From cd7d9a392241c2708c390a6f58ca938030d81dd8 Mon Sep 17 00:00:00 2001 From: chrisklus Date: Thu, 16 Apr 2020 22:16:40 -0400 Subject: [PATCH] Refactor Sim.name as instrumented Sim.simNameProperty and make title texts update from its changes, see https://github.com/phetsims/joist/issues/627 --- js/HomeScreen.js | 6 +++--- js/HomeScreenView.js | 24 ++++++++++++++++-------- js/NavigationBar.js | 43 ++++++++++++++++++++++++++++--------------- js/PhetMenu.js | 6 +++--- js/Sim.js | 14 ++++++++++---- js/simInfo.js | 4 ++-- 6 files changed, 62 insertions(+), 35 deletions(-) diff --git a/js/HomeScreen.js b/js/HomeScreen.js index ae458ff9..88e2e194 100644 --- a/js/HomeScreen.js +++ b/js/HomeScreen.js @@ -21,7 +21,7 @@ const BACKGROUND_COLOR = 'black'; class HomeScreen extends Screen { /** - * @param {string} simName + * @param {Property.} simNameProperty * @param {function():Property.} getScreenProperty - at the time of construction, the Sim.screenProperty is * - not yet assigned (because it may itself include the * - HomeScreen), so we must use a function to lazily get it @@ -31,7 +31,7 @@ class HomeScreen extends Screen { * @param {Object} [options] * @constructor */ - constructor( simName, getScreenProperty, simScreens, tandem, options ) { + constructor( simNameProperty, getScreenProperty, simScreens, tandem, options ) { options = merge( { @@ -47,7 +47,7 @@ class HomeScreen extends Screen { super( () => new HomeScreenModel( getScreenProperty(), simScreens, tandem.createTandem( 'model' ) ), - model => new HomeScreenView( simName, model, tandem.createTandem( 'view' ), _.pick( options, [ + model => new HomeScreenView( simNameProperty, model, tandem.createTandem( 'view' ), _.pick( options, [ 'warningNode' ] ) ), options diff --git a/js/HomeScreenView.js b/js/HomeScreenView.js index c4d79950..28b44436 100644 --- a/js/HomeScreenView.js +++ b/js/HomeScreenView.js @@ -6,12 +6,12 @@ * @author Sam Reid (PhET Interactive Simulations) */ -import PDOMPeer from '../../scenery/js/accessibility/pdom/PDOMPeer.js'; import Bounds2 from '../../dot/js/Bounds2.js'; import inherit from '../../phet-core/js/inherit.js'; import merge from '../../phet-core/js/merge.js'; import StringUtils from '../../phetcommon/js/util/StringUtils.js'; import PhetFont from '../../scenery-phet/js/PhetFont.js'; +import PDOMPeer from '../../scenery/js/accessibility/pdom/PDOMPeer.js'; import HBox from '../../scenery/js/nodes/HBox.js'; import Node from '../../scenery/js/nodes/Node.js'; import Text from '../../scenery/js/nodes/Text.js'; @@ -30,14 +30,14 @@ const LAYOUT_BOUNDS = new Bounds2( 0, 0, 768, 504 ); const TITLE_FONT_FAMILY = 'Century Gothic, Futura'; /** - * @param {string} simName - the internationalized text for the sim name + * @param {Property.} simNameProperty - the internationalized text for the sim name * @param {HomeScreenModel} model * @param {Tandem} tandem * @param {Object} [options] * @constructor */ -function HomeScreenView( simName, model, tandem, options ) { - assert && assert( simName, 'simName is required: ' + simName ); +function HomeScreenView( simNameProperty, model, tandem, options ) { + assert && assert( simNameProperty.value, 'simName is required: ' + simNameProperty.value ); const self = this; options = merge( { @@ -53,14 +53,14 @@ function HomeScreenView( simName, model, tandem, options ) { includePDOMNodes: false, // pdom - labelContent: simName, + labelContent: simNameProperty.value, descriptionContent: StringUtils.fillIn( homeScreenDescriptionPatternString, { - name: simName, + name: simNameProperty.value, screens: model.simScreens.length } ) } ); - const titleText = new Text( simName, { + const titleText = new Text( simNameProperty.value, { font: new PhetFont( { size: 52, family: TITLE_FONT_FAMILY @@ -68,7 +68,15 @@ function HomeScreenView( simName, model, tandem, options ) { fill: 'white', y: 130, maxWidth: this.layoutBounds.width - 10, // To support PhET-iO Clients setting this - tandem: tandem.createTandem( 'titleText' ) + tandem: tandem.createTandem( 'titleText' ), + phetioComponentOptions: { + textProperty: { phetioReadOnly: true } + } + } ); + + // update the titleText when the sim name changes + simNameProperty.link( simTitle => { + titleText.setText( simTitle ); } ); // Have this before adding the child to support the startup layout. diff --git a/js/NavigationBar.js b/js/NavigationBar.js index 521f3560..2c3e9231 100644 --- a/js/NavigationBar.js +++ b/js/NavigationBar.js @@ -24,6 +24,7 @@ */ import DerivedProperty from '../../axon/js/DerivedProperty.js'; +import Property from '../../axon/js/Property.js'; import Dimension2 from '../../dot/js/Dimension2.js'; import inherit from '../../phet-core/js/inherit.js'; import StringUtils from '../../phetcommon/js/util/StringUtils.js'; @@ -36,8 +37,8 @@ import A11yButtonsHBox from './A11yButtonsHBox.js'; import HomeButton from './HomeButton.js'; import HomeScreen from './HomeScreen.js'; import HomeScreenView from './HomeScreenView.js'; -import joistStrings from './joistStrings.js'; import joist from './joist.js'; +import joistStrings from './joistStrings.js'; import NavigationBarScreenButton from './NavigationBarScreenButton.js'; import PhetButton from './PhetButton.js'; @@ -115,17 +116,8 @@ function NavigationBar( sim, isMultiScreenSimDisplayingSingleScreen, tandem ) { this.barContents = new Node(); this.addChild( this.barContents ); - let title = sim.name; - - // If the 'screens' query parameter only selects 1 screen, than update the nav bar title to include that screen name. - if ( isMultiScreenSimDisplayingSingleScreen && this.simScreens[ 0 ].nameProperty.value ) { - title = StringUtils.fillIn( simTitleWithScreenNamePatternString, { - simName: sim.name, - screenName: this.simScreens[ 0 ].nameProperty.value - } ); - } + let title = sim.simNameProperty.value; - // Sim title this.titleText = new Text( title, { font: new PhetFont( 16 ), fill: sim.lookAndFeel.navigationBarTextFillProperty, @@ -133,12 +125,33 @@ function NavigationBar( sim, isMultiScreenSimDisplayingSingleScreen, tandem ) { phetioDocumentation: 'Displays the title of the simulation in the navigation bar (bottom left)', phetioComponentOptions: { visibleProperty: { phetioFeatured: true }, - textProperty: { phetioFeatured: true } + textProperty: { phetioReadOnly: true } } } ); this.titleText.setVisible( false ); this.barContents.addChild( this.titleText ); + // update the titleText based on values of the sim name and screen name + Property.multilink( [ sim.simNameProperty, this.simScreens[ 0 ].nameProperty ], + ( simName, screenName ) => { + + if ( isMultiScreenSimDisplayingSingleScreen && simName && screenName ) { + // If the 'screens' query parameter selects only 1 screen, than update the nav bar title to include that screen name. + title = StringUtils.fillIn( simTitleWithScreenNamePatternString, { + simName: simName, + screenName: screenName + } ); + } + else if ( isMultiScreenSimDisplayingSingleScreen && screenName ) { + title = screenName; + } + else { + title = simName; + } + + this.titleText.setText( title ); + } ); + // @private - PhET button, fill determined by state of navigationBarFillProperty this.phetButton = new PhetButton( sim, @@ -167,8 +180,8 @@ function NavigationBar( sim, isMultiScreenSimDisplayingSingleScreen, tandem ) { // title can occupy all space to the left of the PhET button this.titleText.maxWidth = HomeScreenView.LAYOUT_BOUNDS.width - TITLE_LEFT_MARGIN - TITLE_RIGHT_MARGIN - - PHET_BUTTON_LEFT_MARGIN - this.a11yButtonsHBox.width - PHET_BUTTON_LEFT_MARGIN - - this.phetButton.width - PHET_BUTTON_RIGHT_MARGIN; + PHET_BUTTON_LEFT_MARGIN - this.a11yButtonsHBox.width - PHET_BUTTON_LEFT_MARGIN - + this.phetButton.width - PHET_BUTTON_RIGHT_MARGIN; } else { /* multi-screen sim */ @@ -262,7 +275,7 @@ function NavigationBar( sim, isMultiScreenSimDisplayingSingleScreen, tandem ) { // Now determine the actual width constraint for the sim title. this.titleText.maxWidth = this.screenButtonsContainer.left - TITLE_LEFT_MARGIN - TITLE_RIGHT_MARGIN - - HOME_BUTTON_RIGHT_MARGIN - this.homeButton.width - HOME_BUTTON_LEFT_MARGIN; + HOME_BUTTON_RIGHT_MARGIN - this.homeButton.width - HOME_BUTTON_LEFT_MARGIN; } // initial layout (that doesn't need to change when we are re-laid out) diff --git a/js/PhetMenu.js b/js/PhetMenu.js index b8b06dd4..61420ad7 100644 --- a/js/PhetMenu.js +++ b/js/PhetMenu.js @@ -117,7 +117,7 @@ function PhetMenu( sim, phetButton, tandem, options ) { Node.call( this ); const aboutDialogCapsule = new PhetioCapsule( tandem => { - return new AboutDialog( sim.name, sim.version, sim.credits, sim.locale, phetButton, tandem ); + return new AboutDialog( sim.simNameProperty.value, sim.version, sim.credits, sim.locale, phetButton, tandem ); }, [], { tandem: tandem.createTandem( 'aboutDialogCapsule' ), phetioType: PhetioCapsuleIO( DialogIO ) @@ -198,7 +198,7 @@ function PhetMenu( sim, phetButton, tandem, options ) { present: isPhETBrand && !isApp, callback: function() { const url = 'http://phet.colorado.edu/files/troubleshooting/' + - '?sim=' + encodeURIComponent( sim.name ) + + '?sim=' + encodeURIComponent( sim.simNameProperty.value ) + '&version=' + encodeURIComponent( sim.version + ' ' + ( phet.chipper.buildTimestamp ? phet.chipper.buildTimestamp : '(require.js)' ) ) + '&url=' + encodeURIComponent( window.location.href ) + @@ -256,7 +256,7 @@ function PhetMenu( sim, phetButton, tandem, options ) { const blob = new window.Blob( [ byteArray ], { type: 'image/png' } ); // our preferred filename - const filename = stripEmbeddingMarks( sim.name ) + ' screenshot.png'; + const filename = stripEmbeddingMarks( sim.simNameProperty.value ) + ' screenshot.png'; if ( !fuzzes ) { window.saveAs( blob, filename ); diff --git a/js/Sim.js b/js/Sim.js index ddb36a59..bcf51acd 100644 --- a/js/Sim.js +++ b/js/Sim.js @@ -18,6 +18,7 @@ import NumberProperty from '../../axon/js/NumberProperty.js'; import ObservableArray from '../../axon/js/ObservableArray.js'; import Property from '../../axon/js/Property.js'; import PropertyIO from '../../axon/js/PropertyIO.js'; +import StringProperty from '../../axon/js/StringProperty.js'; import timer from '../../axon/js/timer.js'; import Bounds2 from '../../dot/js/Bounds2.js'; import Dimension2 from '../../dot/js/Dimension2.js'; @@ -124,8 +125,13 @@ function Sim( name, allSimScreens, options ) { // override rootRenderer using query parameter, see #221 and #184 options.rootRenderer = phet.chipper.queryParameters.rootRenderer || options.rootRenderer; - // @public (joist-internal) - this.name = name; + // @public {Property.} (joist-internal) + this.simNameProperty = new StringProperty( name, { + tandem: Tandem.GENERAL_VIEW.createTandem( 'simNameProperty' ), + phetioFeatured: true, + phetioDocumentation: 'The name of the sim. Changing this value will update the title text on the navigation bar ' + + 'and the title text on the home screen, if it exists.' + } ); // playbackModeEnabledProperty cannot be changed after Sim construction has begun, hence this listener is added before // anything else is done, see https://github.com/phetsims/phet-io/issues/1146 @@ -324,7 +330,7 @@ function Sim( name, allSimScreens, options ) { screensQueryParameter, QueryStringMachine.containsKey( 'screens' ), selectedSimScreens => { - return new HomeScreen( this.name, () => this.screenProperty, selectedSimScreens, Tandem.ROOT.createTandem( 'homeScreen' ), { + return new HomeScreen( this.simNameProperty, () => this.screenProperty, selectedSimScreens, Tandem.ROOT.createTandem( 'homeScreen' ), { warningNode: options.homeScreenWarningNode } ); } @@ -791,7 +797,7 @@ export default inherit( Object, Sim, { screen.initializeModel(); } ); workItems.push( function() { - screen.initializeView( self.name, self.screens.length ); + screen.initializeView( self.simNameProperty.value, self.screens.length ); } ); } ); diff --git a/js/simInfo.js b/js/simInfo.js index 3e448bf8..18cac435 100644 --- a/js/simInfo.js +++ b/js/simInfo.js @@ -67,14 +67,14 @@ const simInfo = { // no need to add this again if the method has already been called if ( !info.simName ) { - putInfo( 'simName', sim.name ); + putInfo( 'simName', sim.simNameProperty.value ); putInfo( 'simVersion', sim.version ); putInfo( 'repoName', packageJSON.name ); putInfo( 'screens', sim.screens.map( screen => { const screenObject = { // likely null for single screen sims, so use the sim name as a default - name: screen.nameProperty.value || sim.name + name: screen.nameProperty.value || sim.simNameProperty.value }; if ( Tandem.PHET_IO_ENABLED ) { screenObject.phetioID = screen.tandem.phetioID;