Focus Order for alternative Inputs

[DianaTavares]

The simulation doesn't have the right order of selection for alternative -inputs. I make a suggestion in this document.

Can you review it @arouinfar ?

[arouinfar]

Looks good @DianaTavares! I noticed the scale was missing on the Applications screen, so I added it to the doc as a suggestion.

The only thing I'm not sure about is where to put the Block control panels. They're currently after Forces and Fluid Density. This order prioritizes turning on the desired forces before experimenting with the blocks and it also minimizes how much focus moves around the screen. However, I think it's more likely the users will first experiment with the block material/mass/volume, so I would consider moving the Block control panels before the Forces panel.

Focus order is relatively easy to change, so you could try both options with the devs and see which you prefer.

[DianaTavares]

Finish! I take your advice @arouinfar, of moving above in the order the controls that modify the material/mass/volume, and also the fluid density. In the beginning OI thought in the first interacion with the sim, but is logic that after that first interaction where user ant to explore all controls, then is going to be moving only between the block, change its properties and change the properties of the fluid in the pool, then is logic that those three are close together. Ready to implement!

[zepumph]

@DianaTavares, do you want to add a section for Buoyancy: Basics too?

[DianaTavares]

Added to my ToDo list!

[zepumph]

Tagging phetsims/projectile-data-lab#64 as previous work in this area. @samreid @DianaTavares will reach out to @terracoda to confirm the order before implementation.

[DianaTavares]

I finish the design of the Focus Order and Help dialog.

Buoyancy
Buoyancy Basics

But first, we need to address the issue related to the change in the fluid control in the explore screen.
For Buoynacy we need to address this issue: phetsims/buoyancy#150

[samreid]

Patch for updating the pdom order when blocks are created:

Subject: [PATCH] Update focus order for the explore screen, see https://github.com/phetsims/density-buoyancy-common/issues/121
---
Index: js/common/view/DensityBuoyancyScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/DensityBuoyancyScreenView.ts b/js/common/view/DensityBuoyancyScreenView.ts
--- a/js/common/view/DensityBuoyancyScreenView.ts	(revision d4aa9af852b5ef4ec0226b3d9dc374e0f92353d1)
+++ b/js/common/view/DensityBuoyancyScreenView.ts	(date 1715199777824)
@@ -64,6 +64,7 @@
 import DuckView from '../../buoyancy/view/DuckView.js';
 import NumberIO from '../../../../tandem/js/types/NumberIO.js';
 import Utils from '../../../../dot/js/Utils.js';
+import createObservableArray, { ObservableArray } from '../../../../axon/js/createObservableArray.js';
 
 // constants
 const MARGIN = DensityBuoyancyCommonConstants.MARGIN;
@@ -100,7 +101,7 @@
 
   private readonly massDecorationLayer = new MassDecorationLayer();
 
-  public readonly massViews: MassView[];
+  public readonly massViews: ObservableArray<MassView>;
 
   private readonly debugView?: DebugView;
 
@@ -159,7 +160,7 @@
 
     this.addChild( this.massDecorationLayer );
 
-    this.massViews = [];
+    this.massViews = createObservableArray<MassView>();
 
     this.sceneNode.stage.threeCamera.zoom = options.cameraZoom;
     this.sceneNode.stage.threeCamera.up = new THREE.Vector3( 0, 0, -1 );
Index: js/buoyancy/view/BuoyancyIntroScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/buoyancy/view/BuoyancyIntroScreenView.ts b/js/buoyancy/view/BuoyancyIntroScreenView.ts
--- a/js/buoyancy/view/BuoyancyIntroScreenView.ts	(revision d4aa9af852b5ef4ec0226b3d9dc374e0f92353d1)
+++ b/js/buoyancy/view/BuoyancyIntroScreenView.ts	(date 1715200278299)
@@ -34,6 +34,9 @@
 import FluidSelectionPanel from './FluidSelectionPanel.js';
 import CuboidView from '../../common/view/CuboidView.js';
 import ScaleView from '../../common/view/ScaleView.js';
+import MassView from '../../common/view/MassView.js';
+import Checkbox from '../../../../sun/js/Checkbox.js';
+import BooleanProperty from '../../../../axon/js/BooleanProperty.js';
 
 const MARGIN = DensityBuoyancyCommonConstants.MARGIN;
 
