phetsims / joist

Joist is the main framework for PhET Interactive Simulations. Joist creates and displays the simulation content, home screen, navigation bar, About dialog, enables switching between tabs, and other framework-related features.
http://scenerystack.org/
MIT License
9 stars 6 forks source link

Add phetioDocumentation to preferencesModel linked elements #910

Closed zepumph closed 1 year ago

zepumph commented 1 year ago

From https://github.com/phetsims/phet-io/issues/1913

So that something like . . . preferencesModel.audioModel.voicePitchProperty has doc.

```diff Subject: [PATCH] open based on stringified state, not escaped, https://github.com/phetsims/phet-io/issues/1913 --- Index: js/preferences/PreferencesModel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/preferences/PreferencesModel.ts b/js/preferences/PreferencesModel.ts --- a/js/preferences/PreferencesModel.ts (revision e4babfbaee30f5c36fe5d8b160c1a8acd503b8a7) +++ b/js/preferences/PreferencesModel.ts (date 1677523304456) @@ -337,7 +337,7 @@ ] ); this.addPhetioLinkedElementsForModel( options.tandem, this.audioModel, [ - { property: this.audioModel.audioEnabledProperty, tandemName: 'audioEnabledProperty' }, + { property: this.audioModel.audioEnabledProperty, tandemName: 'audioEnabledProperty', phetioDocumentation: 'hi documentation' }, { property: this.audioModel.soundEnabledProperty, tandemName: 'soundEnabledProperty' }, { property: this.audioModel.extraSoundEnabledProperty, tandemName: 'extraSoundEnabledProperty' }, { property: this.audioModel.voicingEnabledProperty, tandemName: 'voicingEnabledProperty' }, @@ -443,7 +443,11 @@ for ( let j = 0; j < propertiesToLink.length; j++ ) { const modelPropertyObject = propertiesToLink[ j ]; const tandemName = modelPropertyObject.tandemName || modelPropertyObject.property.tandem.name; - this.addLinkedElement( modelPropertyObject.property, { tandem: tandem.createTandem( tandemName ) } ); + const options = { tandem: tandem.createTandem( tandemName ) }; + if ( modelPropertyObject.phetioDocumentation ) { + options.phetioDocumentation = modelPropertyObject.phetioDocumentation + } + this.addLinkedElement( modelPropertyObject.property, options ); } }
zepumph commented 1 year ago

I was thinking about doing this patch, but actually I should just be putting that documentation in the elements they are linking to.

```diff Subject: [PATCH] add phetioDocumentation to audio preferences linked elements, --- Index: js/preferences/PreferencesModel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/preferences/PreferencesModel.ts b/js/preferences/PreferencesModel.ts --- a/js/preferences/PreferencesModel.ts (revision c583fe6199c3bbf29eee138b4e5549c4248b6a5f) +++ b/js/preferences/PreferencesModel.ts (date 1677529074021) @@ -15,7 +15,7 @@ import audioManager from '../audioManager.js'; import Property from '../../../axon/js/Property.js'; import NumberProperty from '../../../axon/js/NumberProperty.js'; -import PhetioObject, { PhetioObjectOptions } from '../../../tandem/js/PhetioObject.js'; +import PhetioObject, { LinkedElementOptions, PhetioObjectOptions } from '../../../tandem/js/PhetioObject.js'; import optionize, { EmptySelfOptions } from '../../../phet-core/js/optionize.js'; import regionAndCultureManager from './regionAndCultureManager.js'; import SpeechSynthesisAnnouncer from '../../../utterance-queue/js/SpeechSynthesisAnnouncer.js'; @@ -30,6 +30,7 @@ type ModelPropertyLinkable = { property: TReadOnlyProperty & PhetioObject; tandemName?: string; // if blank, will use the tandem.name of the Property. + phetioDocumentation?: string; }; type CustomPreference = { @@ -337,18 +338,40 @@ ] ); this.addPhetioLinkedElementsForModel( options.tandem, this.audioModel, [ - { property: this.audioModel.audioEnabledProperty, tandemName: 'audioEnabledProperty' }, - { property: this.audioModel.soundEnabledProperty, tandemName: 'soundEnabledProperty' }, - { property: this.audioModel.extraSoundEnabledProperty, tandemName: 'extraSoundEnabledProperty' }, - { property: this.audioModel.voicingEnabledProperty, tandemName: 'voicingEnabledProperty' }, - { property: this.audioModel.voicingMainWindowVoicingEnabledProperty, tandemName: 'voicingMainWindowVoicingEnabledProperty' }, - { property: this.audioModel.voicingObjectResponsesEnabledProperty, tandemName: 'voicingObjectResponsesEnabledProperty' }, - { property: this.audioModel.voicingContextResponsesEnabledProperty, tandemName: 'voicingContextResponsesEnabledProperty' }, - { property: this.audioModel.voicingHintResponsesEnabledProperty, tandemName: 'voicingHintResponsesEnabledProperty' }, - { property: this.audioModel.voicePitchProperty, tandemName: 'voicePitchProperty' }, - { property: this.audioModel.voiceRateProperty, tandemName: 'voiceRateProperty' }, - { property: this.audioModel.voiceProperty, tandemName: 'voiceProperty' } - ] ); + { + property: this.audioModel.audioEnabledProperty, tandemName: 'audioEnabledProperty', + phetioDocumentation: 'Toggles all audio features on and off; supported only if this sim supports audio features.' + }, { + property: this.audioModel.soundEnabledProperty, tandemName: 'soundEnabledProperty', + phetioDocumentation: 'Toggles sound on and off; supported only if this sim supportsSound=true.' + }, { + property: this.audioModel.extraSoundEnabledProperty, tandemName: 'extraSoundEnabledProperty', + phetioDocumentation: 'Toggles extra sound on and off; supported only if this sim supportsExtraSound=true.' + }, { + property: this.audioModel.voicingEnabledProperty, tandemName: 'voicingEnabledProperty', + phetioDocumentation: 'Toggles the voicing feature on and off; supported only if this sim supportsVoicing=true.' + }, { + property: this.audioModel.voicingMainWindowVoicingEnabledProperty, tandemName: 'voicingMainWindowVoicingEnabledProperty', + phetioDocumentation: 'Toggles the voicing feature on and off for the simulation screen (not the voicing preferences and toolbar controls); supported only if this sim supportsVoicing=true.' + }, { + property: this.audioModel.voicingObjectResponsesEnabledProperty, tandemName: 'voicingObjectResponsesEnabledProperty', + phetioDocumentation: 'Toggles voicing responses for object details and changes; supported only if this sim supportsVoicing=true.' + }, { + property: this.audioModel.voicingContextResponsesEnabledProperty, tandemName: 'voicingContextResponsesEnabledProperty', + phetioDocumentation: 'Toggles voicing responses for surrounding context changes; supported only if this sim supportsVoicing=true.' + }, { + property: this.audioModel.voicingHintResponsesEnabledProperty, tandemName: 'voicingHintResponsesEnabledProperty', + phetioDocumentation: 'Toggles voicing responses for extra help; supported only if this sim supportsVoicing=true.' + }, { + property: this.audioModel.voicePitchProperty, tandemName: 'voicePitchProperty', + phetioDocumentation: 'Changes the pitch of the voicing-feature voice; supported only if this sim supportsVoicing=true.' + }, { + property: this.audioModel.voiceRateProperty, tandemName: 'voiceRateProperty', + phetioDocumentation: 'Changes the rate of the voicing-feature voice; supported only if this sim supportsVoicing=true.' + }, { + property: this.audioModel.voiceProperty, tandemName: 'voiceProperty', + phetioDocumentation: 'See voice is currently voicing; supported only if this sim supportsVoicing=true.' + } ] ); this.addPhetioLinkedElementsForModel( options.tandem, this.inputModel ); this.addPhetioLinkedElementsForModel( options.tandem, this.localizationModel, [ { property: this.localizationModel.localeProperty, tandemName: 'localeProperty' } @@ -432,7 +455,8 @@ } } - private addPhetioLinkedElementsForModel( parentTandem: Tandem, featureModel: FeatureModel, additionalProperties: Array = [] ): void { + private addPhetioLinkedElementsForModel( parentTandem: Tandem, featureModel: FeatureModel, + additionalProperties: Array = [] ): void { const tandem = parentTandem.createTandem( featureModel.tandemName ); const propertiesToLink = additionalProperties; for ( let i = 0; i < featureModel.customPreferences.length; i++ ) { @@ -443,7 +467,11 @@ for ( let j = 0; j < propertiesToLink.length; j++ ) { const modelPropertyObject = propertiesToLink[ j ]; const tandemName = modelPropertyObject.tandemName || modelPropertyObject.property.tandem.name; - this.addLinkedElement( modelPropertyObject.property, { tandem: tandem.createTandem( tandemName ) } ); + const options: LinkedElementOptions = { tandem: tandem.createTandem( tandemName ) }; + if ( modelPropertyObject.phetioDocumentation ) { + options.phetioDocumentation = modelPropertyObject.phetioDocumentation; + } + this.addLinkedElement( modelPropertyObject.property, options ); } }
zepumph commented 1 year ago

Ok. This is much nicer to have them in the actual element, than in the linkedElement. @arouinfar please review in master.

arouinfar commented 1 year ago

This is much nicer to have them in the actual element, than in the linkedElement. @arouinfar please review in master.

I agree. It's also very easy to see the documentation in the preview of the linked element. Changes all look good to me @zepumph.