Cannot deselect circuit components on the Lab screen

[matthew-blackman]

I am unable to deselect a highlighted circuit component on the Lab screen. This does not occur in CCK-AC or CCK-Virtual Lab. To reproduce the issue:

1. Go to the Lab screen on CCK DC
2. Drag any component from the carousel
3. Select the component to show a yellow highlight
4. Click in the blue area to try and deselect the component

In mine and @samreid's initial testing, the component could not be deselected without selecting another component. This is a high-priority bug to address, since users are likely to run into this frequently on the Lab screen of the published sim.

[KatieWoe]

Confirmed on published. Assigning @samreid

[samreid]

Through investigation with this patch, I have identified the troublesome area:

```diff Subject: [PATCH] Add TODOs, see https://github.com/phetsims/density-buoyancy-common/issues/154 --- Index: js/view/CircuitNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/view/CircuitNode.ts b/js/view/CircuitNode.ts --- a/js/view/CircuitNode.ts (revision c6a08ea65f78942da48ed6dca37b3fe04442fcbb) +++ b/js/view/CircuitNode.ts (date 1717593895237) @@ -593,6 +593,7 @@ } // listener for 'click outside to dismiss' + console.log('add listener'); phet.joist.display.addInputListener( new DisplayClickToDismissListener( ( event: SceneryEvent ) => { // if the target was in a CircuitElementEditContainerNode, don't dismiss the event because the user was @@ -604,8 +605,22 @@ return node instanceof CircuitElementEditContainerNode || node instanceof CircuitElementNode || node instanceof VertexNode; } ); + console.log( 'click to dismiss' ); + // debugger; + if ( trails.length === 0 ) { + const oldSelection = this.circuit.selectionProperty.value; + this.circuit.selectionProperty.value = null; + this.circuit.dirty = true; + // debugger; + + // iterate over all CircuitElementNodes and mark as dirty + + Object.values( this.circuitElementNodeMap ).forEach( value => { + // debugger; + value.dirty = true; + } ); } } ) ); } Index: js/model/Circuit.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/model/Circuit.ts b/js/model/Circuit.ts --- a/js/model/Circuit.ts (revision c6a08ea65f78942da48ed6dca37b3fe04442fcbb) +++ b/js/model/Circuit.ts (date 1717593739448) @@ -282,8 +282,12 @@ phetioValueType: NullableIO( ReferenceIO( OrIO( [ CircuitElement.CircuitElementIO, Vertex.VertexIO ] ) ) ), phetioFeatured: true } ); + this.selectionProperty.link( selection => { + console.log( 'selection changed: ', selection === null ? null : selection.phetioID ); + } ); const emitCircuitChanged = () => { + // debugger; this.dirty = true; this.circuitChangedEmitter.emit(); }; @@ -903,6 +907,7 @@ * dirty and compute in step if anything has changed. */ private markDirty(): void { + // debugger; this.dirty = true; } Index: js/view/WireNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/view/WireNode.ts b/js/view/WireNode.ts --- a/js/view/WireNode.ts (revision c6a08ea65f78942da48ed6dca37b3fe04442fcbb) +++ b/js/view/WireNode.ts (date 1717593497263) @@ -220,6 +220,7 @@ * When the view type changes (lifelike vs schematic), update the node */ const markAsDirty = () => { + console.log( 'hello mark as dirty' ); if ( this.isDisposed ) { return; } @@ -298,6 +299,7 @@ } ); this.addInputListener( this.dragListener ); + console.log('WIRE TO SELECTION PROPERTY'); circuitNode!.circuit.selectionProperty.link( markAsDirty ); } else { @@ -355,6 +357,7 @@ * CCKCLightBulbNode calls updateRender for its child socket node */ public updateRender(): void { + console.log( 'updat.e render ' ); const view = this.viewTypeProperty.value; if ( view === CircuitElementViewType.LIFELIKE ) { Index: js/view/CircuitElementNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/view/CircuitElementNode.ts b/js/view/CircuitElementNode.ts --- a/js/view/CircuitElementNode.ts (revision c6a08ea65f78942da48ed6dca37b3fe04442fcbb) +++ b/js/view/CircuitElementNode.ts (date 1717593184989) @@ -82,6 +82,7 @@ // Flag to indicate when updating view is necessary, in order to avoid duplicate work when both vertices move this.dirty = true; + debugger; this.disposeEmitter.addListener( () => circuitElement.startDragEmitter.removeListener( startDragListener ) ); } @@ -89,6 +90,7 @@ * Mark dirty to batch changes, so that update can be done once in view step, if necessary */ protected markAsDirty(): void { + debugger; this.dirty = true; } @@ -133,7 +135,9 @@ * called during the view step */ public step(): void { + console.log( 'step' ); if ( this.dirty ) { + console.log( 'was dirty' ); this.updateRender(); this.dirty = false; ```

The problem is that DisplayClickToDismissListener does not support more than one instance. For instance, testing on main for CCK-AC, neither screen 2 nor 3 can deselect circuit elements. However, if running any single screen with ?screens=2 or ?screens=3, it is fine.

DisplayClickToDismissListener is only used by Circuit Construction Kit, and attaches to pointer itself on press. Hence when there are 3 DisplayClickToDismissListeners, only the 1st one (for screen 1) attaches to the pointer. The others fail to attach because of this line:

https://github.com/phetsims/joist/blob/084b04e87cf07445a11a0c7915984617795ddad1/js/DisplayClickToDismissListener.ts#L59-L65

This code has been out for review in https://github.com/phetsims/joist/issues/770 since Feb 2022. What is the best way to address this problem?

[jessegreenberg]

Thanks for identifying the problem. A solution has been committed in https://github.com/phetsims/joist/issues/770, would you like to review or verify?

It sounds like this is in the published version and we may want to redeploy CCK sims, let me know if I should do that or help with that.

[samreid]

The problem is in the DC sim but not in the AC sim. @kathy-phet @matthew-blackman should we republish? Or just when we publish with changes for data fluency?

I confirmed @jessegreenberg fixes are great, thanks!