We need a GrabDragInteraction for groups.

[jessegreenberg]

https://github.com/phetsims/center-and-variability/issues/459, we would like GrabDragInteraction to work well with groups. Currently, it was designed with a single target Node in mind. For example, input listeners are added to the target Node like this

https://github.com/phetsims/scenery-phet/blob/db713ca794457e00cc4e3d5e58563456f1800227/js/accessibility/GrabDragInteraction.js#L545-L546

That caused problems when adding GrabDragInteraction to the group. In https://github.com/phetsims/center-and-variability/issues/459, we tried to add the listener to the parent of the group items but this PressListener made the GrabDragInteraction switch states to "grabbed" when the parent received a press.

We also saw that mouse/touch highlights activated in unexpected ways.

We thought about adding a GrabDragInteraction to every item in the group, but that would have required more focus management logic than we wanted to implement.

Maybe we want to change GrabDragInteraction, or maybe we want a new "interaction" type for groups.

[zepumph]

Some notes about CAV's specific implemnetation for group based grabDrag:

• Highlights are drawn custom by the sim, perhaps we want to try to use scenery a bit more to support interactive highlights + PDOM focus
• Listbox as a motivating design for the keyboard interaction: https://github.com/phetsims/center-and-variability/issues/351#issuecomment-1646118127 (not sure if that would work well for 2d selection though).
• Usages that may do well to use this:
  • CAV soccer ball
  • CAV cards on first screen
  • MSB chocolate bars (top notebook part)(maybe)
  • MSB soccer balls on 4th screen (https://docs.google.com/document/d/1B76VP6M6_-CNkf1PeyVO6Ymut3s3wxJdq8SSPtGGb9Y/edit#heading=h.mhg9bz70huc5)
• Card node interaction animates, so be careful.
• Why we didn't use GrabDragInteraction: https://github.com/phetsims/center-and-variability/issues/459
• Why we didn't use AccessibleValueHandler: https://github.com/phetsims/center-and-variability/issues/367 (mostly because of the quantity of range inputs).
• What should this be called? We like "GroupSortInteraction" most, but here is our brainstorm:
  • SelectGrabDragInteraction
  • GroupGrabDragInteraction
  • ListGroupInteraction
  • GroupKeyboardInteraction
  • GroupInteraction
  • GroupedItemsInteraction
  • ListBox
  • RearrangeableListInteraction
  • SortingGroupInteraction
  • GroupSortInteraction
  • GroupMovableInteraction
  • GroupRearrangeInteraction

[zepumph]

I started on a first pass of factoring out GroupSortInteractionModel, and it got a bit messy. I need to better understand why CAVDragIndicatorModel was needed as a subtype, and how best to support the subtyping with the modularity.

```diff Subject: [PATCH] factor out a GroupSortInteractionModel --- Index: soccer-common/js/view/SoccerSceneView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/soccer-common/js/view/SoccerSceneView.ts b/soccer-common/js/view/SoccerSceneView.ts --- a/soccer-common/js/view/SoccerSceneView.ts (revision f4647e23a6893b31336eb3b67445264922119fea) +++ b/soccer-common/js/view/SoccerSceneView.ts (date 1704405277366) @@ -40,9 +40,7 @@ public constructor( soccerModel: Pick, - 'soccerBallsEnabledProperty' | 'focusedSoccerBallProperty' | 'isKeyboardFocusedProperty' | - 'isSoccerBallKeyboardGrabbedProperty' | 'hasKeyboardGrabbedBallProperty' | 'hasKeyboardMovedBallProperty' | - 'hasKeyboardSelectedDifferentBallProperty'>, + 'soccerBallsEnabledProperty' | 'groupSortInteractionModel'>, public readonly sceneModel: SoccerSceneModel, keyboardDragArrowNode: Node, soccerBallHasBeenDraggedProperty: TProperty, @@ -53,10 +51,10 @@ tandem: Tandem ) { const soccerBallsEnabledProperty = soccerModel.soccerBallsEnabledProperty; - const focusedSoccerBallProperty = soccerModel.focusedSoccerBallProperty; - const hasKeyboardFocusProperty = soccerModel.isKeyboardFocusedProperty; - const isSoccerBallKeyboardGrabbedProperty = soccerModel.isSoccerBallKeyboardGrabbedProperty; - const hasKeyboardGrabbedBallProperty = soccerModel.hasKeyboardGrabbedBallProperty; + const focusedSoccerBallProperty = soccerModel.groupSortInteractionModel.focusedSoccerBallProperty; + const hasKeyboardFocusProperty = soccerModel.groupSortInteractionModel.isKeyboardFocusedProperty; + const isSoccerBallKeyboardGrabbedProperty = soccerModel.groupSortInteractionModel.isSoccerBallKeyboardGrabbedProperty; + const hasKeyboardGrabbedBallProperty = soccerModel.groupSortInteractionModel.hasKeyboardGrabbedBallProperty; const soccerBallMap = new Map(); @@ -257,13 +255,13 @@ if ( focusedSoccerBallProperty.value !== null ) { if ( ( [ 'arrowRight', 'arrowLeft', 'a', 'd', 'arrowUp', 'arrowDown', 'w', 's' ].includes( keysPressed ) ) ) { if ( [ 'arrowRight', 'arrowLeft', 'arrowUp', 'arrowDown' ].includes( keysPressed ) && !isSoccerBallKeyboardGrabbedProperty.value ) { - soccerModel.hasKeyboardSelectedDifferentBallProperty.value = true; + soccerModel.groupSortInteractionModel.hasKeyboardSelectedDifferentBallProperty.value = true; const delta = [ 'arrowRight', 'arrowUp' ].includes( keysPressed ) ? 1 : -1; moveFocusByDelta( delta, topBallNodes ); } else if ( isSoccerBallKeyboardGrabbedProperty.value ) { - soccerModel.hasKeyboardMovedBallProperty.value = true; + soccerModel.groupSortInteractionModel.hasKeyboardMovedBallProperty.value = true; const delta = [ 'arrowLeft', 'a', 'arrowDown', 's' ].includes( keysPressed ) ? -1 : 1; const soccerBall = focusedSoccerBallProperty.value; @@ -303,7 +301,7 @@ soccerBall.valueProperty.value; if ( typeof soccerBall.valueProperty.value === 'number' ) { soccerBall.toneEmitter.emit( soccerBall.valueProperty.value ); - soccerModel.hasKeyboardMovedBallProperty.value = true; + soccerModel.groupSortInteractionModel.hasKeyboardMovedBallProperty.value = true; } } } Index: center-and-variability/js/common/model/CAVModel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/center-and-variability/js/common/model/CAVModel.ts b/center-and-variability/js/common/model/CAVModel.ts --- a/center-and-variability/js/common/model/CAVModel.ts (revision 4c1e488cc54938b74e563d7b5628cda9e70a581f) +++ b/center-and-variability/js/common/model/CAVModel.ts (date 1704411554998) @@ -38,8 +38,6 @@ export default class CAVModel extends SoccerModel { - public override readonly dragIndicatorModel: CAVDragIndicatorModel; - public readonly isPlayAreaMedianVisibleProperty: BooleanProperty; // Screens 1-3 public readonly isPlayAreaMeanVisibleProperty: BooleanProperty; // Screens 2-3 public readonly isPredictMedianVisibleProperty: BooleanProperty; // Screens 1-2 @@ -115,28 +113,22 @@ const allValueProperties = sceneModels.flatMap( sceneModel => sceneModel.soccerBalls.map( soccerBall => soccerBall.valueProperty ) ); - this.dragIndicatorModel = new CAVDragIndicatorModel( - this.isKeyboardFocusedProperty, - this.soccerBallsEnabledProperty, - this.soccerAreaTandem.createTandem( 'soccerBallDragIndicatorModel' ) - ); - // It is important to link to the values of all the soccer balls in the screen, so that the dragIndicator can be // updated after all the balls have landed, and not just after they have been kicked. Multilink.multilinkAny( [ ...allValueProperties, this.selectedSceneModelProperty, - this.dragIndicatorModel.soccerBallHasBeenDraggedProperty, this.selectedSceneStackedSoccerBallCountProperty, - this.selectedSceneMaxKicksProperty, this.isKeyboardFocusedProperty, + this.groupSortInteractionModel.dragIndicatorModel.soccerBallHasBeenDraggedProperty, this.selectedSceneStackedSoccerBallCountProperty, + this.selectedSceneMaxKicksProperty, this.groupSortInteractionModel.isKeyboardFocusedProperty, // The median is queried in the subclass implementation, so needs to trigger an update ...this.sceneModels.map( sceneModel => sceneModel.medianValueProperty ) ], () => { - this.dragIndicatorModel.updateDragIndicator( + this.groupSortInteractionModel.dragIndicatorModel.updateDragIndicator( this.selectedSceneModelProperty.value, this.selectedSceneStackedSoccerBallCountProperty.value, this.selectedSceneMaxKicksProperty.value ); - this.dragIndicatorModel.moveToFocus( this.focusedSoccerBallProperty.value ); + this.groupSortInteractionModel.dragIndicatorModel.moveToFocus( this.groupSortInteractionModel.focusedSoccerBallProperty.value ); } ); } @@ -164,7 +156,6 @@ this.isPredictMedianVisibleProperty.reset(); - this.dragIndicatorModel.reset(); this.isAccordionBoxExpandedProperty.reset(); } } Index: center-and-variability/js/common/view/CAVScreenView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/center-and-variability/js/common/view/CAVScreenView.ts b/center-and-variability/js/common/view/CAVScreenView.ts --- a/center-and-variability/js/common/view/CAVScreenView.ts (revision 4c1e488cc54938b74e563d7b5628cda9e70a581f) +++ b/center-and-variability/js/common/view/CAVScreenView.ts (date 1704411555017) @@ -252,11 +252,11 @@ } } ); - const dragIndicatorArrowNode = SoccerCommonConstants.CREATE_KEYBOARD_ARROW_NODE( model.dragIndicatorModel.isDragIndicatorVisibleProperty, DRAG_CUE_SCALE ); + const dragIndicatorArrowNode = SoccerCommonConstants.CREATE_KEYBOARD_ARROW_NODE( model.groupSortInteractionModel.dragIndicatorModel.isDragIndicatorVisibleProperty, DRAG_CUE_SCALE ); const dragIndicatorHandImage = new Image( dragIndicatorHand_png, { scale: 0.07, - visibleProperty: model.dragIndicatorModel.isDragIndicatorVisibleProperty, + visibleProperty: model.groupSortInteractionModel.dragIndicatorModel.isDragIndicatorVisibleProperty, rotation: Math.PI / 4 } ); @@ -275,8 +275,8 @@ }; this.updateDragIndicatorNode = () => { - const dragIndicatorVisible = model.dragIndicatorModel.isDragIndicatorVisibleProperty.value; - const dragIndicatorValue = model.dragIndicatorModel.valueProperty.value; + const dragIndicatorVisible = model.groupSortInteractionModel.dragIndicatorModel.isDragIndicatorVisibleProperty.value; + const dragIndicatorValue = model.groupSortInteractionModel.dragIndicatorModel.valueProperty.value; if ( dragIndicatorVisible && dragIndicatorValue ) { const topObjectPositionY = this.getTopObjectPositionY( dragIndicatorValue ); @@ -306,17 +306,17 @@ dragIndicatorHandImage.top = dragIndicatorArrowNodeProxy.bottom + Math.abs( this.modelViewTransform.modelToViewDeltaY( CAVObjectType.SOCCER_BALL.radius ) ) - 5; } ); - dragIndicatorArrowNode.addLinkedElement( model.dragIndicatorModel.valueProperty ); + dragIndicatorArrowNode.addLinkedElement( model.groupSortInteractionModel.dragIndicatorModel.valueProperty ); - model.dragIndicatorModel.isDragIndicatorVisibleProperty.link( this.updateDragIndicatorNode ); - model.dragIndicatorModel.valueProperty.link( this.updateDragIndicatorNode ); + model.groupSortInteractionModel.dragIndicatorModel.isDragIndicatorVisibleProperty.link( this.updateDragIndicatorNode ); + model.groupSortInteractionModel.dragIndicatorModel.valueProperty.link( this.updateDragIndicatorNode ); this.visibleBoundsProperty.link( this.updateDragIndicatorNode ); this.model.selectedSceneModelProperty.link( this.updateDragIndicatorNode ); this.model.sceneModels.forEach( sceneModel => { sceneModel.medianValueProperty.link( this.updateDragIndicatorNode ); sceneModel.objectChangedEmitter.addListener( this.updateDragIndicatorNode ); } ); - this.model.focusedSoccerBallProperty.link( this.updateDragIndicatorNode ); + this.model.groupSortInteractionModel.focusedSoccerBallProperty.link( this.updateDragIndicatorNode ); const playAreaMedianIndicatorNode = new PlayAreaMedianIndicatorNode(); @@ -353,12 +353,12 @@ playAreaMedianIndicatorNode.bottom = topObjectPositionY; // If cueing indicators are visible and their stack matches the median stack, adjustments needs to be made. - if ( medianValue === model.dragIndicatorModel.valueProperty.value && model.dragIndicatorModel.isDragIndicatorVisibleProperty.value ) { + if ( medianValue === model.groupSortInteractionModel.dragIndicatorModel.valueProperty.value && model.groupSortInteractionModel.dragIndicatorModel.isDragIndicatorVisibleProperty.value ) { adjustMedianIndicatorBottom( topObjectPositionY ); } - if ( medianValue === model.focusedSoccerBallProperty.value?.valueProperty.value && - ( model.isKeyboardDragArrowVisibleProperty.value ) ) { + if ( medianValue === model.groupSortInteractionModel.focusedSoccerBallProperty.value?.valueProperty.value && + ( model.groupSortInteractionModel.isKeyboardDragArrowVisibleProperty.value ) ) { adjustMedianIndicatorBottom( topObjectPositionY ); } @@ -368,7 +368,8 @@ playAreaMedianIndicatorNode.visible = visible; }; - Multilink.multilink( [ model.dragIndicatorModel.isDragIndicatorVisibleProperty, model.isKeyboardDragArrowVisibleProperty ], + Multilink.multilink( [ model.groupSortInteractionModel.dragIndicatorModel.isDragIndicatorVisibleProperty, + model.groupSortInteractionModel.isKeyboardDragArrowVisibleProperty ], () => { this.updateMedianNode(); } ); Index: soccer-common/js/view/SoccerScreenView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/soccer-common/js/view/SoccerScreenView.ts b/soccer-common/js/view/SoccerScreenView.ts --- a/soccer-common/js/view/SoccerScreenView.ts (revision f4647e23a6893b31336eb3b67445264922119fea) +++ b/soccer-common/js/view/SoccerScreenView.ts (date 1704405256013) @@ -74,7 +74,8 @@ y: SoccerCommonConstants.GROUND_POSITION_Y } ); - this.keyboardDragArrowNode = SoccerCommonConstants.CREATE_KEYBOARD_ARROW_NODE( model.isKeyboardDragArrowVisibleProperty, DRAG_CUE_SCALE ); + // TODO: factor out DRAG_CUE_SCALE? https://github.com/phetsims/scenery-phet/issues/815 + this.keyboardDragArrowNode = SoccerCommonConstants.CREATE_KEYBOARD_ARROW_NODE( model.groupSortInteractionModel.isKeyboardDragArrowVisibleProperty, DRAG_CUE_SCALE ); this.addChild( this.keyboardDragArrowNode ); } @@ -84,7 +85,7 @@ const grabReleaseCueNode = new GrabReleaseCueNode( { centerTop: this.modelViewTransform.modelToViewXY( 7.5, 4 ), - visibleProperty: this.model.isGrabReleaseVisibleProperty + visibleProperty: this.model.groupSortInteractionModel.isGrabReleaseVisibleProperty } ); this.addChild( grabReleaseCueNode ); Index: center-and-variability/js/common/view/CAVSceneView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/center-and-variability/js/common/view/CAVSceneView.ts b/center-and-variability/js/common/view/CAVSceneView.ts --- a/center-and-variability/js/common/view/CAVSceneView.ts (revision 4c1e488cc54938b74e563d7b5628cda9e70a581f) +++ b/center-and-variability/js/common/view/CAVSceneView.ts (date 1704411555039) @@ -33,7 +33,10 @@ physicalRange: Range, tandem: Tandem ) { - super( model, sceneModel, keyboardDragArrowNode, model.dragIndicatorModel.soccerBallHasBeenDraggedProperty, model.dragIndicatorModel.valueProperty, getKickerImageSet, modelViewTransform, physicalRange, tandem ); + super( model, sceneModel, keyboardDragArrowNode, + model.groupSortInteractionModel.dragIndicatorModel.soccerBallHasBeenDraggedProperty, + model.groupSortInteractionModel.dragIndicatorModel.valueProperty, getKickerImageSet, modelViewTransform, + physicalRange, tandem ); const medianHighlightLayer = new MedianHighlightLayer( sceneModel.soccerBalls, modelViewTransform, model.isPlayAreaMedianVisibleProperty, { visibleProperty: model.isPlayAreaMedianVisibleProperty Index: soccer-common/js/model/SoccerModel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/soccer-common/js/model/SoccerModel.ts b/soccer-common/js/model/SoccerModel.ts --- a/soccer-common/js/model/SoccerModel.ts (revision f4647e23a6893b31336eb3b67445264922119fea) +++ b/soccer-common/js/model/SoccerModel.ts (date 1704411555030) @@ -18,15 +18,10 @@ import IOType from '../../../tandem/js/types/IOType.js'; import EnabledProperty from '../../../axon/js/EnabledProperty.js'; import DynamicProperty from '../../../axon/js/DynamicProperty.js'; -import SoccerBall from './SoccerBall.js'; -import TReadOnlyProperty from '../../../axon/js/TReadOnlyProperty.js'; -import BooleanProperty from '../../../axon/js/BooleanProperty.js'; -import DerivedProperty from '../../../axon/js/DerivedProperty.js'; -import DragIndicatorModel from './DragIndicatorModel.js'; - +import GroupSortInteractionModel, { GroupSortInteractionModelOptions } from './GroupSortInteractionModel.js'; type SelfOptions = { - dragIndicatorModelTandem?: Tandem | null; + groupSortInteractionModelOptions?: GroupSortInteractionModelOptions; }; type SoccerModelOptions = SelfOptions & PhetioObjectOptions; export default class SoccerModel extends PhetioObject { @@ -46,37 +41,12 @@ // The maximum number of kicks in the currently active scene protected readonly selectedSceneMaxKicksProperty: DynamicProperty; - // The model for the visual indicator for dragging soccer balls via mouse, touch or keyboard - public readonly dragIndicatorModel: DragIndicatorModel; - - // The soccerBall that is receiving highlight focus in the backLayerSoccerBallLayer group highlight - public readonly focusedSoccerBallProperty = new Property( null ); - - // Whether a soccer ball is being grabbed via keyboard interaction - public readonly isSoccerBallKeyboardGrabbedProperty = new Property( false ); - - // Whether the 'Press SPACE to Grab or Release' dialog is showing - public readonly isGrabReleaseVisibleProperty: TReadOnlyProperty; - - // Whether the keyboard drag arrow is showing - public readonly isKeyboardDragArrowVisibleProperty: TReadOnlyProperty; - - // Whether the keyboard is currently focused on a sim component - public readonly isKeyboardFocusedProperty = new BooleanProperty( false ); - - // Properties that switch to true when the specified action has occurred once. - public readonly hasKeyboardGrabbedBallProperty = new BooleanProperty( false ); - - // Whether a soccer ball has been moved with the keyboard controls - public readonly hasKeyboardMovedBallProperty = new BooleanProperty( false ); - - // Whether the user has changed the selected soccer ball with the keyboard controls - public readonly hasKeyboardSelectedDifferentBallProperty = new BooleanProperty( false ); + public readonly groupSortInteractionModel: GroupSortInteractionModel; protected constructor( public readonly sceneModels: T[], providedOptions: SoccerModelOptions ) { const options = optionize()( { isDisposable: false, - dragIndicatorModelTandem: null + groupSortInteractionModelOptions: {} }, providedOptions ); super( options ); @@ -91,13 +61,6 @@ phetioDocumentation: 'Indicates which kicker is active by jersey number.' } ); - this.selectedSceneModelProperty.link( selectedScene => { - this.sceneModels.forEach( sceneModel => { - sceneModel.isVisibleProperty.value = sceneModel === selectedScene; - } ); - - this.focusedSoccerBallProperty.value = null; - } ); this.soccerBallsEnabledProperty = new EnabledProperty( true, { tandem: this.soccerAreaTandem.createTandem( 'soccerBallsEnabledProperty' ), @@ -106,8 +69,16 @@ checkTandemName: false } ); - const dragIndicatorModelTandem = options.dragIndicatorModelTandem !== null ? options.dragIndicatorModelTandem : Tandem.OPT_OUT; - this.dragIndicatorModel = new DragIndicatorModel( this.isKeyboardFocusedProperty, this.soccerBallsEnabledProperty, dragIndicatorModelTandem ); + this.groupSortInteractionModel = new GroupSortInteractionModel( this.soccerBallsEnabledProperty, options.groupSortInteractionModelOptions ); + + this.selectedSceneModelProperty.link( selectedScene => { + this.sceneModels.forEach( sceneModel => { + sceneModel.isVisibleProperty.value = sceneModel === selectedScene; + } ); + + this.groupSortInteractionModel.focusResetEmitter.emit(); + } ); + // These DynamicProperties allow us to track all the necessary scenes Properties for dragIndicator update, and not // just the first selectedScene @@ -117,18 +88,6 @@ this.selectedSceneMaxKicksProperty = new DynamicProperty( this.selectedSceneModelProperty, { derive: 'maxKicksProperty' } ); - - this.isGrabReleaseVisibleProperty = new DerivedProperty( [ this.focusedSoccerBallProperty, this.hasKeyboardGrabbedBallProperty, this.isKeyboardFocusedProperty ], - ( focusedSoccerBall, hasGrabbedBall, hasKeyboardFocus ) => { - return focusedSoccerBall !== null && !hasGrabbedBall && hasKeyboardFocus; - } ); - - const createDerivedProperty = ( conditionProperty: TReadOnlyProperty, grabCondition: ( isSoccerBallKeyboardGrabbed: boolean ) => boolean ) => new DerivedProperty( - [ this.focusedSoccerBallProperty, this.isSoccerBallKeyboardGrabbedProperty, this.isKeyboardFocusedProperty, conditionProperty ], - ( focusedBall, isSoccerBallKeyboardGrabbed, isKeyboardFocused, condition ) => - focusedBall !== null && grabCondition( isSoccerBallKeyboardGrabbed ) && isKeyboardFocused && !condition ); - - this.isKeyboardDragArrowVisibleProperty = createDerivedProperty( this.hasKeyboardMovedBallProperty, isSoccerBallKeyboardGrabbed => isSoccerBallKeyboardGrabbed ); } public step( dt: number ): void { @@ -141,19 +100,14 @@ // We need to reset keyboard focus Properties that are used by the sceneModels, before // resetting the sceneModels themselves. - this.focusedSoccerBallProperty.reset(); - this.isSoccerBallKeyboardGrabbedProperty.reset(); - this.hasKeyboardGrabbedBallProperty.reset(); - this.isKeyboardFocusedProperty.reset(); - this.hasKeyboardSelectedDifferentBallProperty.reset(); - this.hasKeyboardMovedBallProperty.reset(); + this.groupSortInteractionModel.reset(); this.sceneModels.forEach( sceneModel => sceneModel.reset() ); this.selectedSceneModelProperty.reset(); } public clearData(): void { - this.focusedSoccerBallProperty.value = null; + this.groupSortInteractionModel.clearData(); this.selectedSceneModelProperty.value.clearData(); } } Index: soccer-common/js/model/GroupSortInteractionModel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/soccer-common/js/model/GroupSortInteractionModel.ts b/soccer-common/js/model/GroupSortInteractionModel.ts new file mode 100644 --- /dev/null (date 1704411787554) +++ b/soccer-common/js/model/GroupSortInteractionModel.ts (date 1704411787554) @@ -0,0 +1,105 @@ +// Copyright 2023, University of Colorado Boulder + +/** + * GroupSortInteractionModel + * @author Michael Kauzmann (PhET Interactive Simulations) + * + */ + +import soccerCommon from '../soccerCommon.js'; +import optionize from '../../../phet-core/js/optionize.js'; +import Property from '../../../axon/js/Property.js'; +import Tandem from '../../../tandem/js/Tandem.js'; +import SoccerBall from './SoccerBall.js'; +import TReadOnlyProperty from '../../../axon/js/TReadOnlyProperty.js'; +import BooleanProperty from '../../../axon/js/BooleanProperty.js'; +import DerivedProperty from '../../../axon/js/DerivedProperty.js'; +import DragIndicatorModel from './DragIndicatorModel.js'; +import Emitter from '../../../axon/js/Emitter.js'; +import CAVDragIndicatorModel from '../../../center-and-variability/js/common/model/CAVDragIndicatorModel.js'; + + +type SelfOptions = { + dragIndicatorModelTandem?: Tandem | null; + dragIndicatorModel?: DragIndicatorModel; +}; + +export type GroupSortInteractionModelOptions = SelfOptions; + +export default class GroupSortInteractionModel { + + // The model for the visual indicator for dragging soccer balls via mouse, touch or keyboard + public readonly dragIndicatorModel: DragIndicatorModel; + + // The soccerBall that is receiving highlight focus in the backLayerSoccerBallLayer group highlight + public readonly focusedSoccerBallProperty = new Property( null ); + + // Whether a soccer ball is being grabbed via keyboard interaction + public readonly isSoccerBallKeyboardGrabbedProperty = new Property( false ); + + // Whether the 'Press SPACE to Grab or Release' dialog is showing + public readonly isGrabReleaseVisibleProperty: TReadOnlyProperty; + + // Whether the keyboard drag arrow is showing + public readonly isKeyboardDragArrowVisibleProperty: TReadOnlyProperty; + + // Whether the keyboard is currently focused on a sim component + public readonly isKeyboardFocusedProperty = new BooleanProperty( false ); + + // Properties that switch to true when the specified action has occurred once. + public readonly hasKeyboardGrabbedBallProperty = new BooleanProperty( false ); + + // Whether a soccer ball has been moved with the keyboard controls + public readonly hasKeyboardMovedBallProperty = new BooleanProperty( false ); + + // Whether the user has changed the selected soccer ball with the keyboard controls + public readonly hasKeyboardSelectedDifferentBallProperty = new BooleanProperty( false ); + + // TODO: + public readonly focusResetEmitter = new Emitter(); + + public constructor( dragEnabledProperty: TReadOnlyProperty, providedOptions: GroupSortInteractionModelOptions ) { + const dragIndicatorModelTandem = providedOptions?.dragIndicatorModelTandem || Tandem.OPT_OUT; + + const options = optionize()( { + dragIndicatorModelTandem: null, + dragIndicatorModel: new CAVDragIndicatorModel( this.isKeyboardFocusedProperty, dragEnabledProperty, dragIndicatorModelTandem ) + }, providedOptions ); + + this.focusResetEmitter.addListener( () => { + this.focusedSoccerBallProperty.value = null; + } ); + + this.dragIndicatorModel = options.dragIndicatorModel; + + this.isGrabReleaseVisibleProperty = new DerivedProperty( [ this.focusedSoccerBallProperty, this.hasKeyboardGrabbedBallProperty, this.isKeyboardFocusedProperty ], + ( focusedSoccerBall, hasGrabbedBall, hasKeyboardFocus ) => { + return focusedSoccerBall !== null && !hasGrabbedBall && hasKeyboardFocus; + } ); + + const createDerivedProperty = ( conditionProperty: TReadOnlyProperty, grabCondition: ( isSoccerBallKeyboardGrabbed: boolean ) => boolean ) => new DerivedProperty( + [ this.focusedSoccerBallProperty, this.isSoccerBallKeyboardGrabbedProperty, this.isKeyboardFocusedProperty, conditionProperty ], + ( focusedBall, isSoccerBallKeyboardGrabbed, isKeyboardFocused, condition ) => + focusedBall !== null && grabCondition( isSoccerBallKeyboardGrabbed ) && isKeyboardFocused && !condition ); + + this.isKeyboardDragArrowVisibleProperty = createDerivedProperty( this.hasKeyboardMovedBallProperty, isSoccerBallKeyboardGrabbed => isSoccerBallKeyboardGrabbed ); + } + + public reset(): void { + + // We need to reset keyboard focus Properties that are used by the sceneModels, before + // resetting the sceneModels themselves. + this.focusedSoccerBallProperty.reset(); + this.isSoccerBallKeyboardGrabbedProperty.reset(); + this.hasKeyboardGrabbedBallProperty.reset(); + this.isKeyboardFocusedProperty.reset(); + this.hasKeyboardSelectedDifferentBallProperty.reset(); + this.hasKeyboardMovedBallProperty.reset(); + } + + public clearData(): void { + this.focusedSoccerBallProperty.value = null; + } +} + +soccerCommon.register( 'GroupSortInteractionModel', GroupSortInteractionModel );

[zepumph]

Lots of good progress today. We now have a model and view type. Commits are on branches, and there are 26 TODOs. I don't think it will be too too much to clean this up from here and push to main, but I expect that another conversation with @marlitas will be needed about the view type (we only went through the model type).

[zepumph]

Lots of good progress today. Still there are 30 TODOs, but the main work is to pull CAV and soccer-common logic out of GroupSortInteractionView. I have a meeting set up with @jessegreenberg tomorrow to help me design the best system I can. I'll also try to come back to this this evening.

[zepumph]

Alright. Here is a report on progress from the above commits:

• These classes have been untangled from soccer common and CAV. There are subclasses and options implemented to help support this.

• These now live in scenery-phet. This was done without deleting the git history.

• PhET-iO api and migration rules updated

• [ ] 20 TODOs still exist in the issue. Most of this cleanup is (in my opinion) is pretty important to finishing this project. Some of them will need a bit of input with @marlitas and a designer.

• [ ] I noticed a bit of a bug where the median indicator doesn't update when the drag indicator is hidden below it.

• [ ] I believe it would be very very good of us to use this outside of soccer balls before calling this sprint done. Perhaps we can work on the chocolate bars in MSaB, and maybe the interactive cards would work well.

[zepumph]

We are down to 14 TODOs, which are really just questions for MS and a designer. Many are simple and straight forward, so I'd say not too much more work here to unblock MSaB. Yay!

[zepumph]

• [ ] CT State and migration wrappers are failing for CAV which is expected until we figure out how to correctly instrument this. I believe it will be best when we can think more fully about the phet-io design needs, so I won't fix this right away.

[zepumph]

Lots of good progress here today! I believe there are only four things left for this issue:

1. I have two more TODOs that have to do with the interaction between mouse and keyboard when used at the same time (especially when interactive highlights are used). This is a bit messy and I'd like to improve this.
2. PhET-iO design and implementation (meeting scheduled for Thursday)
3. Interaction design confirmation with @terracoda (scheduled for Friday).
4. Code review (not scheduled). Likely the best people to do this will be @marlitas and @jbphet as part of using this in MSaB.

[zepumph]

@jessegreenberg and I will meet up tomorrow to see if the two mouse/keyboard workarounds can be added to common code.

[zepumph]

isKeyboardFocusedProperty-> usingKeyboardProperty

[zepumph]

Good conversation with @terracoda today. I'm going to confirm the sound behavior with JB and AM and then we can move forward from here.

[zepumph]

Everything is done here except for a single TODO, that is blocked by https://github.com/phetsims/scenery/issues/1602. I'm meeting @jessegreenberg about it tomorrow. Marking on hold and removing high priority label.

[terracoda]

@zepumph, I couldn't find anything in the ARIA Practices Guide, so I sent an email to the W3C-WAI Interest Group, basically asking:

Should the ball keep moving when a non-required modifier key is pressed at the same time the Arrow key is pressed? I’m thinking no, and that our implementation is fine, but I wanted to hear what others thought about this.

We'll see if we get a response.

FYI, I did notice that in a typical rearrangeable listbox implementation that ALT+Arrow is the key combination to create a shortcut to move the selected item up and down within the list without requiring the use of the Action buttons. But since our group sort does not use Action buttons that would require a change in focus, this shortcut is irrelevant.

Example here: https://www.w3.org/WAI/ARIA/apg/patterns/listbox/examples/listbox-rearrangeable/

[terracoda]

By the way the group sort with number hotkeys and the WASD alternatives seems really robust in my opinion.

[zepumph]

Excellent! Let's pick up this conversation more generally in https://github.com/phetsims/scenery/issues/1597.

[zepumph]

Ok, the last TODO was handled now that https://github.com/phetsims/scenery/issues/1597 was handled. This is behaving really well, and it ended up being quite simple. The only weird thing was that you get into cases where you really notice the visible threshold for showing highlights again:

https://github.com/phetsims/joist/blob/7c3f2201f401142ccd0841a6ebb30a1812897850/js/HighlightVisibilityController.ts#L17-L19

But that is not a problem that we need to solve here in GroupSortInteraction. All of other TODOs have been handled. We are ready to close this, and the changes will be more thoroughly reviewed over in #841. Closing