How much logic should we embed in fluent values?

[samreid]

In working on this issue, I realized that writing:

      myFluentKey:             >-
                           { $screenType ->
                           [sumScreen] Special string for the sumScreen
                           *[other] String for the other screens.
                           }

Is a significant improvement over:

myFluentKeyOnTheSumScreen: Special string for the sumScreen
myFluentKeyOnOtherScreens: String for the other screens.

Because of how much it streamlines the call site. Now I no longer need to pass through different strings, choose between them, and add other listeners (if screen were dynamic), instead we just bind it with createProperty. I feel like we may want to aggregate more things in our fluent in the future, and this helps answer questions we previously had about "how do we describe the same component in different contexts?" It doesn't need different string keys in the yaml, it just needs to know about the context with fluent variables.

I'd like to check in with @marlitas about this philosophy, and to ask if we should push more in this direction for existing code (perhaps combining bar and bond descriptions?) or if we agree this is good for new code.

Originally posted by @samreid in #344

[samreid]

After writing the above, I realized it contradicts the recommendations in https://discourse.mozilla.org/t/how-i-can-check-variable-for-non-empty-in-selector/90181/2 and https://github.com/projectfluent/fluent/wiki/Good-Practices-for-Developers. Still, I think we should find a way to streamline description, whether it is via logic in fluent fragments or another adapter layer that selects from fluent fragments.

[samreid]

This seems important to discuss soon, since it affects most/all of the description we are writing and publishing. @jessegreenberg or @marlitas what is your opinion on the matter?

[samreid]

Here is one way to write a patch to use different keys, putting the logic in the sim, while it reduces complexity in fluent, it feels like net added complexity.

Details

Subject: [PATCH] hello
---
Index: number-pairs-strings_en.yaml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/number-pairs-strings_en.yaml b/number-pairs-strings_en.yaml
--- a/number-pairs-strings_en.yaml	(revision f719fd31e9dc1637d24eeb4383318421f1367f89)
+++ b/number-pairs-strings_en.yaml	(date 1762441015648)
@@ -348,15 +348,14 @@
                                                    *[other] Two matching rectangles are stacked on top of each other. Top rectangle is not split and tells total number of objects, bottom rectangle is split by addends, showing two sections that adjust based on value of addends.
                                                     [sumScreen] Two matching rectangles are stacked on top of each other. Top rectangle is split by addends, showing two sections that adjust based on value of addends. Bottom rectangle is not split and tells the sum of two addends, also called the total.
                                                  }
-      currentBarModelStateAccessibleParagraph:   |-
-                                                 { $screenType ->
-                                                 *[ other ] Right now, { $total } in top rectangle. { $left } and { $right } in bottom rectangle, split into { $proportions } sections.
-                                                  [ sumScreen ] Right now, { $left } and { $right } top rectangle, split into { $proportions } sections. 
-                                                        { $totalView ->
-                                                             [shown] { $total } in bottom rectangle.
-                                                            *[hidden] Total hidden from bottom rectangle.
-                                                  }
-                                                 }
+      currentBarModelStateAccessibleParagraph:
+        sumScreen:   |-
+                     Right now, { $left } and { $right } top rectangle, split into { $proportions } sections. 
+                                                { $totalView ->
+                                                              [shown] { $total } in bottom rectangle.
+                                                             *[hidden] Total hidden from bottom rectangle.
+                                                   }
+        otherScreen: Right now, { $total } in top rectangle. { $left } and { $right } in bottom rectangle, split into { $proportions } sections.
       largerAndSmaller:                          larger and smaller
       smallerAndLarger:                          smaller and larger
       equal:                                     equal
Index: js/common/view/NumberBondAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/NumberBondAccordionBox.ts b/js/common/view/NumberBondAccordionBox.ts
--- a/js/common/view/NumberBondAccordionBox.ts	(revision f719fd31e9dc1637d24eeb4383318421f1367f89)
+++ b/js/common/view/NumberBondAccordionBox.ts	(date 1762440860490)
@@ -59,13 +59,18 @@
       ( left, right, total, largerAndSmaller, smallerAndLarger, equal ) => {
         return left === right ? equal : left > right ? largerAndSmaller : smallerAndLarger;
       } );
