autofill still occurs with invisible dropper

[loganbraywork]

Test Device

Windows 10 Laptop

Operating System

Windows 10 v.1903

Browser

Chrome Version 84.0.4147.89

Problem Description

For phetsims/qa#515 & phetsims/qa#514

When the query parameter autoFill is set to true and the dropper is no longer visible via Studio, the filling animation still occurs as if the dropper was visible.

Visuals

[pixelzoom]

I don't see anything in the Client Guide that asks to be able to hide the dropper. So the easy solution to this is to make dropperNode.visibleProperty read-only.

The more involved solution is to autofill only if the dropper is visible. Autofill is handled in the model, so this would require adding a new phet-io-only Property, dropperVisibleProperty, in the model, have it control the visibility of the dropper view, and makedropperNode.visibleProperty read-only.

@arouinfar how do you want to proceed?

[arouinfar]

Nice find @KatieWoe.

I don't see anything in the Client Guide that asks to be able to hide the dropper. So the easy solution to this is to make dropperNode.visibleProperty read-only.

That sounds like a perfect solution to me @pixelzoom. Let's do it.

I don't think we need to complicate things with the dropperVisibleProperty, though it's good to know there's a potential solution if a future client insists on being able to hide the DropperNode.

[pixelzoom]

dropperNode.visibleProperty is now read-only. Committed to master, patched into 1.4 branches of ph-scale and ph-scale-basics. To be regression tested in new RC.

[pixelzoom]

To verify in the next RC:

1. Run the sim in Studio
2. Select the "All" radio button for "PhET-iO Elements".
3. Verify that phScale.macroScreen.view.dropperNode.visibleProperty and phScale.microScreen.view.dropperNode.visibleProperty are read-only.

[brooklynlash]

Instead of it being read-only, it does not even seem to exist on Chromebook. Not sure if this is intended or not.

[loganbraywork]

As far as Windows Chrome and Firefox go, I can see both phScale.microScreen.view.dropperNode.visibleProperty & phScale.macroScreen.view.dropperNode.visibleProperty as read only, so it seems fixed to me

[liammulh]

phScale.microScreen.view.dropperNode.visibleProperty is read-only on macOS.

[pixelzoom]

@brooklynlash said:

Instead of it being read-only, it does not even seem to exist on Chromebook. Not sure if this is intended or not.

@brooklynlash Did you have "PhET-iO Elements" set to "All" or "Features"? This is not a featured element, so you'll need to select the "All" radio button, as shown in the screenshot below. Please verify whether that fixes the problem. And apologies that this was not specified in the instructions in #178 (comment), I've updated them.

[brooklynlash]

Yes, my mistake again. I see it now, thank you.

[pixelzoom]

Excellent, thanks @brooklynlash.

Closing.

[arouinfar]

@kathy-phet saw this issue and messaged me on Slack:

I can think of a reason a client might want to do that - if they want to create something really focused on dilution and not have the dropper node visible - since that complicates when you add back into the solution. So I think I would lean to adding this visible property to control that if its not too much work for CM? What do you think about this potential use ...?

I hadn't previously thought of this use case, but @kathy-phet makes a good point. Hiding the dropper would be useful in a lesson focused on dilution. I would recommend that we revert this change and make dropperNode.visibleProperty phetioReadOnly: false.

This means that autofill will occur through an invisible dropper, so we should decide what (if anything) to do. Some options include:
(1) Do nothing, but document the issue in the client guide. If clients want to hide the dropper, they should not switch solutes.
(2) Keep autofill, but hide the stream dispensed from the dropper.
(3) Disable autofill. Switching solutes will result in a pre-filled beaker.

I would be fine with any of these options. @pixelzoom what would you recommend?

[pixelzoom]

(1) requires no work.

(2) is significantly easier than (3).

Let me know how you want to proceed.

[arouinfar]

@pixelzoom let's proceed with (2). It prevents us from telling users, "if you do X, you also have to do Y".

[pixelzoom]

@arouinfar Question: In ph-scale-basics, the default is autofill: false. So if you make the dropper invisible, nothing will happen when you switch solutes via the combo box. The client would have to know to start the sim with autofill: true, or change phScaleBasics.macroScreen.model.autofillEnabledProperty. Is this OK?

Or should the autofill setting be ignored when the dropper is made invisible, and always autofill? This seems a little more complicated to describe, and potentially confusing.

Either way, something about this probably needs to be added to the Client Guide for ph-scale and ph-scale-basics. Something like "if you make the dropper invisible, then be sure to enable autofill".

[pixelzoom]

In the above commit, I added model.dropper.visibleProperty, which is linked to view.dropperNode.visibleProperty and the dropperFluidNode (which is internal, and not instrumented). So if you set model.dropper.visibleProperty, the dropper and the fluid that comes out of the dropper will be hidden. @arouinfar please verify in master.

@arouinfar and I discussed #178 (comment) via Zoom, and agreed that ignoring autofill setting could be confusing. So @arouinfar will add appropriate info to the Client Guides for ph-scale and ph-scale-basics, indicating that autofill should be enabled if you're going to hide the dropper.

[arouinfar]

@pixelzoom the behavior looks good in master. I've edited the client guides in the above commits, so closing.