Text that appears when switch is selected should be instrumented

[arouinfar]

Discovered during #797

When a switch is selected, there is a string at the bottom of the screen that reads The switch is open. or The switch is closed..

I expected to find this text under circuitElementEditContainerNode since it seems similar to the tapInstructionTextNode and other Buttons and NumberControls housed within it. I didn't get any hits from autoselect or search, so I don't think it's instrumented.

I don't think this is a serious enough issue to prevent the client preview, so I've added this to the full release milestone.

[arouinfar]

Seems like a reasonable place for it would be under view.circuitElementEditContainerNode and call it something like switchTextNode with its own textProperty (read-only) and visibleProperty.

[samreid]

Upon review, I saw that the switch readout node has 2 children that it switches between. This seemed a good match for PhET-iO because the client can change how the text should read in the textNode and it will apply to all switches. The tree looks like this:

@arouinfar does that seem OK?

[samreid]

I also made the visibleProperty phetioReadOnly: true in the commit. But at the moment there is no way for a client to hide both the texts. The trash button is a child of the SwitchReadoutNode, so if I instrument that then hiding both texts will also hide the trash button. Do we need another container so that the client can hide the open/closed texts but not the trash button?

[arouinfar]

Thanks @samreid, this looks good in master.

The trash button is a child of the SwitchReadoutNode, so if I instrument that then hiding both texts will also hide the trash button. Do we need another container so that the client can hide the open/closed texts but not the trash button?

Adding a container that allows users to hide the open/closed text independently of the trash button would be consistent with how we've treated the other elements in the circuitElementEditContainerNode, so I would recommend this change.

[samreid]

I added a messageNode container which is working well. This is ready for review, but I wanted to ask some specific questions about the desired behavior.

• Currently switchReadoutNode contains the garbage can too. But it sounds like it is just the readout. What should this name be?
• Should the garbage can be centered if the messageNode is hidden?
• Want to rename messageNode?

[arouinfar]

I added a messageNode container which is working well.

Thanks @samreid. I agree it's all looking good in master and links back to the appropriate stringProperty.

Currently switchReadoutNode contains the garbage can too. But it sounds like it is just the readout. What should this name be?

It's a child of the circuitElementEditContainerNode, so maybe switchContainerNode? I'm not too worried about the element name since it's clearly related to the switch and testing out its visibleProperty will make its contents pretty obvious.

Should the garbage can be centered if the messageNode is hidden?

Ideally, yes, but I think that applies to all variations of edit panels. If some of the contents of the element edit panel are hidden, it would be nice to re-center the panel. However, I would consider this non-essential.

Want to rename messageNode?

No, messageNode sounds good to me.

[matthew-blackman]

@samreid and I discussed what to name the container Node that appears at the bottom of the screen when a circuit component is highlighted. For most components this container will have controls to edit the component, but for the switch it only shows a message and the trash button. The container is currently called 'switchReadoutNode' which doesn't seem to match what it is, but changing it to 'switchMessageContainerNode' could create issues if we ever want to add features/options to the switch component.

In response to @arouinfar's request, we discussed changing the container name to 'switchContainerNode', but this sounds like the node should contain a switch, which it does not.

An idea that @samreid mentioned is to rename the component-specific UI to 'circuitElementControlNode', with specific cases having names like 'resistorControlNode', 'switchControlNode' etc.

[arouinfar]

An idea that @samreid mentioned is to rename the component-specific UI to 'circuitElementControlNode', with specific cases having names like 'resistorControlNode', 'switchControlNode' etc.

This is a great idea @samreid. I think part of our current troubles is that the contents of circuitElementEditContainerNode look like a bit of a hodgepodge. Your proposal would clean it up significantly, I think.

[matthew-blackman]

@samreid and I discussed this and agree that the current heirarchy of circuitElementEditContainerNode makes sense even though the children are of different types. Some components like the trash button are being shared across multiple components, so having them as direct children of circuitElementEditContainerNode is helpful for simplicity.

The components that end in 'NumberControl' are already a specific type of Node (distict from a Panel), and we don't think it makes sense to nest it inside another Node or to rename as the current system is consistent.

@arouinfar is the current system workable? Did we miss anything?

[arouinfar]

@matthew-blackman let's review this one together in #917

[arouinfar]

Reviewed with @matthew-blackman and the current structure looks reasonable.

We also now have the select feature to make the components easier to find in the tree. Closing.