phetsims / ohms-law

"Ohm's Law" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/ohms-law
GNU General Public License v3.0
5 stars 6 forks source link

Draft changes to Ohm's Law descriptions ready for review #90

Closed terracoda closed 6 years ago

terracoda commented 7 years ago

Based on user feedback this week, I have made some changes to the Ohm's Law descriptions.

Would anyone like to comment on the changes. The changed descriptions are in the first column (orange column).

@emily-phet, @arouinfar, @oliver-phet, @jessegreenberg, @zepumph

You can comment here, or in the document. I find commenting on google sheets to be a little cumbersome.

jessegreenberg commented 7 years ago

@terracoda everything looks good to me!

I noticed you changed summaries to sentences instead of lists. Is that because of https://github.com/phetsims/ohms-law/issues/82, or do you believe that is a better experience? If because of https://github.com/phetsims/ohms-law/issues/82, we can probably prioritize fixing that issue before this sim is deployed with a11y (and we will probably have to if we want to use <em> and <strong> tags.)

terracoda commented 7 years ago

@jessegreenberg, I did remove the list, partly due to #82 , but also exploring a briefer representation. I will look now at the design doc to see how the new descriptions work while keeping the lists. I'd actually like to know in general if users prefer getting little lists, or more paragraph-like descriptions.

terracoda commented 7 years ago

@jessegreenberg, I added both options, list structure and paragraph structure, so we can potentially more easily update the strings without changing the current structure.

Note I also changed to using curly braces as that's what you use in the a11y strings file - keeps things simple for copying.

arouinfar commented 7 years ago

Looks good @terracoda!

B19 & B22:

Question: Should we refer to "current" as "current, I" in the alert?

I don't think it's necessary here, and I think it'd look a bit unbalanced unless you also say "voltage, V" and "resistance, R".

B22:

Question: Should add "a lot" to communicate large size changes for I?

I like this suggestion! Since R and I have an inverse relationship, users can definitely encounter bigger changes, so I think using the "a lot" modifier when jumping 2+ sizes may help cue the differences between the V/I and R/I relationships.

terracoda commented 7 years ago

@ariel-phet, @arouinfar thinks the updates are good. Could you have a read through of column B (the orange column) in the google sheet. I've marked content changes in red. You can see where content has been reduced by comparing with the orange column (column B) with the green column (column C).

terracoda commented 7 years ago

@emily-phet and @ariel-phet, do either of you want to read through the changes to Ohm's Law descriptions?

I'd like to ask @jessegreenberg or @zepumph update the current descriptions.

zepumph commented 7 years ago

Just so everyone is aware of the current plan. Once this issue has been reviewed by @ariel-phet & co and implemented by @jessegreenberg. I will go ahead with #92 so that the sonification wrapper can use a new dev version that has updated descriptions for @terracoda to use in interviews hopefully mid next week.

ariel-phet commented 7 years ago

@terracoda a few comments

B4: Why did your remove the equation description? Isn't knowning that V=I*R is displayed important (since there are several other ways one could algebraically rearrange this equation...or if someone does not remember the equation?) Is it because the student explores to B12 and that clears it up?

B12: yes, that is the best order IMO

B14: "Batteries total" is kind of a strange phrasing. Perhaps "Batteries outputting" or something along those lines? "Batteries providing" "Batteries supplying" (I think "Batteries supplying" is best)

B19: I think just "current" is fine

terracoda commented 7 years ago

@ariel-phet, awesome feedback! Thank-you.

B4: I removed the actual equation from the Scene Summary to simplify the set up, however, I think it can stay. One repetition of the actual equation is likely not a bad thing.

B14: Great suggestions for the batteries. I was thinking of them representing a total voltage, but you are correct, batteries supply voltage.

B19: We'll keep I out of the alert and see how more users make sense of it. I think the other simplifications with set things up more directly.

terracoda commented 7 years ago

@jessegreenberg and @zepumph I've updated the Ohm's Law description spreadsheet. The new descriptions are ready for implementation.

We can try again with the list structure to see how the simplified content sounds. I think there is at least one less list now.

