marlitas
commented
1 year ago Made some progress here, but the slider functionality for the cards needs work. It will navigate to cards, but we need to figure out which property is going to be best to move the cards along. It could be the cellPositionProperty, but we'll need a way to adjust the enabledRangeProperty as the list of cards grows. We'll also need a way to track it's old value... I'm wondering if some of this could be done through a link to the cellPositionProperty, but I ran out of time to really investigate things.
Here's a patch that will hopefully help a bit.
```diff
Subject: [PATCH] Set pdom order for Median screen and add alternative input to cards
---
Index: js/median/view/MedianAccordionBox.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/median/view/MedianAccordionBox.ts b/js/median/view/MedianAccordionBox.ts
--- a/js/median/view/MedianAccordionBox.ts (revision a993c261b5a60d7cb40d94f8ccc0a19510f52632)
+++ b/js/median/view/MedianAccordionBox.ts (date 1689199740546)
@@ -47,8 +47,9 @@
margin: CAVConstants.ACCORDION_BOX_HORIZONTAL_MARGIN
} );
- backgroundNode.addChild( new CAVInfoButton( model.isInfoVisibleProperty, backgroundShape, tandem.createTandem( 'infoButton' ) ) );
+ const infoButton = new CAVInfoButton( model.isInfoVisibleProperty, backgroundShape, tandem.createTandem( 'infoButton' ) );
+ backgroundNode.addChild( infoButton );
backgroundNode.addChild( cardNodeContainer );
backgroundNode.addChild( checkboxGroupAlignBox );
@@ -62,6 +63,12 @@
} );
this.cardNodeContainer = cardNodeContainer;
+
+ this.pdomOrder = [
+ this.cardNodeContainer,
+ checkboxGroup,
+ infoButton
+ ];
}
}
Index: js/median/view/CardNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/median/view/CardNode.ts b/js/median/view/CardNode.ts
--- a/js/median/view/CardNode.ts (revision a993c261b5a60d7cb40d94f8ccc0a19510f52632)
+++ b/js/median/view/CardNode.ts (date 1689201086743)
@@ -14,7 +14,6 @@
import SoccerBall from '../../soccer-common/model/SoccerBall.js';
import Vector2 from '../../../../dot/js/Vector2.js';
import CardModel from '../model/CardModel.js';
-import PickRequired from '../../../../phet-core/js/types/PickRequired.js';
import CardNodeContainer from './CardNodeContainer.js';
import SoundClip from '../../../../tambo/js/sound-generators/SoundClip.js';
import soundManager from '../../../../tambo/js/soundManager.js';
@@ -22,9 +21,12 @@
import cvCardDropSound_mp3 from '../../../sounds/cvCardDropSound_mp3.js';
import CAVQueryParameters from '../../common/CAVQueryParameters.js';
import CAVConstants from '../../common/CAVConstants.js';
+import AccessibleSlider, { AccessibleSliderOptions } from '../../../../sun/js/accessibility/AccessibleSlider.js';
+import WithRequired from '../../../../phet-core/js/types/WithRequired.js';
type SelfOptions = EmptySelfOptions;
-export type CardNodeOptions = SelfOptions & NodeOptions & PickRequired;
+type ParentOptions = WithRequired & WithRequired;
+export type CardNodeOptions = SelfOptions & ParentOptions;
export const cardPickUpSoundClip = new SoundClip( cvCardPickupSound_mp3, {
initialOutputLevel: 0.3,
@@ -42,7 +44,7 @@
export const PICK_UP_DELTA_X = -4;
export const PICK_UP_DELTA_Y = -4;
-export default class CardNode extends Node {
+export default class CardNode extends AccessibleSlider( Node, 0 ) {
public readonly dragListener: DragListener;
@@ -73,10 +75,14 @@
children: [ card ]
} );
- const options = optionize()( {
+ const options = optionize()( {
children: [ offsetContainer ],
cursor: 'pointer',
- phetioVisiblePropertyInstrumented: false
+ phetioVisiblePropertyInstrumented: false,
+ keyboardStep: 1,
+ shiftKeyboardStep: 1,
+ pageKeyboardStep: 5,
+ roundToStepSize: true
}, providedOptions );
super( options );
Index: js/median/view/CardNodeContainer.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/median/view/CardNodeContainer.ts b/js/median/view/CardNodeContainer.ts
--- a/js/median/view/CardNodeContainer.ts (revision a993c261b5a60d7cb40d94f8ccc0a19510f52632)
+++ b/js/median/view/CardNodeContainer.ts (date 1689201086728)
@@ -38,6 +38,7 @@
import CardContainerModel from '../model/CardContainerModel.js';
import CardModel from '../model/CardModel.js';
import CAVSoccerSceneModel from '../../common/model/CAVSoccerSceneModel.js';
+import DynamicProperty from '../../../../axon/js/DynamicProperty.js';
const successSoundClip = new SoundClip( cvSuccessOptions002_mp3, {
initialOutputLevel: 0.2
@@ -77,8 +78,20 @@
// Allocate all the cards at start-up. Each card node must be associated with a card model.
this.cardNodes = model.cards.map( ( cardModel, index ) => {
+ // The drag listener requires a numeric value (does not support null), so map it through a DynamicProperty
+ const dynamicProperty = new DynamicProperty( new Property( cardModel.cellPositionProperty ), {
+ bidirectional: true,
+ map: function( value: number | null ) { return value === null ? 0 : value;},
+ inverseMap: function( value: number ) { return value === 0 ? null : value; }
+ } );
+
const cardNode = new CardNode( this, cardModel, {
- tandem: options.tandem.createTandem( 'cardNodes' ).createTandem1Indexed( 'cardNode', index )
+ tandem: options.tandem.createTandem( 'cardNodes' ).createTandem1Indexed( 'cardNode', index ),
+ enabledRangeProperty: new Property( CAVConstants.PHYSICAL_RANGE ),
+ valueProperty: dynamicProperty,
+ startDrag: () => {
+
+ }
} );
this.cardMap.set( cardNode.cardModel, cardNode );
Index: js/common/view/CAVScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/CAVScreenView.ts b/js/common/view/CAVScreenView.ts
--- a/js/common/view/CAVScreenView.ts (revision a993c261b5a60d7cb40d94f8ccc0a19510f52632)
+++ b/js/common/view/CAVScreenView.ts (date 1689199740574)
@@ -75,6 +75,7 @@
protected readonly backScreenViewLayer;
private readonly middleScreenViewLayer = new Node();
private readonly frontScreenViewLayer;
+ protected readonly screenViewRootNode = new Node();
protected readonly intervalToolLayer = new Node();
@@ -89,6 +90,7 @@
private readonly updateMedianNode: () => void;
private readonly updateDragIndicatorNode: () => void;
protected readonly numberOfKicksProperty: DynamicProperty;
+ protected readonly kickButtonGroup: KickButtonGroup;
public constructor( model: CAVModel, providedOptions: CAVScreenViewOptions ) {
const options = optionize()( {}, providedOptions );
@@ -163,6 +165,10 @@
backLayerToggleNode
]
} );
+ this.backScreenViewLayer.pdomOrder = [
+ backLayerToggleNode,
+ this.intervalToolLayer
+ ];
this.resetAllButton = new ResetAllButton( {
listener: () => {
@@ -202,7 +208,7 @@
}
}, options.questionBarOptions ) );
- const kickButtonGroup = new KickButtonGroup( model, {
+ this.kickButtonGroup = new KickButtonGroup( model, {
// Center under where the soccer player nodes will be. Since the SoccerPlayerNode are positioned in the
// SceneView, we can't use those node bounds to position the kick buttons, so this is a manually tuned magic number.
@@ -265,7 +271,7 @@
this.eraseButton,
this.resetAllButton,
this.questionBar,
- kickButtonGroup,
+ this.kickButtonGroup,
playAreaMedianIndicatorNode
]
} );
@@ -298,9 +304,12 @@
this.middleScreenViewLayer.addChild( dragIndicatorArrowNode );
- this.addChild( this.backScreenViewLayer );
- this.addChild( this.middleScreenViewLayer );
- this.addChild( this.frontScreenViewLayer );
+ // Add to screenViewRootNode for alternativeInput
+ this.screenViewRootNode.addChild( this.backScreenViewLayer );
+ this.screenViewRootNode.addChild( this.middleScreenViewLayer );
+ this.screenViewRootNode.addChild( this.frontScreenViewLayer );
+
+ this.addChild( this.screenViewRootNode );
}
// calculate where the top object is at a given value
@@ -341,7 +350,7 @@
this.accordionBox.boundsProperty.link( this.updateDragIndicatorNode );
}
- protected setBottomControls( controlNode: Node, tandem: Tandem ): void {
+ protected setBottomControls( controlNode: Node, tandem: Tandem ): AlignBox {
// In order to use the AlignBox we need to know the distance from the top of the screen, to the top of the grass.
const BOTTOM_CHECKBOX_PANEL_MARGIN = 12.5;
@@ -362,13 +371,17 @@
]
} );
- this.addChild( new AlignBox( controlsVBox, {
+ const bottomControls = new AlignBox( controlsVBox, {
alignBounds: this.layoutBounds,
xAlign: 'right',
yAlign: 'bottom',
xMargin: BOTTOM_CHECKBOX_PANEL_MARGIN,
yMargin: BOTTOM_CHECKBOX_PANEL_Y_MARGIN
- } ) );
+ } );
+
+ this.screenViewRootNode.addChild( bottomControls );
+
+ return bottomControls;
}
public getSoccerPlayerImageSet( soccerPlayer: SoccerPlayer, sceneModel: SoccerSceneModel ): SoccerPlayerImageSet {
Index: js/median/view/MedianScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/median/view/MedianScreenView.ts b/js/median/view/MedianScreenView.ts
--- a/js/median/view/MedianScreenView.ts (revision a993c261b5a60d7cb40d94f8ccc0a19510f52632)
+++ b/js/median/view/MedianScreenView.ts (date 1689199357799)
@@ -49,7 +49,7 @@
const iconGroup = new AlignGroup();
- this.setBottomControls( new VerticalCheckboxGroup( [
+ const bottomControls = this.setBottomControls( new VerticalCheckboxGroup( [
PlayAreaCheckboxFactory.getPredictMedianCheckboxItem( iconGroup, model ),
PlayAreaCheckboxFactory.getMedianCheckboxItem( iconGroup, model )
], {
@@ -74,6 +74,16 @@
infoDialog.hide();
}
} );
+
+ this.screenViewRootNode.pdomOrder = [
+ this.kickButtonGroup,
+ this.backScreenViewLayer,
+ bottomControls,
+ this.accordionBox,
+ infoDialog,
+ this.eraseButton,
+ this.resetAllButton
+ ];
}
}
```
samreid
terracoda
emily-phet
catherinecarter
jessegreenberg
Currently on the Median screen, the tabs skip over the cards in the Accordion box. When tabbing into the accordion box, the cards will each get their own tab so the user can move each card if they wish.