Should labels in carousel be instrumented?

[arouinfar]

The labels in the toolbox are instrumented and clients can customize the strings.

Should we do the same for the labels in the carousel?

[arouinfar]

2/24/22 design meeting, we would like to make the strings in the carousel customizable. Additionally, the visibleProperty should be read-only because the labels are controlled by the Labels checkbox.

[samreid]

Patch with current status:

Index: js/view/CircuitElementToolNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/view/CircuitElementToolNode.ts b/js/view/CircuitElementToolNode.ts
--- a/js/view/CircuitElementToolNode.ts	(revision e04cc3b4724ea7486dfbe46ad27e4edb6b01e192)
+++ b/js/view/CircuitElementToolNode.ts	(date 1646872159589)
@@ -45,14 +45,21 @@
   constructor( labelText: string, showLabelsProperty: Property<boolean>, viewTypeProperty: Property<CircuitElementViewType>,
                circuit: Circuit, globalToCircuitLayerNodePoint: ( v: Vector2 ) => Vector2, iconNode: Node, maxNumber: number,
                count: () => number, createElement: ( v: Vector2 ) => CircuitElement, providedOptions?: any ) {
-    const labelNode = new Text( labelText, { fontSize: 12, maxWidth: TOOLBOX_ICON_WIDTH } );
-    showLabelsProperty.linkAttribute( labelNode, 'visible' );
+
+    let labelNode: Node | any = null;
+    if ( labelText.length > 0 ) {
+      labelNode = new Text( labelText, {
+        fontSize: 12, maxWidth: TOOLBOX_ICON_WIDTH,
+        tandem: providedOptions.tandem.createTandem( 'label' )
+      } );
+      showLabelsProperty.linkAttribute( labelNode, 'visible' );
+    }
     providedOptions = merge( {
       spacing: 2, // Spacing between the icon and the text
       cursor: 'pointer',
 
       // hack because the series ammeter tool node has text rendered separately (joined with probe ammeter)
-      children: labelText.length > 0 ? [ iconNode, labelNode ] : [ iconNode ],
+      children: labelNode ? [ iconNode, labelNode ] : [ iconNode ],
 
       // Expand touch area around text, see https://github.com/phetsims/circuit-construction-kit-dc/issues/82
       touchAreaExpansionLeft: 0,

[samreid]

Implemented above (and made visibleProperty readonly). Ready for review.

[arouinfar]

Thanks @samreid. I'm not sure where these changes fell in relation to the changes made in phetsims/chipper#1302. However, reviewing master, each carousel item has a link back to the appropriate stringProperty in general.model.strings, which is what I would expect to see. Closing.