Instrument PatternStringProperty and DerivedProperty for strings that need to be autoselectable in Studio.

[zepumph]

From https://github.com/phetsims/phet-io/issues/1942, I passed some tandems through but mostly Tandem.OPT_OUTs, please review this sim's instrumentation of PatternStringProperties in correlation to this sims' phet-io priorities. In general all PatternStringProperties should be instrumented unless they are just symbols or patterns that aren't for i18n purposes.

[pixelzoom]

Not that we also need to address DerivedProperty.

As soon as I started working on this, I hit a PhET-iO design issue that we have not discussed.

This sim has derived static strings, like these in DomainChartNode.ts:

const DEFAULT_X_SPACE_LABEL_PROPERTY = new PatternStringProperty( FourierMakingWavesStrings.symbolUnitsStringProperty, {
  symbol: FMWSymbols.xStringProperty,
  units: FourierMakingWavesStrings.units.metersStringProperty
}, { tandem: Tandem.OPT_OUT } );
const DEFAULT_X_TIME_LABEL_PROPERTY = new PatternStringProperty( FourierMakingWavesStrings.symbolUnitsStringProperty, {
  symbol: FMWSymbols.tStringProperty,
  units: FourierMakingWavesStrings.units.millisecondsStringProperty
}, { tandem: Tandem.OPT_OUT } );

Because they are static, they don't belong under any specific view element in the Studio tree. Like the string in FourierMakingWavesStrings.ts, they are used for multiple elements. So where should they live in the Studio tree? Should they be under fourierMakingWaves.general.model.strings.fourierMakingWaves, with the translated strings from FourierMakingWavesStrings.ts? Should they be somewhere else in the tree?

This situation is not unique to this sim, and should be standardized. @arouinfar @zepumph @samreid opinions?

[pixelzoom]

@arouinfar and I discussed this. We're going to try putting global instances of PatternStringProperty and DerivedProperty<string> under:

fourierMakingWaves.general.model.strings.fourierMakingWaves.derivedStrings

This will put them in the same part of the tree as generated (translated) string Properties, but differentiate them by grouping them under derivedString.

Again, what we do here will become the convention for all sims. So please weigh in if you have objections or other ideas.

[pixelzoom]

I'm at a point where I think @arouinfar should have a look at the Studio tree, then we should discuss.

Please:

• Review the tree under fourierMakingWaves.general.model.strings.fourierMakingWaves.derivedStrings
• Use the option/alt key to navigate to strings in Studio

Notes about naming conventions that I used:

• DescriptionStringProperty suffix indicates a description of some symbol. For example, kDescriptionStringProperty is the description of "k" that appears in the Info dialogs.
• MarkupStringProperty suffix indicates the markup used to render a symbol in PhET's math font. For example, kMarkupStringProperty has initial value "<span style='font-family: \"Times New Roman\", Times, serif;font-style: italic'>‪k‬</span>"
• functionOf prefix indicates a string that appears as a choice in the "Function of" combo box. For example, functionOfSpaceStringProperty has initial value "space (x)".

[pixelzoom]

I commented in https://github.com/phetsims/studio/issues/305#issuecomment-1581539777, but worth repeating here:

... autoselect is (imo) resulting in an unreasonable development burdern. In #235, I've had to do a ton of work to make derived StringProperties show up in the Studio tree, and it also created PhET-iO design issues that we have yet to resolve. And as I've started on phetsims/graphing-quadratics#178, I'm finding the same "heavy lift", and the same design issues.

[zepumph]

This will put them in the same part of the tree as generated (translated) string Properties, but differentiate them by grouping them under derivedStrings.

Are there some cases where these won't technically be derived? Perhaps a symbol or something that we would still want to instrument, though it isn't translatable? Would it be acceptable to put something like unitsStringProperty( 'Ω') in there? Or perhaps that would just go elsewhere. I don't care too much, I just want to make sure that derivedStrings or whatever we call it can be our one stop shop for all possible stringProperty cases we may desire.

[pixelzoom]

Are there some cases where these won't technically be derived? ...

I'm not entirely clear on what you're asking or suggesting here. There are definitely strings (typically symbols) that are not translatable, have no associated string Property, and therefore will not be autoselectable. For example, these in FMWSymbols.ts:

  infinityMarkup: MathSymbolFont.getRichTextMarkup( '\u221e', 'normal' ),

  // integration symbol
  integralMarkup: MathSymbolFont.getRichTextMarkup( '\u222B', 'normal' ),

  // pi
  piMarkup: MathSymbolFont.getRichTextMarkup( '\u03c0', 'normal' ),

  // summation symbol
  sigmaMarkup: MathSymbolFont.getRichTextMarkup( '\u03a3', 'normal' )

And then there are strings in MathSymbols -- EQUAL_TO, MINUS, etc.

@zepumph Are you suggesting that we should create Properties for these, so that they are autoselectable?

I think we need another meeting about autoselect. Because our decision to make all strings in the UI autoselectable is (in practice) resulting in a large amount of work.

[zepumph]

I definitely missed the point in #235 (comment). Sorry about that. I am now up to speed and not recommending any changes. derivedStrings sounds like a great pattern in general.

@zepumph Are you suggesting that we should create Properties for these, so that they are autoselectable?

