Alternative Input

[AgustinVallejo]

2023-02-27 (AV and MK):

• First, we are turning on the default alt input for MSS. This will help us guide some design questions.
• Some buttons and PDOM Order were moved around, more work would be needed here.
• IMPORTANT: We should make sure to remove MSS from the interactive description list if we're not going to proceed with this.

[zepumph]

We have been working on adding KeyboardDragListener to the BodyNodes, and it is going well. We found one spot where KeyboardDragListener doesn't have the same API as DragListener, called mapPosition, and we should fix that! This will add support for the drag bounds in BodyNode.

[zepumph]

KeyboardDragListeners were added to MeasuringTapeNode in phetsims/scenery-phet#798. It went pretty well and wasn't too challenging! Let's keep working on this next week @AgustinVallejo.

[zepumph]

• We have to be very careful to not accidentaly publish keypad alt input without reviewing the changes over in Add keyboard input to Keypad scenery-phet#790

[zepumph]

• For the number display, it already has a FireListener added to it, which is well set up for pdom input, we just need to add it to the PDOM with a tagName. Try this patch out

Subject: [PATCH] mark Poolable as deprecated, https://github.com/phetsims/phet-core/issues/103
---
Index: js/common/view/InteractiveNumberDisplay.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/InteractiveNumberDisplay.ts b/js/common/view/InteractiveNumberDisplay.ts
--- a/js/common/view/InteractiveNumberDisplay.ts	(revision b640e76a15983d4338224dbd89e620f875b60b6b)
+++ b/js/common/view/InteractiveNumberDisplay.ts	(date 1678208831489)
@@ -80,7 +80,9 @@
       ], ( isUserControlled, isKeypadActive, backgroundColor ) => {
         return isUserControlled || isKeypadActive ? backgroundColor.colorUtilsBrighter( 0.7 ) : Color.WHITE;
       } ),
-      backgroundStroke: Color.BLACK
+      backgroundStroke: Color.BLACK,
+
+      tagName: 'button'
     }, providedOptions );
 
     super( property, range, options );

[zepumph]

RE: f6f45eb

Sorry if there will be a merge conflict @AgustinVallejo, but I needed it to make progress on phetsims/scenery-phet#790 and get a review on master.

[zepumph]

• The value of the numberDisplay in the PDOM does not update when the mass changes. Likely we shouldn't store it like that de8a9f4

[zepumph]

• It would be nice to have the outline match the shape of the body. phetsims/solar-system-common@e4c03ce
• It would be nice to have the outline match the shape of the veclocity draggable.
• Investigate the order of return objects button. Let's put it just after the "Timing Controls" section.
• Finish Add KeyboardDragListeners to MeasuringTapeNode scenery-phet#798

[AgustinVallejo]

Pending with this:

• Fix the DragListener on DraggableVectorNode and add a KeyboardDragListener. (AV MK meeting today to work on this.)
• Set up a proper a11y logic for the sliders. (see Use NumberControl instead of custom button+slider in SolarSystemCommonSlider #105)
• Not sure if this counts as alt input but was discussed in the meeting: Add a hover listener on InteractiveNumberDisplays so you know they are actually interactive, even if we don't do hover on touch screen devices. (MK and SR do not have enough info to work on this part)

[zepumph]

Working on this now.

[samreid]

Subject: [PATCH] Compound key directions instead of favoring left/up, see https://github.com/phetsims/scenery/issues/1549
---
Index: js/view/DraggableVectorNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/view/DraggableVectorNode.ts b/js/view/DraggableVectorNode.ts
--- a/js/view/DraggableVectorNode.ts	(revision a5a58d9cb76bb2e0a92dcc553ecfe2da797e2c9b)
+++ b/js/view/DraggableVectorNode.ts	(date 1679508929265)
@@ -138,6 +138,8 @@
       }
     } );
 
+    body.userControlledVelocityProperty.debug( 'hello' );
+
     // Add the drag handler
     const start = () => {
       body.userControlledVelocityProperty.value = true;
@@ -152,8 +154,13 @@
       transform: transformProperty,
       positionProperty: vectorPositionProperty,
       canStartPress: () => !body.userControlledVelocityProperty.value,
-      start: start,
-      end: end
+      start: () => {
+        keyboardDragListener.interrupt();
+        start();
+      },
+      end: end,
+
+      // blur should also reset userControlledVelocityProperty
     } );
     grabArea.addInputListener( dragListener );
     // move behind the geometry created by the superclass
@@ -165,8 +172,13 @@
       transform: transformProperty,
       dragVelocity: 450,
       shiftDragVelocity: 100,
-      start: start,
-      end: end
+      start: () => {
+        dragListener.interrupt();
+        start();
+      },
+      end: end,
+
+      // blur should also reset userControlledVelocityProperty
     } );
     this.addInputListener( keyboardDragListener );
 

[zepumph]

After a brief talk with @jessegreenberg about the above patch. It seems best to commit the interrupts. The general solution seems incredibly challenging to make sure that all types of drag listeners know about all types of pointer to know how to interface between them.

I'll commit shortly.

[zepumph]

I see no usages of accessibleName: ' or innerContent: '. Yay!

[zepumph]

More good work done here. In my mind, we are fast approaching diminishing returns as it pertains to the publication of MSS, but each time @samreid and I have spent time here, we have found really good questions that warrant more thought. Ideally here would be what would happen next in this issue. I think that anyone could review if they had an open and inquiring mind about this:

• Looking at the dragging listeners in BodyNode, anything strange? Is the behavior and dragging velocities correct?
• Should the BodyNode dragging listeners get the same multiple-interrupts that the vectors got in phetsims/solar-system-common@9cf9e93? I think so!
• How were accessibleNames added to the sim? When not using accessibleName, but instead innerContent/ariaLabel/etc, is it worth it. Would it be preferred to change the accessibleNameBehavior of that type instead of using a "lower level" API option?
• Look through the above commits, anything stand out or suspicious? (note there are hidden comments)
• Please make sure to note that this issue's review largely covers the discussion and work done over in Interactive Description #103 where it was decided exactly what features to implement (alt input, screen summaries, accessibleNames, and NOTHING ELSE), and where some of it was committed against.

[samreid]

I'm not planning further work on this issue at the moment, so will self-unassign and assign members from the subteam for this sim. Please invite me back if needed.

[zepumph]

Yes, and me as well. Please let me know if you want help or consulting on the above points.

[arouinfar]

@AgustinVallejo and I reviewed the checklist in #86 (comment). @AgustinVallejo will check off the items he can, and we will ask @jonathanolson to consult on the remaining items.

[AgustinVallejo]

04/07: With @arouinfar and @jonathanolson we considered that the way accessibleNames are added to the sim are not a main priority right now, but it could be looked into in the future. For now, what we have is sufficient for the level of description the sim has.