phetsims / number-compare

"Number Compare" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 0 forks source link

stringTest=dynamic is not leveraged for the comparison statement #37

Closed veillette closed 1 year ago

veillette commented 1 year ago

This is done as part of https://github.com/phetsims/qa/issues/934

As part of the QA process one must carry out a series of stringTest.

The following tests work well https://phet-dev.colorado.edu/html/number-compare/1.0.0-rc.1/phet/number-compare_all_phet.html?stringTest=double image

https://phet-dev.colorado.edu/html/number-compare/1.0.0-rc.1/phet/number-compare_all_phet.html?stringTest=O%20Canada image

However, the comparison statement is impervious to the stringTest=dynamic image

The comparison statement will clearly passed the spirit of the string test, but is is odd that the dynamic test does not seem to work. Perhaps it is a problem with dynamic test itself, or the comparison statement is combining strings in a way that the dynamic string test cannot change.

veillette commented 1 year ago

I should clarify. The comparison statement does not get updated while you use the arrow keys to change the strings using stringTest=dynamic.

However, if one carries an action in the sim afterwards, say adding a puppy to the playground, it does update itself.

image

I think it is just a quirk of stringTest=dynamic.

veillette commented 1 year ago

Hmm, I feel that is going into a rabbit hole that may not be very productive. Feel free to close.

zepumph commented 1 year ago

Ahh excellent. Yes thanks. We did this manually in Number Play in https://github.com/phetsims/number-play/commit/3301f630c8554c8dba9d91c772264e74ff614824.

Let's do this here too. While perhaps we wouldn't necessarily need to cherry pick, it seems smartest, since we were just using localeProperty as a proxy for every dependent i18n stringProperty that could effect the comparison.

zepumph commented 1 year ago

@veillette, please take a listen on master and unassign yourself if all is well.

veillette commented 1 year ago

@zepumph I can confirm that it works on phettest.

zepumph commented 1 year ago

Alright! We confirmed that this is fixed in 1.0.0-rc.2 closing