Population graph layout issue with very long strings

[ariel-phet]

Discovered while performing string tests for phetsims/qa#539

With very long strings the population graph (but not the pedigree or proportion graph) gets very messed up in layout

I suspect there is something going on with the "generations" string on the bottom of the graph.

The sim passed all string tests, but this was noticed during ?stringTest=xss (but passes for ?stringTest=long

Although not a showstopper for the first release, this is probably good to fix for iO since a long string could potentially be put in place by an instructional designer

To reproduce:

1. Go to https://phet-dev.colorado.edu/html/natural-selection/1.1.0-dev.18/phet/natural-selection_all_phet.html?stringTest=xss

[pixelzoom]

Agree that this can be deferred for the prototype.

I thought this might be a problem with the string that stringTest=xss uses, so I tried doubling up the stringTest=long string, i.e.:

?stringTest=1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890

... and the result was:

So as the string gets longer, the Population graph is getting pushed more to the right.

This also indicates that the current stringTest=long may be insufficient (issue phetsims/chipper#976 created.)

[pixelzoom]

Note to self... Testing with stringTest=xss is part of the CRC, and I recently completed a self code review. I know that I tested with xss, but I don't recall seeing this problem -- maybe I was just looking for a redirect. In any case, test with a dev version from around that time, to see if this is something that snuck in recently.

[pixelzoom]

This problem first appears in 1.1.0-dev.4. 1.1.0-dev.3 was published 2020-08-03 19:47. 1.1.0-dev.4 was published 2020-08-10 13:42. I suspect that it's related to the "Zoom in to see data" string, which may not have a maxWidth set.

[pixelzoom]

Making the "Zoom to see data" string visible at all times confirms the problem, screenshot below. And looking at the code in PopulationGraphNode.js, I neglected to set a maxWidth:

    const zoomOutToSeeDataText = new Text( naturalSelectionStrings.zoomOutToSeeData, {
      font: NaturalSelectionConstants.INSTRUCTIONS_FONT,
      centerX: gridNode.x + gridWidth / 2,
      centerY: gridNode.y + gridHeight / 2
    } );

[pixelzoom]

Here's a 1-line patch that fixes the problem. I'm going to wait to commit this until we decide when this issue should be addressed.

patch

Index: js/common/view/population/PopulationGraphNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- js/common/view/population/PopulationGraphNode.js	(revision 4b4266ed8b81d55958056710a5e8d8b8b75d9fec)
+++ js/common/view/population/PopulationGraphNode.js	(date 1598582041505)
@@ -116,6 +116,7 @@
 
     const zoomOutToSeeDataText = new Text( naturalSelectionStrings.zoomOutToSeeData, {
       font: NaturalSelectionConstants.INSTRUCTIONS_FONT,
+      maxWidth: 0.75 * gridWidth,
       centerX: gridNode.x + gridWidth / 2,
       centerY: gridNode.y + gridHeight / 2
     } );

[pixelzoom]

This fix is almost zero risk. I recommend that we roll this fix into the prototype RC, and @ariel-phet approves.

[pixelzoom]

Fixed in the above commit and ready for verification.

To verify in 1.1.0-rc.1:

1. Run the sim with ?stringTest=xss
2. Go to either screen
3. Verify that the layout looks correct, and in particular does not look like the screenshot in Population graph layout issue with very long strings #210 (comment)

[amanda-phet]

Checked this out on the iPad and it looks good. I also made sure the "zoom in.." text showed up and it still looked good.

[ariel-phet]

Looks good, checked as phetsims/qa#540

Closing