@@ -170,15 +173,21 @@
 
     this.addChild( this.popupLayer );
 
-    const cuboidViews = this.massViews.filter( massView => massView instanceof CuboidView );
+    // const cuboidViews = this.massViews.filter( massView => massView instanceof CuboidView );
     const scaleViews = this.massViews.filter( massView => massView instanceof ScaleView );
 
+    const myCuboidLayer = new Node( {
+      pdomOrder: []
+    } );
+    this.addChild( myCuboidLayer );
+
     // The focus order is described in https://github.com/phetsims/density-buoyancy-common/issues/121
     this.pdomPlayAreaNode.pdomOrder = [
 
       // TODO: When the cuboids change from the radio buttons, this needs to update. So consider a different parent for this part
       // see https://github.com/phetsims/density-buoyancy-common/issues/121
-      ...cuboidViews.map( cuboidView => cuboidView.focusablePath ),
+      // ...cuboidViews.map( cuboidView => cuboidView.focusablePath ),
+      myCuboidLayer,
 
       blocksRadioButtonGroup,
 
@@ -190,6 +199,30 @@
       displayOptionsPanel
     ];
 
+    setTimeout( () => {
+      const checkbox = new Checkbox( new BooleanProperty( true ), new Text( 'htello' ) );
+      displayOptionsPanel.addChild( checkbox );
+      myCuboidLayer.pdomOrder = [ ...myCuboidLayer.pdomOrder!, checkbox ];
+    }, 5000 );
+
+    const listener = ( massView: MassView ) => {
+
+      if ( massView instanceof CuboidView ) {
+        // const cuboidViews = this.massViews.filter( massView => massView instanceof CuboidView );
+        // const scaleViews = this.massViews.filter( massView => massView instanceof ScaleView );
+
+        myCuboidLayer.pdomOrder!.push( massView.focusablePath );
+      }
+
+
+    };
+    this.massViews.addItemAddedListener( listener );
+    this.massViews.forEach( listener );
+
+    this.massViews.addItemRemovedListener( massView => {
+      // nothing to do since disposal of the node will remove it from the pdom order
+    } );
+
     this.pdomControlAreaNode.pdomOrder = [
       this.readoutPanelsVBox,
       this.resetAllButton

[samreid]

• The design specified the boat controls to be before the boat. I found it easiest to put the bottle in the boat layer in the pdom. But that means in the bottle scene, the bottle controls are before the bottle in the tab order. Is that OK?
• The density description prototype is overriding the keyboard focus order. How should I proceed? https://github.com/phetsims/density/blob/main/js/description/density-description-logic.js

[DianaTavares]

I reviewed the interaction and it looks great!! Related to you comment @samreid

The design specified the boat controls to be before the boat. I found it easiest to put the bottle in the boat layer in the pdom. But that means in the bottle scene, the bottle controls are before the bottle in the tab order. Is that OK?

I play with this and look super strange that the first thing that is moved in the bottle is not the bottle. It breaks the coherency with the other screens. I understand that it is for the relationship of bottle and boat, that are part of the same screen, but in that case, I prefer that the bottle is first control in bottle, and in boat the order is: block, boat and next the panel control (I already modify it in the design doc).

UPDATE: I also noticed a non-idiomatic grab-release pattern for the blocks (you have to tab away to release the block). Is that by design? If not, is there an issue to change it?

@terracoda today shares in the designing meeting other sims where you have to click space to grab (Like Faraday's Law. But the reason that those sims are like that is because they have interactive descriptions. The simulation that doesn't have interactive descriptions, like MSS, Kepler Quadrilateral, are ok with this behavior of release when the focus changes to another object.

[samreid]

Thanks, I pushed the change to the bottle/boat order. @DianaTavares please test on phettest. Please close if all is well.

[DianaTavares]

Working well!!

[phet-dev]

Reopening because there is a TODO marked for this issue.

[samreid]

The TODOs are like this and were meant as a question for the reviewer:

        // TODO: https://github.com/phetsims/density-buoyancy-common/issues/121 There is a bit of back and forth here that is
        // unnecessary. Is there a better way? Also, is it OK to tie it to the tandem name?
        const proposedOrder = [ ...cuboidPDOMLayer.pdomOrder!, massView.focusablePath ];

[zepumph]

How does that look?

[samreid]

That is much better, thanks! Closing.