Connection highlight doesn't disappear

[Nancy-Salpepi]

Test device MacBook Air (m1 chip)

Operating System 12.0.1

Browser Chrome

Problem description For https://github.com/phetsims/qa/issues/772

If a connection between 2 elements is selected and then another element is removed from the carousel, the yellow highlight doesn't disappear--only the cut symbol does. The connection will stay highlighted until you select another element. In the published versions of CCK-DC and CCK-AC the yellow highlight disappears once an item is selected from the carousel.

Steps to reproduce

1. On either screen, remove any two items from the carousel and connect them
2. Click on the connection
3. Remove another item from the carousel

Visuals

Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Circuit Construction Kit: DC‬ URL: https://phet-dev.colorado.edu/html/circuit-construction-kit-dc/1.3.0-dev.14/phet/circuit-construction-kit-dc_all_phet.html Version: 1.3.0-dev.14 2022-02-01 01:48:26 UTC Features missing: applicationcache, applicationcache, touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/97.0.4692.99 Safari/537.36 Language: en-US Window: 1440x693 Pixel Ratio: 2/1 WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium) GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 31 uniform: 1024 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 80) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {}

[Nancy-Salpepi]

It will also stay highlighted if you select any of the radio buttons or meters.

[arouinfar]

Nice find @Nancy-Salpepi.

@samreid it seems like vertex.isSelectedProperty is not being properly toggled. It looks rough, but I don't think it seriously impacts the usability of the sim. The selected highlight around the vertex will be dismissed if clicking in a blank area of the play area or if selecting a different element/vertex.

I'll tag this for the preview milestone to document as a known bug, but I think we can defer fixing it until production.

[arouinfar]

I've documented this as a known issue, so switching the milestone.

[samreid]

Should I rewrite the sim to have a single Property indicating what is selected, and it can be a Circuit Element OR Vertex? Perhaps selectionProperty? Then we would uninstrument the vertex.isSelectedProperty values and just use them as internal plumbing to get the same behavior? Would we ever want the user to be able to select more than one thing? (Like a drag selection)?

[arouinfar]

Should I rewrite the sim to have a single Property indicating what is selected, and it can be a Circuit Element OR Vertex?

I really like this idea. That way, the client only has to look at one property to know exactly what the user has selected.

Would we ever want the user to be able to select more than one thing? (Like a drag selection)?

No, I don't think so. If multiple elements are simultaneously selected, we would need to display multiple edit panels/trash buttons which would be really messy and confusing.

[samreid]

I implemented the proposal so there is one selectionProperty which can be CircuitElement | Vertex | null. It is easy to change the selection from studio using "get" --> "set". This also fixes the vertex highlighting bug. Ready for review.

[arouinfar]

Thanks @samreid. I really like selectionProperty, and it seems powerful that users can pass in the phetioID of the element they want to highlight. However, it doesn't seem like Studio does any error checking to make sure the phetioID exists when using Set Value. Should it?

[samreid]

@matthew-blackman and I tested this in a built version. We tried selecting a battery that doesn't exist. It showed this alert dialog and did not crash the sim:

We cannot use a standard validator in this place for the rich studio error reporting since trying to deserialize the non-existent circuit element happens before validation. So I'm not sure what we can do without adding another complex validation layer or channel.

@arouinfar what do you recommend?

[arouinfar]

@matthew-blackman and I tested this in a built version. We tried selecting a battery that doesn't exist. It showed this alert dialog and did not crash the sim:

That seems good enough to me, closing.