If we cannot resolve #82 easily, I think I think it would be a good idea replace the lists with the paragraphs in this sim. There is very little content in general in this sim, so I think the paragraphs will read fine.

Let me know if you have any questions.

zepumph commented 7 years ago

I'll take a look at this. I am going to keep the lists for now, and we can always change them pretty easily if #82 doesn't pan out.

zepumph commented 7 years ago

I implemented everything exactly as it is in the orange column with a few exceptions/questions:

Update: Sorry I am actually not totally done yet, these are just some questions for the 2/3rds or so I have done. I have gotten to everything up to "Slider Controls". You can see it on phettest, http://phettest.colorado.edu/ohms-law/ohms-law-a11y-view.html?brand=phet&accessibility.

terracoda commented 7 years ago

Thanks @zepumph ,

zepumph commented 7 years ago

Did you do the same thing for the 2 sentences describing the equation (no list there, though)?

Not quite, there we have the first p as the accessibleDescription to the whole FormulaNode ("Voltage, V, is equal to Current, I, . . ."), and then another 'p' tag is a child of that node which holds the dynamic 'In equation, letter V. . . '

Update: changed above, not accessibleLabel but accessibleDescription

zepumph commented 7 years ago

@terracoda I'm confused about another box B19. I see that some of the dynamic pieces are italicized.

{{3.6 volts.}} As letter V {{shrinks}}, letter I {{shrinks}}. Current now {{7.2 millamps}} with voltage at {{3.6 volts}}.

Is this deliberate? Part of my confusion is that the curly braces are italicized also, perhaps just meaning that you are emphasizing that this value is dynamic?

Currently we only support these html formatting tags in descriptions and not in alerts. I think that eventually that would be a good thing to add to the mixin, but we aren't quite there yet. @jessegreenberg how long would you expect it to take to support html formatting tags in alerts if we go down that road?

jessegreenberg commented 7 years ago

@zepumph I would guess 2-4 hours, we would need to make the changes to AriaHerald/UtteranceQueue/Utterance doing similar things as were done in the Accessibility mixin.

terracoda commented 7 years ago

@zepumph, yes the emphasis was to highlight that they are dynamic. You are correct, however, these are alerts! It is not so important to highlight and I do not want to complicate any live region content!

I do not think we need formatted content in the alerts.

zepumph commented 7 years ago

All other changes have been pushed to master

jessegreenberg commented 7 years ago

I do not think we need formatted content in the alerts.

Thanks @terracoda, we won't add formatting to alerts until it is requested.

zepumph commented 7 years ago

Awesome!!! @terracoda thanks for all the work on these. I have updated the descriptions on master, and you can get them here or phettest. http://www.colorado.edu/physics/phet/dev/html/ohms-law/1.4.0-dev.14/ohms-law-a11y-view.html

I have also published a new phetio version and hooked it into the sonification wrapper, then I deployed that.

http://www.colorado.edu/physics/phet/dev/html/phet-io-wrapper-sonification/1.0.0-dev.27/phet-io-wrapper-sonification/sonification.html

You should be all set to use updated descriptions in future interviews. Let me know if there is more to do!

emily-phet commented 7 years ago

@terracoda @zepumph This may have been addressed in a newer version, but when playing with th e1.4.0-dev.14 linked above, the circuit arrow description stays at "tiny arrow" for the vast majority of cases. It's only when the arrow visually appears quite large that this "tiny arrow" description changes. Feels incorrect to me.

terracoda commented 7 years ago

@zepumph, it looks like the description for the Current Arrows changes more often, but the size description still doesn't match what I think it should be visually.

terracoda commented 7 years ago

@zepumph re my last comment, would it be possible to map out the 7 sizes visually, and see how that fits with the model. It's possible that the ranges for each size are not even.

terracoda commented 6 years ago

@zepumph and @jessegreenberg, I think all the descriptions in this issue have been implemented. So I will close it.

Based on subsequent user comments, I think it is safe to shorten the alert by removing the repeated slider value. I will start a new issue for that.