phetsims / balancing-chemical-equations

"Balancing Chemical Equations" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/balancing-chemical-equations
GNU General Public License v3.0
2 stars 5 forks source link

Chemical formulae are getting scaled in the game #126

Closed phet-steele closed 6 years ago

phet-steele commented 6 years ago

@pixelzoom, something funky changed in master with this sim. In the game, notice how the right side of the equation is scaled down in the below screenshot. Pretty sure it has to do with the length of the chemical formula and the subscript.

screen shot 2018-05-11 at 10 56 39 am

Seen on macOS 10.13.4 Chrome.

pixelzoom commented 6 years ago

Reproduced. It does appear to be only the longer chemicals that are scaled down. So there's probably a maxWidth that needs to be increased.

pixelzoom commented 6 years ago

This problem is not present in the 1.1 release branch (latest 1.1.11).

pixelzoom commented 6 years ago

This problem was introduced somewhere between 1.2.0-dev.1 and 1.2.0-dev.2 (4/15/18 2:48PM) .

Unfortunately there's no record in GitHub of when 1.2.0-dev.1 was published. The directory on bayes has datestamp 1/9/2016, and dependencies.json says "Fri Jan 08 2016 13:12:56 GMT-0700 (MST)".

pixelzoom commented 6 years ago

For testing purposes, the longest molecule in the sim is C2H5OH. To see that molecule in the game, run with ?showAnswers&playAll, go to Level 2, press the "Skip" button 4 times.

pixelzoom commented 6 years ago

This problem was most likely introduced in the conversion from SubSupText to RichText. The maxWidth applied to the formula in TermNode (see above commit) was resulting in longer formulas being scaled down. I inspected SubSupText in the release branch dependencies (it was deprecated and deleted from master), and my best guess is that maxWidth was formerly being ignored by SubSupText. In any case, since we want all formulas to have the same font size, the maxWidth is not desirable here, and I've removed it.

@phet-steele please verify in master.

pixelzoom commented 6 years ago

Demonstration of the longest formula, C2H5OH, before and after the fix.

Before:

screenshot_621

After:

screenshot_620

pixelzoom commented 6 years ago

I did a little more digging. In the 1.1 release branch, TermNode has no maxWidth setting for the formula. It was introduced in master in https://github.com/phetsims/balancing-chemical-equations/commit/fc7fb9a59df4e12b3db0d08a07d720437081b395 to address #121, stringTest=xss. #121 is no longer an issue after moving from SubSupText to RichText, so removing the maxWidth is definitely the correct fix.

ghost commented 6 years ago

@pixelzoom, sorry for not getting around to this sooner. Looks good in master.