-    const barModelParagraphProperty = NumberPairsFluent.a11y.controls.numberModel.currentBarModelStateAccessibleParagraph.createProperty( {
+
+    const barModelParagraphProperty = model instanceof SumModel ? NumberPairsFluent.a11y.controls.numberModel.currentBarModelStateAccessibleParagraph.sumScreen.createProperty( {
       left: model.leftAddendProperty,
       right: model.rightAddendProperty,
       total: model.totalProperty,
       proportions: proportionsStringProperty,
-      screenType: model instanceof SumModel ? 'sumScreen' : 'other',
       totalView: model.totalVisibleProperty.derived( totalVisible => totalVisible ? 'shown' : 'hidden' )
+    } ) : NumberPairsFluent.a11y.controls.numberModel.currentBarModelStateAccessibleParagraph.otherScreen.createProperty( {
+      left: model.leftAddendProperty,
+      right: model.rightAddendProperty,
+      total: model.totalProperty,
+      proportions: proportionsStringProperty
     } );
     const numberBondParagraphProperty = NumberPairsFluent.a11y.controls.numberModel.currentNumberBondStateAccessibleParagraph.createProperty( {
       left: model.leftAddendProperty,

[samreid]

See this code for how description can be overridden:

    this.bothAddendsEyeToggleButton = new AddendEyeToggleButton( bothAddendsVisibleProperty, {
      accessibleName: NumberPairsFluent.a11y.controls.bothAddendsVisibleButton.accessibleNameStringProperty,
      accessibleHelpText: options.bothAddendsEyeToggleButtonAccessibleHelpText || NumberPairsFluent.a11y.controls.bothAddendsVisibleButton.accessibleHelpTextPattern.createProperty( {
        modelRepresentation: numberBondOrBarModelStringProperty
      } ),
      left: countingAreaBounds.minX + COUNTING_AREA_MARGIN,
      bottom: countingAreaBounds.maxY - COUNTING_AREA_MARGIN,
      visibleProperty: bothAddendsEyeToggleButtonVisibleProperty,
      tandem: bothAddendsEyeToggleButtonTandem,
      accessibleContextResponseOff: options.bothAddendsEyeToggleButtonAccessibleContextResponseOff || NumberPairsFluent.a11y.controls.addendVisibleButton.accessibleContextResponseOffStringProperty,
      accessibleContextResponseOn: options.bothAddendsEyeToggleButtonAccessibleContextResponseOn || NumberPairsFluent.a11y.controls.addendVisibleButton.accessibleContextResponseOnStringProperty
    } );

And there is a bunch of code like this:

    const accordionBoxTitleStringProperty = new DerivedProperty( [
        NumberPairsPreferences.numberModelTypeProperty,
        NumberPairsFluent.numberBondStringProperty,
        NumberPairsFluent.barModelStringProperty ],
      ( numberModelType, numberBondString, barModelString ) => {
        return numberModelType === NumberModelType.NUMBER_BOND_MODEL ? numberBondString : barModelString;
      } );

[jessegreenberg]

Yea, I can see how moving logic into fluent reduces simulation complexity. https://github.com/projectfluent/fluent/wiki/Good-Practices-for-Developers is a great resource, I haven't seen that before.

I think that document makes good arguments for keeping application logic out of the fluent contents.

• This would effectively make translators responsible for maintaining UI logic within localization files.
• Adding more application logic into the Fluent file will increase cognitive load for translators. It is hard to gauge how important this is today since we don’t have a clear plan for Fluent translations or know how much AI we’ll use.
• With the app logic implemented/duplicated in many localization files, it will be very diffficult (impossible?) to modify in the future.

[samreid]

According to the "Good Practices" document, we wouldn't even use selectors for this:

      currentDetails:  |-
                       { $shownSides -> 
                           [ none ]   Counting area currently hidden.
                           [ right ]  Currently, { $total } total, left counting area hidden.
                           [ left ]   Currently, { $total } total, right counting area hidden.
                          *[ both ]   { $representationType ->
                                    [numberLine] Currently, total of { $total } splits colored sections with addend splitter knob in counting area.
                                    [beads] Currently, { $total } total { $itemType } split by color in counting area.
                                    [kittens] Currently, { $total } total { $itemType } split by color in counting area.
                                   *[location] { a11y.introScreen.screenSummary.currentDetails.objectsPattern }
                                  }
                           }

Or even this case from membrane-transport:

    movedAboveLigandGatedChannelResponse:        |-
                                                 Above { $openOrClosed ->
                                                 [open]   open
                                                 *[closed] closed
                                                 } { a11y.transportProtein.briefName } { $proteinType ->
                                                 [sodiumIonLigandGatedChannel] Triangle Site
                                                 *[potassiumIonLigandGatedChannel]     Star Site
                                                 }, { $index } of { $transportProteinCount } Proteins.

Fortunately, Fluent provides a domain specific language that is a very good match for how we want to do the "string lookup". Either building our own typescript-specific code or building a new typescript library to fill this role seems like we are taking on the burden that Fluent is already really good at (even though fluent was not designed to be used in that way).

I am concerned that converting either of those examples above to use "logic in typescript" will be nontrivial, and would basically take the complexity from:
fluent complexity: 2, sim complexity 1
to
fluent complexity 1, sim complexity 5

So, yes, a simplification for fluent, but a net complexity increase for the entire system.

This would effectively make translators responsible for maintaining UI logic within localization files.

It's true, but the burden on the translator for:

      myFluentKey:             >-
           { $screenType ->
             [sumScreen] Special string for the sumScreen
            *[other] String for the other screens.
           }

is not an order of magnitude more burden than:

  myFluentKey:
      onTheSumScreen: Special string for the sumScreen
      onOtherScreens: String for the other screens.

To a translator, those are just different types of syntax. And in fact, if we build our own rosetta UI for translating it, then translators may not even be able to type arbitrary selector keys (for this kind of string union). Finally, would we have our own "checker" to make sure the client didn't delete a yaml key or selector value, if we continue to use fluent in this way? But then we would have to be careful to distinguish between a required selector vs optional selector. (Like, a translator should be able to delete a pluralization selector that is not relevant in their language, but not delete a different selector that shows a different string on a different screen). So maybe this is the best argument for moving logic into the sim, and keeping 100% of the selectors language-centric?

I agree with your question about AI assisted translation.

With the app logic implemented/duplicated in many localization files

Not sure what this means, can you discuss this in context of the membrane transport logic above, or the number pairs currentDetails above? What is duplicated in many localization files?

[jessegreenberg]

Good points, and I see that we are already doing lots of UI specific fluent logic.

With the app logic implemented/duplicated in many localization files

Every fluent pattern appears once in every translation file. Possibly duplicated, or possibly with language specific changes. So if we need to change a selector or its value, or add a nested selector, we need to do that in every translation file. Currently, its difficult to change strings once they have been translated. Now, it will be very difficult to change the logic.

These and the guidelines described in https://github.com/projectfluent/fluent/wiki/Good-Practices-for-Developers made me nervous about leaning more heavily into UI specific logic in Fluent. But it may be a good fit for PhET.

[samreid]

Brainstorming... we could diverge from fluent syntax a little bit more when defining a "required" selector. The syntax would be to remove the * and in this way we could indicate it is a logical, not linguistic, selector, and therefore required:

      myFluentKey:             >-
           { $screenType ->
             [sumScreen] Special string for the sumScreen
             [other] String for the other screens.
           }

This proposal raises a lot of questions and doesn't seem to be a "win win", but just wanted to think it through in case we keep logic in the fluent files to some extent.