-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
remove pH values from combo box #69
Comments
My guess is this was done so that you could see how adding water or starting out with water and adding different known pH substances affected the solution. Definitely a question for @amyh-phet and @emily-phet (we could also reach out to YY since this was her sim). I imagine this is documented in the design doc, YY was pretty thorough. |
Whoops, accidentally deleted my comment. |
Attempt to reproduce my comment.... Gotta say that I agree with this one. I've always found it odd that we display the pH values in the combo boxes. (And the combo boxes are ordered by descending pH, another potential give away.) On the first screen, you can put the pH probe in the solution to measure the pH. Simply leave it there if you want to switch between solutions for comparison. On the second screen, the pH display is sitting right next to the combo box, kind of redundant. And the expand/collapse button on the pH display let's you show/hide the pH. But what's the point when the pH value is in the combo box? |
@ariel-phet said:
I guess that's a reasonable reason. But (as this teacher points out) it sabotages the more introductory task of experimenting with (and comparing) various known solutions. |
@pixelzoom I would suggest reaching out to YY. I agree she will likely have something about this decision documented. Off the top of my head it seems needlessly redundant as you suggest, but I'd be surprised if there wasn't some reasoning behind it. We might disagree with the reasoning, but it would be nice to know what it was. |
@oliver-phet @ariel-phet @pixelzoom I agree that having the pH in the combo box does provide redundant information and gives away a lot of information to the user. I can look at the design docs to see if we can figure out why this was done. My impression is that YY likely had a good reason for doing this, so it would be helpful to know her thinking before making changes. |
@amyh-phet when you get a chance please peruse the design doc...if nothing is obvious, let me know and I will reach out to YY |
@ariel-phet I looked through the HTML 5 design doc and did not find any reason for labeling the solutions in the combo box with their pH. It looks like this behavior was carried over from the Java version. The only mention of the label for the solutions in the combo box was a justification for labeling solutions as Spit (pH 7.4) instead of Spit (7.4). Thinking we'll need to reach out to YY to figure this out. I'm guessing it is related to what you mentioned above as one of the learning goals for both MS and HS is to explain how adding water or removing liquid from the beaker effects pH. |
I looked through the design doc for the Java version - it's checked in to Unfuddle at https://phet.unfuddle.com/a#/projects/9404/repositories/23262/file?path=%2Ftrunk%2Fsimulations-java%2Fsimulations%2Fph-scale%2Fdoc%2Fph-scale-design.pdf&commit=74659. The mockups in that document don't show the pH value in the combo box. So I'm not sure when or why it was introduced. You might also consider asking Trish and Kathy. Besides me, they are the only team members who were involved with the design process for the Java version. |
@pixelzoom Thanks for checking the Java design doc. I'll get in touch with Trish to see if she knows why this might have been included. |
@oliver-phet @pixelzoom @ariel-phet I checked with Trish and she does not recall why this was included in the design. Let me know if you need me to reach out to YY. |
I will reach out to YY |
Yes, let's reach out to YY. If she doesn't have a reason, I think we should consider removing. |
YY says:
|
Presumably, if we decide to remove it from ph-scale, then we'll also want to remove it from ph-scale-basics. Note to self: This will be relatively easy to patch into a maintenance release. The change is in SoluteComboBox, from this: 68 var textNode = new Text(
69 StringUtils.format( pattern0Name1PHString, solute.name, Util.toFixed( solute.pH, PHScaleConstants.PH_COMBO_BOX_DECIMAL_PLACES ) ),
70 { font: new PhetFont( 22 ) } );, to this: var textNode = new Text( solute.name, {
font: new PhetFont( 22 )
} ) Also: |
Hi team, Sorry for the delay getting back to you. I dug around a bit and looked back on the old design doc, and although this feature is not mentioned in the old Java documentation, it was a feature in the Java version by the time we were beginning the conversion to HTML5. So, the initial rationale for including the pH in the description/dropdown menu was from porting the Java design. Thinking back on interviews, I remember a few students using that value as a reference point, so that they could compare the original pH to the pH after some action they performed (e.g. dilution), and I think that was part of our rationale for keeping this or including it in the original design. However, most students (especially thinking back on MS students and pH-scale basics) follow the pH changes dynamically, without referring back to the original solution pH, and track the increase or decrease upon dilution moment-by-moment, rather than referring back to the original value. Given the advantages of allowing for prediction/testing activities by removing this from the solution description, and the fact that students are still able to make comparisons without it, I recommend removing. |
Hi @ycarpenterphet. Nice to see your smiling face on GitHub. And happy new year! @amyh-phet @ariel-phet If you'd like me to proceed with this change, assign to me. |
@pixelzoom marking low priority - basically I would say make this change when you need a mental break from unit rates. |
Code changes are done (master & 1.2 branches) and tested in requirejs mode. I was unable to publish because the recent requirejs update (phetsims/chipper#538) broke the sim deployment instructions, and I couldn't build from the 1.2 branch. Discussing on Skype, there are now additional "npm update" instructions that are needed. Deployment is blocked until instructions are fixed (phetsims/phet-info#39). |
Note to self: After phetsims/phet-info#39 is addressed, need to deploy ph-scale 1.2.8 and ph-scale-basics 1.2.8. |
@ariel-phet Do you want the screenshots updated for ph-scale and ph-scale-basics? The screenshots show the pH values in the combo box, but you'd really need to be looking for them to notice. Updating screenshots is not something that I'm equipped to do (no retina iPad). |
Actually, @ariel-phet never mind. I'll take the pH value out of the current screenshots in Photoshop. |
ph-scale 1.2.8 and ph-scale-basics 1.2.8 have been deployed to the PhET website. English versions of both sims were tested, several translations were spot tested. This took about twice as long as it should have, because I spent considerable time dealing with faulty "sim deployment" instructions (see phetsims/phet-info#39). Closing. |
Reopening. @oliver-phet please notify the user who requested this change. Then you can close this issue. |
Email to user sent. Re-closing. |
From user:
|
@oliver-phet Thanks for posting the user's reaction - nice to hear. |
The text was updated successfully, but these errors were encountered: