phetsims / capacitor-lab-basics

"Capacitor Lab: Basics" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
6 stars 4 forks source link

User suggestion: timer and play pause #234

Open ariel-phet opened 6 years ago

ariel-phet commented 6 years ago

A twitter user suggested this sim would be nice for students taking data regarding an RC circuit if the sim had a Play/Pause button on the Light Bulb screen.

The also suggested adding a timer.

It seems the play/pause/step buttons are a no-brainer and easy to add. Somewhat surprised we did not have this on the Light Bulb screen already. (Certainly no need on the first screen so no action is dynamic).

I think a timer would also be easy to add if supported by the model. Overall these seem like good suggestions and easy tweaks. Will ask @jonathanolson to confirm.

@arouinfar - thoughts here?

jonathanolson commented 6 years ago

Yes, this should be pretty easy to add.

arouinfar commented 6 years ago

@ariel-phet I think playback controls and a stopwatch would be useful additions.

Here's a mockup showing the proposed changes @jonathanolson. image

I'm slightly concerned with the contrast between the sim background and the standard pause color. Since I used screenshots, the colors may be off a bit, so I'd like to see how it looks in the sim before making any further recommendations.

ariel-phet commented 6 years ago

@jonathanolson can you give me an estimated number of hours to do this work and do a release? (I think we would do a full RC just to get the sim on chipper 2.0).

I won't necessarily have you do the work, but if it is in the 1-2 hour range I might consider having you do it.

jonathanolson commented 6 years ago

but if it is in the 1-2 hour range

It would definitely be in that range (only qualification is if the slow-motion is tricky to implement).

ariel-phet commented 6 years ago

@jonathanolson since in that range, let's plan to do in early April.

ariel-phet commented 6 years ago

@jonathanolson changing my mind on timing...let's do this once Area Model is published. Assigning to myself to hand back to you once that is done.

jonathanolson commented 6 years ago

This is somewhat tangential, but wave-on-a-string has the visual order swapped:

woas-slow

Whereas other sims have the Normal/Slow. Should WOAS be fixed up to match the others?

jonathanolson commented 6 years ago

Also, the desired behavior looks exactly like what TimeControlHBox.js (masses-and-springs) is doing. @Denz1994, mind if I generalize it and move the strings to common code? (I'll probably want to add settings for the properties, may convert the normal/slow to a boolean property, and would add other options).

Denz1994 commented 6 years ago

Sounds like this is worth the refactor into common code. Let me know if I can help at all with this.

jonathanolson commented 6 years ago

Implemented (pull capacitor-lab-basics and scenery-phet). Two main thoughts/concerns:

Certain things happen instantly (charge accumulating on the plate, and the light bulb "starting" to emit light). Do you need this to happen "instantly, but delayed until things play/step"?

The colors seem pretty bad for a11y. Should this be wrapped in a panel/background for CLB? Should the button colors change? I'm just assuming we'll need to do something:

clb-contrast

phet-steele commented 6 years ago

Current arrows still fade when the sim is paused. Is this correct @arouinfar?

arouinfar commented 6 years ago

Yikes, looks like I let this issue get a little stale...

@jonathanolson I was wondering if you had included the timer in your original estimate in https://github.com/phetsims/capacitor-lab-basics/issues/234#issuecomment-375723589?

Certain things happen instantly (charge accumulating on the plate, and the light bulb "starting" to emit light). Do you need this to happen "instantly, but delayed until things play/step"?

@jonathanolson I think we'll want to keep the "instantly, but delayed until things play/step" behavior. The play/pause/step controls will most likely be used to set up experiments and measure the decay time. This means that users need to be able change the battery voltage while paused. If the battery voltage can be changed while paused, the plate charges will also need to change while paused. Likewise, if moving the switch doesn't unpause the sim, the consequences of moving the switch should be seen (initial halo).

The colors seem pretty bad for a11y. Should this be wrapped in a panel/background for CLB? Should the button colors change? I'm just assuming we'll need to do something

Agreed that the colors are pretty awful. I think the blue background has great contrast against the bulb halo, so I am hesitant to change it. I tried your suggestion of putting the controls in a panel. It's a little heavy, but not terrible.

screen shot 2018-06-08 at 5 22 45 pm

Current arrows still fade when the sim is paused. Is this correct @arouinfar?

That's a bit complicated. I can think of two different scenarios which are at odds with one another.

  1. Current arrow appears because battery voltage is changed. In this case, it's an instantaneous change, so I would think the arrow should fade away.
  2. Current arrow appears because capacitor is discharging through bulb. A fading current arrow looks wrong because the lit bulb indicates that there is a current in the circuit. I think it makes more sense for the current arrow to be responsive to play/pause.

Even if we could implement the fade differently, depending on the switch position, I don't think we should. The behavior would look inconsistent/buggy. I think a fading current arrow in situation (2) is more problematic, so I would be inclined for the current arrow to always be responsive to play/pause.

ariel-phet commented 6 years ago

@Denz1994 is going to start working on this issue, and will consult @arouinfar as needed

Denz1994 commented 6 years ago

@arouinfar Can you review the current version of CLB on master for feedback? I added the timerNode and the panel around the play controls. Seems like a good checkpoint for review.

arouinfar commented 6 years ago

@Denz1994 the toolbox layout and i18n of the playback panel look good! The step size of 0.2 s also looks reasonable. The timerNode is responsive to play/pause/step, but it should also be responsive to normal/slow. Since the charges/halo appear instantaneously when paused, the current arrow should also appear (but not fade) while paused.

I'm still on the fence about putting the playback controls in a panel. The panel increases their visual weight more than I'd like, but I think the circuit and graphs still carry more weight, so I'm not super worried about flow. @ariel-phet can you take a look?

ariel-phet commented 6 years ago

@arouinfar in this case (given the sim background) I think a panel makes sense.

A few possible options:

  1. Tweak the background color on this screen

  2. If keeping the background color, lighten the visual appearance of the panel with the play/pause/slow motion controls. I think this could be accomplished with a more transparent panel and perhaps removing the stroke on the panel.

arouinfar commented 6 years ago

Thanks @ariel-phet!

I tried several different background colors, and it's tricky to find a non-blue color that contrasts well against the halo. I settled for the CCK background color rgb(153, 193, 255). This blue isn't quite as close to the play/pause color and it had good contrast with the halo.

To lighten up the panel, I got rid of the stroke, and changed the background color to white with 60% opacity.

Here's a mockup. Please ignore the outdated panels. image

@Denz1994 can you update the sim background color (for both screens) and update the playback panel appearance?

Denz1994 commented 6 years ago

I've implemented the above changes to CLB. Could you please review in master @arouinfar? Also, could you review the state of the current arrow when paused to ensure the arrow directions are correct?

Denz1994 commented 6 years ago

Also, could you check the behavior of the arrows when fading in response to the playback controls?

Last point: Some logic was added to fade the arrows when manually stepping through the sim. So if you discharge the capacitor via the light bulb and pause the sim (before the light bulb dims), you should be able to step through the light bulb dimming and see the current arrows fade shortly after in response to the manual steps.

The prior behavior kept the arrows visible even during the step.

arouinfar commented 6 years ago

Overall things are looking really nice @Denz1994!

One slightly strange thing is that the current arrows do not appear on pause, though they do behave normally once the user starts stepping forward. For example:

  1. Pause sim
  2. Charge capacitor, no current arrow appears
  3. Step forward, current arrow appears
  4. Continue stepping forward, current arrow fades as expected

The same behavior occurs when discharging the capacitor. When paused, the halo appears when the light bulb is connected (as it should) but the current arrow will not appear until the user starts stepping forward. If the user steps forward until the capacitor is fully discharged, the current arrows will correctly fade out.

@Denz1994 is it possible for the current arrows to appear in step (2) above? The same would apply to a discharging capacitor.

I've also noticed that the time step for the step forward button depends on playback speed: 0.20 s in Normal, 0.025 s in Slow. Generally, stepping forward should advance the sim by the same time step, regardless of Normal/Slow. A good comparison would be Pendulum Lab.

arouinfar commented 6 years ago

Tagging for #237 so this issue shows up there.

Denz1994 commented 5 years ago

An additional set of step functions have been added to LightBulbModel.js so the currentAmplitudeProperty can be updated while the sim is playing. A side effect of this is updating the visibility of the CurrentIndicatorNode when stepped through the sim.

Also, the model has been updated to support a dt step regardless of playback speed (slow/normal).

Please review master @arouinfar.

Also, please you review the behavior of the current arrows' behavior when toggling the switch between the capacitors and the lightbulb?

arouinfar commented 5 years ago

An additional set of step functions have been added to LightBulbModel.js so the currentAmplitudeProperty can be updated while the sim is playing. A side effect of this is updating the visibility of the CurrentIndicatorNode when stepped through the sim.

I'm not sure I follow. The current arrow was behaving perfectly fine when the sim was paused and stepped through. The only issue was that the current arrow wouldn't initially appear while paused, and that still seems to be the case. My question in https://github.com/phetsims/capacitor-lab-basics/issues/234#issuecomment-422924657 was if it would be possible for the current arrows to appear when paused (without needing to step forward).

For example, pause the sim, then charge the capacitor. The charges appear, but not the current arrows. image

After stepping forward once, the current arrow will appear. image

Likewise, connect the bulb while paused. The halo appears, but not the current arrows. image

Stepping forward once, the current arrows will appear. image

Also, the model has been updated to support a dt step regardless of playback speed (slow/normal).

Looks great!

Also, please you review the behavior of the current arrows' behavior when toggling the switch between the capacitors and the lightbulb?

This also looks good to me @Denz1994.

Denz1994 commented 5 years ago

It turns out my comment in git was asynchronous with my push. Sorry for the confusion.

@arouinfar Master wasn't updated at the time of the comment so could please you review again from master. Reassigning.

arouinfar commented 5 years ago

Thanks @Denz1994. Everything's looking good now, so I've tagged this issue with status:fixed-awaiting-deploy.

KatieWoe commented 5 years ago

I've noticed something odd about the disappearing behavior of the current arrows in dev.7. When unpaused it disappears after about 10 seconds. When paused and stepping through it starts to disappear almost immediately. @arouinfar is this intended?

Edit: I had the sim in slow motion I think. Sorry

KatieWoe commented 5 years ago

Also, is it just supposed to be on the light bulb screen, or should it be on both?

KatieWoe commented 5 years ago

Those are the only things of note on this issue in dev.7. Otherwise, this can likely be closed.

arouinfar commented 5 years ago

Edit: I had the sim in slow motion I think. Sorry

No worries! I double checked dev.7, and the fade is looking correct to me, though slower in Slow Motion.

Also, is it just supposed to be on the light bulb screen, or should it be on both?

The TimerNode and play/pause controls are only on the Light Bulb screen, as the first screen doesn't have any time-dependent behaviors.

Those are the only things of note on this issue in dev.7. Otherwise, this can likely be closed.

I'll go ahead and close, then. Thanks @KatieWoe!

arouinfar commented 4 years ago

This feature hasn't been deployed. I'm going to re-open and leave tagged as status:fixed-awaiting-deploy.