Fix bad title string/width, update TODOs, see phetsims/sun#738

phetsims · Mar 22, 2022 · fddcf83 · fddcf83
1 parent 585f713
commit fddcf83
Show file tree

Hide file tree

Showing 2 changed files with 4 additions and 6 deletions.
diff --git a/js/common/view/NumberPlayAccordionBox.ts b/js/common/view/NumberPlayAccordionBox.ts
@@ -13,7 +13,6 @@ import numberPlay from '../../numberPlay.js';
 import NumberPlayConstants from '../NumberPlayConstants.js';
 import optionize from '../../../../phet-core/js/optionize.js';
 
-// TODO-TS: These should be required
 type SelfOptions = {
   titleString: string,
   titleMaxWidth: number
@@ -41,7 +40,6 @@ class NumberPlayAccordionBox extends AccordionBox {
     contentNode.localBounds = innerContentBounds;
 
     super( contentNode, optionize<NumberPlayAccordionBoxOptions, SelfOptions, AccordionBoxOptions>( {
-      // @ts-ignore TODO-TS: titleString should be required
       titleNode: new Text( options.titleString, {
         font: NumberPlayConstants.ACCORDION_BOX_TITLE_FONT,
         maxWidth: options.titleMaxWidth

diff --git a/js/common/view/NumberPlayScreenView.ts b/js/common/view/NumberPlayScreenView.ts
@@ -34,7 +34,7 @@ import SpeechSynthesisAnnouncer from '../../../../utterance-queue/js/SpeechSynth
 
 // types
 type NumberPlayScreenViewOptions = {
-  wordAccordionBoxOptions: Partial<WordAccordionBoxOptions>, // TODO-TS: MK: Is this alright as a partial?
+  wordAccordionBoxOptions: Partial<WordAccordionBoxOptions>, // TODO-TS: These should not be partial
   totalAccordionBoxOptions: Partial<TotalAccordionBoxOptions>,
   tenFrameAccordionBoxOptions: Partial<TenFrameAccordionBoxOptions>,
   upperAccordionBoxHeight: number,
@@ -108,7 +108,7 @@ class NumberPlayScreenView extends ScreenView {
         expandedProperty: this.onesAccordionBoxExpandedProperty,
         titleString: numberPlayStrings.ones,
         fill: NumberPlayColors.pinkBackgroundColorProperty,
-        titleMaxWidth: 1000 // TODO-TS: MK: this should be a real number
+        titleMaxWidth: NumberPlayConstants.LOWER_ACCORDION_BOX_TITLE_MAX_WIDTH // TODO-TS: this should use the default in CountingAccordionBox
       } );
     onesAccordionBox.left = this.layoutBounds.minX + NumberPlayConstants.ACCORDION_BOX_MARGIN_X;
     onesAccordionBox.bottom = this.layoutBounds.maxY - NumberPlayConstants.SCREEN_VIEW_PADDING_Y;
@@ -126,8 +126,8 @@ class NumberPlayScreenView extends ScreenView {
         linkedPlayArea: model.onesPlayArea,
         expandedProperty: this.objectsAccordionBoxExpandedProperty,
         fill: NumberPlayColors.blueBackgroundColorProperty,
-        titleString: 'TEST FOR NOW', // TODO-TS: MK: this should be a real string
-        titleMaxWidth: 1000 // TODO-TS: MK: this should be a real number
+        titleString: numberPlayStrings.objects, // TODO-TS: this should use the default in CountingAccordionBox
+        titleMaxWidth: NumberPlayConstants.LOWER_ACCORDION_BOX_TITLE_MAX_WIDTH // TODO-TS: this should use the default in CountingAccordionBox
       } );
     this.objectsAccordionBox.right = this.layoutBounds.maxX - NumberPlayConstants.ACCORDION_BOX_MARGIN_X;
     this.objectsAccordionBox.bottom = onesAccordionBox.bottom;