Closed terracoda closed 5 years ago
Please assign to @twant to make any corrections to the read out ranges.
@Matthew-phet and @twant, I went ahead and checked the read out ranges as it was easy. There are 2 more read outs that need correcting: Gold(III) chloride and Potassium chromate.
@twant, I have a feeling these are rounding errors because I think I asked you to make the read out range to a single decimal assuming everything was like Drink mix (0 to 5.0). However, now I realize that this was naive. The readout range is a bit weird because it changes when Solution Values is checked and when it is not checked.
I think we discussed this issue previously. Unfortunately, the implemented solution has some inconsistencies.
If we always provide the numbers for that line (with or without Solution Values checked), maybe the line should refer specifically to the current solution, and always be rounded to the third decimal, as it is visually with Solution Values checked.
I propose we tweaked the wording slightly in a way that can be static except for the ending value, and then always use the actual value used for the solution when Solution Values is checked (with 3 decimal places). Here are 3 examples for that last list item in the Play Area's Beaker description:
Suggested Solution
Cobalt(II) chloride:
Gold(III) chloride:
Potassium chromate:
I feel like wording the line item this way helps us gracefully around the fact that the read out is fixed at "zero to high" when Solution Values is not checked. We already capture the concentration level qualitatively, so no need to refer to that visual representation "zero to high".
Now (finally), I actually think that "zero to high" scale is referring to the saturation point and has nothing to do with a concentration value. We can discuss that idea for a future release.
@Matthew-phet, shall we just do the above suggested solution, or would you like to discuss further? @emily-phet, please chime in if you feel the need. I think the above suggestion solution is a good one.
FYI, I ran this by @emily-phet and she likes it.
@twant, @Matthew-phet and I discussed, too. @Matthew-phet agreed that the new wording with the values to 3 decimal place like the examples I suggested in https://github.com/phetsims/molarity/issues/149#issuecomment-535560953 should work well. Handing over to you, @twant :-)
@terracoda @Matthew-phet, I've changed it to now always go to three decimal places for the concentration readout bullet point. Let me know what you think!
@twant, the numbers look great. We also wanted a small wording change (see bolded text):
Once "for this solution" is added, you can close this issue. Nice work!
Apologies I didn't bold this in the comment in https://github.com/phetsims/molarity/issues/149#issuecomment-535560953
@terracoda @Matthew-phet, apologies, missed that wording change the first time around. I've pushed this change as well! Back over to you.
Looks great. Thanks @twant and no worries, closing!
@Matthew-phet, I just happened to notice that one readout range was incorrect. Notice in the screen shot that the solution is saturated at 4.350 molar, but in the PDOM content (see Voice Over's black box), the readout range is 0 to 4.4
This might be a rounding error. Could you check with @twant if the readout ranges are calculated or fixed strings.
Also, could check the other read out ranges to make sure they match the visual values.