No I'm not at all. I was trying to think of a possible example where we would have something instrumented, that needs to go into the model strings, but isn't derived.

Should we continue with #235 (comment) now.

[pixelzoom]

@zepumph

... Should we continue with #235 (comment) now.

Yes.

I said:

I think we need another meeting about autoselect. Because our decision to make all strings in the UI autoselectable is (in practice) resulting in a large amount of work.

I think we need to do this too. But I'm not sure how to proceed. A new issue? Schedule a meeting?

MK: I added this to the phet-io meeting agenda.

[pixelzoom]

Raising the priority of this. Deciding where to put these strings is a prerequisite for completing phetsims/tandem#298. And phetsims/graphing-quadratics#176 (schedule for 6/9-29 iteration) also requires this.

[pixelzoom]

Since what remains here is a design issue, I'm going to unassign everyone except @arouinfar. @zepumph thank you for your input above. @samreid if you have any input, please comment.

[pixelzoom]

@arouinfar and I had a Zoom call about this. Conclusions:

• We will put derived string Properties under a derived tandem name.
• We should not be strict about instrumenting every string that's visible in the UI. Many of them are not useful, and this is over instrumenting.
• Many of the derived string Properties that have been instrumented for this sim are not useful. (We were pushing the limits here, for the purposes of learning.) There are too many levels (nesting) of dependencies to navigate to get to a desired LocalizedStringProperty.
• The syntax used for dependencies in the Studio tree is ugly, intimidating, and so verbose that you can't see the tandem name at the end of the phetioID. You need to either resize the Studio tree extremely, or click each dependency and view the Element panel.

[pixelzoom]

In the above commit, I used DerivedStringProperty (from https://github.com/phetsims/phet-io/issues/1943) for derived strings. I identified these by searching for phetioValueType: StringIO.

[pixelzoom]

Next (final?) step is to decide whether to uninstrument any of the PatternStringProperty or DerivedStringProperty instances. @arouinfar and I suspect that we should uninstrument some (many?) of them. But since they serve as examples, I'll leave them "as is" until we can discuss this topic in PhET-iO meeting.

[pixelzoom]

What I heard in today's PhET-iO meeting about autoselect was that it's OK if all UI strings are not autoselectable -- see https://github.com/phetsims/studio/issues/305#issuecomment-1591658247. So I think that we should reconsider the derived strings that were added to this sim, and uninstrument many (most?) of them.

Fourier is similar to Graphing Quadratic -- they both have a lot of math symbols that are wrapped in markup, and therefore deeply-nested derives strings. So I'll wait to see what @arouinfar has to say about phetsims/graphing-quadratics#186 before removing any instrumentation in Fourier.

[zepumph]

Discussed during PhET-iO meeting today, and we updated the technical guide to the same effect that @pixelzoom's sentiment reflects above. https://github.com/phetsims/phet-io/commit/6444524780372bf3004e1861398950f73ce89b50

Marking off hold.

[pixelzoom]

My understanding from today's meeting was that @arouinfar was going to test-drive graphing-quadratics and fourier before I make changes here. So this remains on hold until phetsims/graphing-quadratics#186 is closed.

[arouinfar]

I prefer the experience in Graphing Quadratics (pattern/derived strings not instrumented) to the one currently in Fourier: Making Waves (everything instrumented).

If I want to edit the x by autoselecting the x (m) axis label, I need to traverse through several layers of dependencies: xAxisLabelStringProperty > xMetersStringProperty > xMarkupStringProperty > xStringProperty. This is not user-friendly, nor is it a particularly compelling customization.

That said, I think the contents of the "Function of:" ComboBox may be worth keeping instrumented (derivedStrings.functionOf*StringProperty). These properties have the same nested dependencies problem, but the most useful dependency (*SymbolsStringProperty) is immediately accessible.

If we are okay with sometimes instrumenting derived strings, I recommend that we keep derivedStrings.functionOf*StringProperty. However, I'm fine with occasionally missing out on strings like this if we prefer an all-or-nothing approach.

[arouinfar]

If we are okay with sometimes instrumenting derived strings, I recommend that we keep derivedStrings.functionOf*StringProperty.

Actually, forget I ever said that. If we keep this, we need to keep all the related dependencies, which will add up. If this string was truly important, I'd cover it in examples.md. But it's not, and I won't.

@pixelzoom please proceed with uninstrumenting all pattern and derived string properties.

[pixelzoom]

Thanks @arouinfar, will do.

[pixelzoom]

In the above commit, I uninstrumented derived strings that I do not think are useful. The end result is a much smaller number of elements under fourierMakingWaves.general.model.strings.fourierMakingWaves.derivedStrings in the Studio tree:

The above elements are the symbols descriptions in the "Info" dialogs for the first 2 screens. While their dependencies do not have a link to the associated symbol's string Property, they do link to the description of the symbol --- and this is most likely what they might want to change.

The only other derived strings that exist are the descriptions of each game level, and the message that appears in the status bar for each game level. For example, for Level 3:

@arouinfar please test drive. If this feels OK, please close. And I'm sure this will get a closer look when this sim is someday "PhET-iO designed".

[arouinfar]

Thanks @pixelzoom I think you found a really good compromise. We no longer have the nested dependencies problem. The derived strings section is much easier to navigate now, and the portion of the string that people likely want to edit is easily accessible. Closing.