`InteractiveCardContainerModel` should use `GroupSortInteractionModel`

[zepumph]

Likely this will go beyond the scope of https://github.com/phetsims/scenery-phet/issues/815, but it would be good to do at some point for this sim's code quality.

[zepumph]

I'd like to at least try this out to see how it goes, before closing https://github.com/phetsims/scenery-phet/issues/815

[zepumph]

Wow. That wasn't nearly as bad as I thought! It is still quite buggy, but surprisingly enough, it is kinda working!

• many todos
• right now select and sort modes are reversed
• PhET-iO instrumentation is kinda wild, and we should think about this stuff as part of the design over in https://github.com/phetsims/scenery-phet/issues/815

```diff Subject: [PATCH] first pass using GroupSortInteraction --- Index: center-and-variability/js/median/model/InteractiveCardContainerModel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/center-and-variability/js/median/model/InteractiveCardContainerModel.ts b/center-and-variability/js/median/model/InteractiveCardContainerModel.ts --- a/center-and-variability/js/median/model/InteractiveCardContainerModel.ts (revision f5c5c53f59fd83cf9934d42fae62872377cb92f4) +++ b/center-and-variability/js/median/model/InteractiveCardContainerModel.ts (date 1705104100419) @@ -27,14 +27,12 @@ import BooleanProperty from '../../../../axon/js/BooleanProperty.js'; import NumberProperty from '../../../../axon/js/NumberProperty.js'; import DerivedProperty from '../../../../axon/js/DerivedProperty.js'; -import NullableIO from '../../../../tandem/js/types/NullableIO.js'; -import ReferenceIO from '../../../../tandem/js/types/ReferenceIO.js'; -import Tandem from '../../../../tandem/js/Tandem.js'; import dotRandom from '../../../../dot/js/dotRandom.js'; import CAVQueryParameters from '../../common/CAVQueryParameters.js'; import Emitter from '../../../../axon/js/Emitter.js'; import isResettingProperty from '../../../../soccer-common/js/model/isResettingProperty.js'; import isSettingPhetioStateProperty from '../../../../tandem/js/isSettingPhetioStateProperty.js'; +import GroupSortInteractionModel from '../../../../scenery-phet/js/accessibility/group-sort/model/GroupSortInteractionModel.js'; const cardMovementSounds = [ cardMovement1_mp3, @@ -77,24 +75,12 @@ // Indicates whether the user has ever dragged a card, used to determine dragIndicationCardProperty. public readonly hasDraggedCardProperty: TReadOnlyProperty; - // The card where the drag indicator should appear. null if no drag indicator should appear. - public readonly dragIndicationCardProperty: Property; - public readonly totalDragDistanceProperty: Property; - // KEYBOARD INPUT PROPERTIES: - // The card which currently has focus. Is part of what controls highlight visibility among other things. - public readonly focusedCardProperty: Property = new Property( null ); - // Tracks when a card is currently grabbed via keyboard input. public readonly isCardGrabbedProperty = new BooleanProperty( false ); - // Visible Properties for keyboard hints - public readonly grabReleaseCueVisibleProperty: TReadOnlyProperty; - public readonly isKeyboardDragArrowVisibleProperty: TReadOnlyProperty; - // Properties that track if a certain action has ever been performed via keyboard input. - public readonly hasKeyboardMovedCardProperty: Property; public readonly hasKeyboardGrabbedCardProperty = new BooleanProperty( false ); public readonly hasKeyboardSelectedDifferentCardProperty = new BooleanProperty( false ); @@ -102,6 +88,8 @@ public readonly isKeyboardFocusedProperty = new BooleanProperty( false ); public readonly manuallySortedEmitter: Emitter; + public readonly groupSortInteractionModel: GroupSortInteractionModel; + public constructor( medianModel: MedianModel, providedOptions: InteractiveCardContainerModelOptions ) { super( medianModel, providedOptions ); @@ -111,68 +99,51 @@ phetioDocumentation: 'Accumulated card drag distance, for purposes of hiding the drag indicator node' } ); - this.hasKeyboardMovedCardProperty = new BooleanProperty( false, { - tandem: providedOptions.tandem.createTandem( 'hasKeyboardMovedCardProperty' ), - phetioReadOnly: true, // controlled by the sim - phetioDocumentation: 'Whether a card been moved using the keyboard, for purposes of hiding the drag indicator node' + this.groupSortInteractionModel = new GroupSortInteractionModel( { + getValueProperty: cardModel => cardModel.indexProperty, + tandem: providedOptions.tandem.createTandem( 'groupSortInteractionModel' ) } ); this.hasDraggedCardProperty = new DerivedProperty( - [ this.totalDragDistanceProperty, this.hasKeyboardMovedCardProperty ], - ( totalDragDistance, hasKeyboardMovedCard ) => totalDragDistance > 15 || hasKeyboardMovedCard + [ this.totalDragDistanceProperty, this.groupSortInteractionModel.hasKeyboardSortedGroupItemProperty ], + ( totalDragDistance, keyboardSortedCard ) => totalDragDistance > 15 || keyboardSortedCard ); - - this.dragIndicationCardProperty = new Property( null, { - phetioReadOnly: true, - phetioValueType: NullableIO( ReferenceIO( CardModel.CardModelIO ) ), - tandem: this.representationContext === 'accordion' ? providedOptions.tandem.createTandem( 'cardDragIndicatorProperty' ) : Tandem.OPT_OUT, - phetioDocumentation: 'This is for PhET-iO internal use only.' - } ); - - this.isKeyboardDragArrowVisibleProperty = new DerivedProperty( [ this.focusedCardProperty, this.hasKeyboardMovedCardProperty, this.hasKeyboardGrabbedCardProperty, - this.isCardGrabbedProperty, this.isKeyboardFocusedProperty ], - ( focusedCard, hasKeyboardMovedCard, hasGrabbedCard, isCardGrabbed, hasKeyboardFocus ) => { - return focusedCard !== null && !hasKeyboardMovedCard && hasGrabbedCard && isCardGrabbed && hasKeyboardFocus; - } ); - - this.grabReleaseCueVisibleProperty = new DerivedProperty( [ this.focusedCardProperty, this.hasKeyboardGrabbedCardProperty, this.isKeyboardFocusedProperty ], - ( focusedCard, hasGrabbedCard, hasKeyboardFocus ) => { - return focusedCard !== null && !hasGrabbedCard && hasKeyboardFocus; - } ); this.cardCellsChangedEmitter.addListener( () => { medianModel.areCardsSortedProperty.value = this.isDataSorted(); } ); - const updateDragIndicationCardProperty = () => { + + const updateMouseSortCueNode = () => { const leftCard = this.getCardsInCellOrder()[ 0 ]; const rightCard = this.getCardsInCellOrder()[ 1 ]; // If the user has not yet dragged a card and there are multiple cards showing, add the drag indicator to leftCard. if ( !this.hasDraggedCardProperty.value && leftCard && rightCard ) { - this.dragIndicationCardProperty.value = leftCard; + // TODO:???? https://github.com/phetsims/center-and-variability/issues/605 + this.groupSortInteractionModel.selectedGroupItemProperty.value = leftCard; } // If the user has dragged a card, then the drag indicator does not need to be shown. if ( this.hasDraggedCardProperty.value || !leftCard || !rightCard ) { - this.dragIndicationCardProperty.value = null; + this.groupSortInteractionModel.mouseSortCueVisibleProperty.value = false; } }; - this.cardCellsChangedEmitter.addListener( updateDragIndicationCardProperty ); - this.hasDraggedCardProperty.link( updateDragIndicationCardProperty ); - this.cards.forEach( card => card.soccerBall.valueProperty.lazyLink( updateDragIndicationCardProperty ) ); + this.cardCellsChangedEmitter.addListener( updateMouseSortCueNode ); + this.hasDraggedCardProperty.link( updateMouseSortCueNode ); + this.cards.forEach( card => card.soccerBall.valueProperty.lazyLink( updateMouseSortCueNode ) ); medianModel.selectedSceneModelProperty.value.resetEmitter.addListener( () => { this.totalDragDistanceProperty.reset(); - this.hasKeyboardMovedCardProperty.reset(); + this.groupSortInteractionModel.reset(); // TODO: not everything!? https://github.com/phetsims/center-and-variability/issues/605 this.hasKeyboardGrabbedCardProperty.reset(); this.hasKeyboardSelectedDifferentCardProperty.reset(); } ); medianModel.selectedSceneModelProperty.value.preClearDataEmitter.addListener( () => { - this.focusedCardProperty.reset(); + this.groupSortInteractionModel.resetInteractionState(); this.isCardGrabbedProperty.reset(); } ); Index: scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.ts b/scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.ts --- a/scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.ts (revision 537750eb59382b8791694741a2df359ec101377c) +++ b/scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.ts (date 1705104134135) @@ -66,7 +66,7 @@ export default class GroupSortInteractionView extends Disposable { // Update group highlight dynamically by setting the `shape` of this path. - protected readonly groupFocusHighlightPath: HighlightPath; + public readonly groupFocusHighlightPath: HighlightPath; // Emitted when the sorting cue should be repositioned. Most likely because the selection has changed. public readonly positionSortCueNodeEmitter = new Emitter(); @@ -126,6 +126,7 @@ isKeyboardFocusedProperty.value = false; }, over: () => { + // TODO: this isn't part of InteractiveCardContainer? // TODO: MS!!!! this is awkward. In this situation: // 1. tab to populated node, the keyboard grab cue is shown. // 2. Move the mouse over a group item, the keyboard grab cue goes away. @@ -263,7 +264,7 @@ } ); } - const defaultGroupShape = primaryFocusedNode.bounds.isFinite() ? Shape.bounds( primaryFocusedNode.visibleBounds ) : null; + const defaultGroupShape = primaryFocusedNode.visibleBounds.isFinite() ? Shape.bounds( primaryFocusedNode.visibleBounds ) : null; // Set the outer group focus highlight to surround the entire area where group items are located. this.groupFocusHighlightPath = new HighlightPath( defaultGroupShape, { Index: center-and-variability/js/median/view/InteractiveCardNodeContainer.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/center-and-variability/js/median/view/InteractiveCardNodeContainer.ts b/center-and-variability/js/median/view/InteractiveCardNodeContainer.ts --- a/center-and-variability/js/median/view/InteractiveCardNodeContainer.ts (revision f5c5c53f59fd83cf9934d42fae62872377cb92f4) +++ b/center-and-variability/js/median/view/InteractiveCardNodeContainer.ts (date 1705104096389) @@ -24,8 +24,9 @@ import InteractiveCardContainerModel from '../model/InteractiveCardContainerModel.js'; import Property from '../../../../axon/js/Property.js'; import CAVSoccerSceneModel from '../../common/model/CAVSoccerSceneModel.js'; -import { animatedPanZoomSingleton, HighlightFromNode, HighlightPath, KeyboardListener, NodeTranslationOptions, Path } from '../../../../scenery/js/imports.js'; +import { animatedPanZoomSingleton, Node, NodeTranslationOptions, Path } from '../../../../scenery/js/imports.js'; import Vector2 from '../../../../dot/js/Vector2.js'; +import Range from '../../../../dot/js/Range.js'; import CAVConstants from '../../common/CAVConstants.js'; import Bounds2 from '../../../../dot/js/Bounds2.js'; import CardNode, { cardDropClip, cardPickUpSoundClip, PICK_UP_DELTA_X } from './CardNode.js'; @@ -34,7 +35,6 @@ import { Shape } from '../../../../kite/js/imports.js'; import Multilink from '../../../../axon/js/Multilink.js'; import isSettingPhetioStateProperty from '../../../../tandem/js/isSettingPhetioStateProperty.js'; -import GrabReleaseCueNode from '../../../../scenery-phet/js/accessibility/nodes/GrabReleaseCueNode.js'; import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; import CelebrationNode from './CelebrationNode.js'; import checkboxCheckedSoundPlayer from '../../../../tambo/js/shared-sound-players/checkboxCheckedSoundPlayer.js'; @@ -43,6 +43,7 @@ import CardDragIndicatorNode from './CardDragIndicatorNode.js'; import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js'; import GroupSortInteractionView from '../../../../scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.js'; +import CardModel from '../model/CardModel.js'; const FOCUS_HIGHLIGHT_Y_MARGIN = CAVConstants.CARD_SPACING + 3; @@ -61,6 +62,7 @@ // The message that appears when the cards are sorted private readonly celebrationNode: CelebrationNode; + private readonly groupSortInteractionView: GroupSortInteractionView; public constructor( model: InteractiveCardContainerModel, private readonly isSortingDataProperty: Property, @@ -78,6 +80,28 @@ this.celebrationNode = new CelebrationNode( model, this.cardMap, this.sceneModel.resetEmitter ); this.addChild( this.celebrationNode ); + this.groupSortInteractionView = new GroupSortInteractionView( model.groupSortInteractionModel, this, { + getNextSelectedGroupItem: delta => { + // TODO: we have to do this every time? https://github.com/phetsims/center-and-variability/issues/605 + const selectedCardModel = model.groupSortInteractionModel.selectedGroupItemProperty.value!; + assert && assert( selectedCardModel, 'sanity' ); + swapCards( this.getActiveCardNodesInOrder(), this.cardMap.get( selectedCardModel )!, delta ); + return selectedCardModel; + }, + getGroupItemToSelect: () => { + const activeCards = this.getActiveCardNodesInOrder(); + return activeCards[ 0 ] ? activeCards[ 0 ].model : null; + }, + // TODO: something like this? https://github.com/phetsims/center-and-variability/issues/605 + // onRelease: ()=>{ + // this.cardNodes.forEach( cardNode => { + // cardNode.model.isDraggingProperty.value = false; + // } ); + // }, + getNodeFromModelItem: cardModel => this.cardMap.get( cardModel ) || null, + sortingRange: new Range( 0, 4 ) // TODO: https://github.com/phetsims/center-and-variability/issues/605 + } ); + this.cardMap.forEach( ( cardNode, cardModel ) => { // Update the position of all cards (via animation) whenever any card is dragged cardNode.model.positionProperty.link( this.createDragPositionListener( cardNode ) ); @@ -107,32 +131,24 @@ } ); } ); - const focusHighlightPath = new HighlightPath( null, { - outerStroke: HighlightPath.OUTER_LIGHT_GROUP_FOCUS_COLOR, - innerStroke: HighlightPath.INNER_LIGHT_GROUP_FOCUS_COLOR, - outerLineWidth: HighlightPath.GROUP_OUTER_LINE_WIDTH, - innerLineWidth: HighlightPath.GROUP_INNER_LINE_WIDTH - } ); - - const grabReleaseCueNode = new GrabReleaseCueNode( { - top: CAVConstants.CARD_DIMENSION + FOCUS_HIGHLIGHT_Y_MARGIN + 15, - visibleProperty: model.grabReleaseCueVisibleProperty + const grabReleaseCueNode = GroupSortInteractionView.createGrabReleaseCueNode( model.groupSortInteractionModel.grabReleaseCueVisibleProperty, { + top: CAVConstants.CARD_DIMENSION + FOCUS_HIGHLIGHT_Y_MARGIN + 15 } ); - const keyboardSortCueNode = GroupSortInteractionView.createSortCueNode( model.isKeyboardDragArrowVisibleProperty ); + const keyboardSortCueNode = GroupSortInteractionView.createSortCueNode( model.groupSortInteractionModel.keyboardSortCueVisibleProperty ); this.addChild( keyboardSortCueNode ); const cardDragIndicatorNode = new CardDragIndicatorNode( { centerTop: new Vector2( 0.5 * CAVConstants.CARD_DIMENSION - PICK_UP_DELTA_X, CAVConstants.CARD_DIMENSION - 10 ), visibleProperty: new DerivedProperty( - [ this.inputEnabledProperty, model.isKeyboardFocusedProperty, model.dragIndicationCardProperty ], + [ this.inputEnabledProperty, model.isKeyboardFocusedProperty, model.groupSortInteractionModel.selectedGroupItemProperty ], ( inputEnabled, hasKeyboardFocus, dragIndicationCard ) => inputEnabled && !hasKeyboardFocus && !!dragIndicationCard ) } ); this.addChild( cardDragIndicatorNode ); - model.dragIndicationCardProperty.lazyLink( ( newCard, oldCard ) => { + model.groupSortInteractionModel.selectedGroupItemProperty.lazyLink( ( newCard, oldCard ) => { if ( oldCard ) { const oldCardNode = this.cardMap.get( oldCard )!; @@ -176,7 +192,7 @@ // Needs to be pickable in accordion box. this.pickable = true; - const focusedCardNodeProperty: TReadOnlyProperty = new DerivedProperty( [ model.focusedCardProperty ], focusedCard => { + const focusedCardNodeProperty: TReadOnlyProperty = new DerivedProperty( [ model.groupSortInteractionModel.selectedGroupItemProperty ], focusedCard => { return focusedCard === null ? focusedCard : this.cardMap.get( focusedCard )!; } ); @@ -187,50 +203,51 @@ // When a user is focused on the card container but there are no cards yet, we want to ensure that a card gets focused // once there is a card. - if ( model.focusedCardProperty.value === null && this.focused && model.getActiveCards().length === 1 ) { - model.focusedCardProperty.value = activeCardNodes[ 0 ].model; + if ( model.groupSortInteractionModel.selectedGroupItemProperty.value === null && this.focused && model.getActiveCards().length === 1 ) { + model.groupSortInteractionModel.selectedGroupItemProperty.value = activeCardNodes[ 0 ].model; } - // If the card cells changed, and we have no more active cards left, that means that all the cards were removed. + // If the card cells changed, and we have no more active cards left, that means that all the cards were removed. // Therefore, we want to set the focused card to null. else if ( model.getActiveCards().length === 0 ) { - model.focusedCardProperty.value = null; - } - } ); - - this.addInputListener( { - focus: () => { - const activeCardNodes = this.getActiveCardNodesInOrder(); - if ( model.focusedCardProperty.value === null && activeCardNodes.length > 0 ) { - model.focusedCardProperty.value = activeCardNodes[ 0 ].model; - } - - // When the group receives keyboard focus, make sure that the focused card is displayed - if ( focusedCardNodeProperty.value ) { - animatedPanZoomSingleton.listener.panToNode( focusedCardNodeProperty.value, true ); - } - model.isKeyboardFocusedProperty.value = true; - }, - blur: () => { - model.isCardGrabbedProperty.value = false; - model.isKeyboardFocusedProperty.value = false; + model.groupSortInteractionModel.selectedGroupItemProperty.value = null; } } ); + // + // this.addInputListener( { + // focus: () => { + // const activeCardNodes = this.getActiveCardNodesInOrder(); + // if ( model.focusedCardProperty.value === null && activeCardNodes.length > 0 ) { + // model.focusedCardProperty.value = activeCardNodes[ 0 ].model; + // } + // + // // When the group receives keyboard focus, make sure that the focused card is displayed + // if ( focusedCardNodeProperty.value ) { + // animatedPanZoomSingleton.listener.panToNode( focusedCardNodeProperty.value, true ); + // } + // model.isKeyboardFocusedProperty.value = true; + // }, + // blur: () => { + // model.isCardGrabbedProperty.value = false; + // model.isKeyboardFocusedProperty.value = false; + // } + // } ); // When pdomFocusHighlightsVisibleProperty become false, interaction with a mouse has begun while using // Interactive Highlighting. When that happens, clear the sim-specific state tracking 'focused' cards. + // TODO: wat? https://github.com/phetsims/center-and-variability/issues/605 phet.joist.sim.display.focusManager.pdomFocusHighlightsVisibleProperty.link( ( visible: boolean ) => { if ( !visible ) { - if ( model.focusedCardProperty.value !== null ) { + if ( model.groupSortInteractionModel.selectedGroupItemProperty.value !== null ) { // Before clearing out the focusedCardProperty the CardModel must be cleared out of it's // dragging state. - model.focusedCardProperty.value.isDraggingProperty.set( false ); + model.groupSortInteractionModel.selectedGroupItemProperty.value.isDraggingProperty.set( false ); // Clear the 'focused' card so that there isn't a flicker to a highlight around that card when // moving between the CardNode interactive highlight and the container group highlight (which has // a custom highlight around the focused card). - model.focusedCardProperty.set( null ); + model.groupSortInteractionModel.selectedGroupItemProperty.value = null; } model.isCardGrabbedProperty.value = false; @@ -244,15 +261,10 @@ Multilink.multilink( [ focusedCardNodeProperty, model.isCardGrabbedProperty ], ( focusedCardNode, isCardGrabbed ) => { if ( focusedCardNode ) { - const focusForSelectedCard = new HighlightFromNode( focusedCardNode.cardNode, { dashed: isCardGrabbed } ); - this.setFocusHighlight( focusForSelectedCard ); - focusedCardNode.model.isDraggingProperty.value = isCardGrabbed; - keyboardSortCueNode.centerBottom = new Vector2( focusedCardNode.centerX + CARD_LAYER_OFFSET + PICK_UP_DELTA_X / 2 + 1, focusForSelectedCard.bottom + 11 ); - } - else { - this.setFocusHighlight( 'invisible' ); + keyboardSortCueNode.centerBottom = new Vector2( focusedCardNode.centerX + CARD_LAYER_OFFSET + PICK_UP_DELTA_X / 2 + 1, + ( this.focusHighlight as unknown as Node ).bottom + 11 ); } } ); @@ -301,84 +313,79 @@ animatedPanZoomSingleton.listener.panToNode( focusedCard, true ); model.cardCellsChangedEmitter.emit(); - - // Gets rid of the hand icon - model.hasKeyboardMovedCardProperty.value = true; } }; - - const keyboardListener = new KeyboardListener( { - fireOnHold: true, - keys: [ 'd', 'a', 'arrowRight', 'arrowLeft', 'w', 's', 'arrowUp', 'arrowDown', 'enter', 'space', 'home', 'end', 'escape', 'pageUp', 'pageDown' ], - callback: ( event, keysPressed ) => { - - const focusedCardNode = focusedCardNodeProperty.value; - const activeCardNodes = this.getActiveCardNodesInOrder(); - const numberOfActiveCards = activeCardNodes.length; - const isCardGrabbed = model.isCardGrabbedProperty.value; - - // If there are no active cards no card can be focused and no keyboard input is allowed. - if ( numberOfActiveCards === 0 ) { - model.focusedCardProperty.value = null; - return; - } - - if ( focusedCardNode ) { - const delta = this.getKeystrokeDelta( keysPressed, numberOfActiveCards ); - - if ( [ 'enter', 'space' ].includes( keysPressed ) ) { - model.isCardGrabbedProperty.value = !model.isCardGrabbedProperty.value; - model.hasKeyboardGrabbedCardProperty.value = true; - - // See if the user unsorted the data. If so, uncheck the "Sort Data" checkbox - if ( !model.isCardGrabbedProperty.value && this.isSortingDataProperty.value && !this.model.isDataSorted() ) { - this.isSortingDataProperty.value = false; - } - } - else if ( isCardGrabbed ) { - if ( keysPressed === 'escape' ) { - model.isCardGrabbedProperty.value = false; - } - - // If we have a nonNull delta and the card is grabbed we want to swap card positions. - else if ( delta !== null ) { - swapCards( activeCardNodes, focusedCardNode, delta ); - } - } - - // If we have a nonNull delta and the card is not grabbed we want to shift focus to a different card. - else if ( delta !== null ) { - - // Shift the card focus when a card is not grabbed. - const currentIndex = activeCardNodes.indexOf( focusedCardNode ); - const nextIndex = Utils.clamp( currentIndex + delta, 0, numberOfActiveCards - 1 ); - model.focusedCardProperty.value = activeCardNodes[ nextIndex ].model; - model.hasKeyboardSelectedDifferentCardProperty.value = true; - animatedPanZoomSingleton.listener.panToNode( focusedCardNode, true ); - } - else { - - // No cards are grabbed! We cleared the 'focused' card because we were using mouse input - start over with - // keyboard interaction and focus the first card AND make sure that all cards individually are no longer - // dragging. - this.cardNodes.forEach( cardNode => { - cardNode.model.isDraggingProperty.value = false; - } ); - model.isCardGrabbedProperty.value = false; - - model.focusedCardProperty.value = activeCardNodes[ 0 ].model; - } - } - else { - model.focusedCardProperty.value = activeCardNodes[ 0 ].model; - } - } - } ); + // + // const keyboardListener = new KeyboardListener( { + // fireOnHold: true, + // keys: [ 'd', 'a', 'arrowRight', 'arrowLeft', 'w', 's', 'arrowUp', 'arrowDown', 'enter', 'space', 'home', 'end', 'escape', 'pageUp', 'pageDown' ], + // callback: ( event, keysPressed ) => { + // + // const focusedCardNode = focusedCardNodeProperty.value; + // const activeCardNodes = this.getActiveCardNodesInOrder(); + // const numberOfActiveCards = activeCardNodes.length; + // const isCardGrabbed = model.isCardGrabbedProperty.value; + // + // // If there are no active cards no card can be focused and no keyboard input is allowed. + // if ( numberOfActiveCards === 0 ) { + // model.focusedCardProperty.value = null; + // return; + // } + // + // if ( focusedCardNode ) { + // const delta = this.getKeystrokeDelta( keysPressed, numberOfActiveCards ); + // + // if ( [ 'enter', 'space' ].includes( keysPressed ) ) { + // model.isCardGrabbedProperty.value = !model.isCardGrabbedProperty.value; + // model.hasKeyboardGrabbedCardProperty.value = true; + // + // // See if the user unsorted the data. If so, uncheck the "Sort Data" checkbox + // if ( !model.isCardGrabbedProperty.value && this.isSortingDataProperty.value && !this.model.isDataSorted() ) { + // this.isSortingDataProperty.value = false; + // } + // } + // else if ( isCardGrabbed ) { + // if ( keysPressed === 'escape' ) { + // model.isCardGrabbedProperty.value = false; + // } + // + // // If we have a nonNull delta and the card is grabbed we want to swap card positions. + // else if ( delta !== null ) { + // } + // } + // + // // If we have a nonNull delta and the card is not grabbed we want to shift focus to a different card. + // else if ( delta !== null ) { + // + // // Shift the card focus when a card is not grabbed. + // const currentIndex = activeCardNodes.indexOf( focusedCardNode ); + // const nextIndex = Utils.clamp( currentIndex + delta, 0, numberOfActiveCards - 1 ); + // model.focusedCardProperty.value = activeCardNodes[ nextIndex ].model; + // model.hasKeyboardSelectedDifferentCardProperty.value = true; + // animatedPanZoomSingleton.listener.panToNode( focusedCardNode, true ); + // } + // else { + // + // // No cards are grabbed! We cleared the 'focused' card because we were using mouse input - start over with + // // keyboard interaction and focus the first card AND make sure that all cards individually are no longer + // // dragging. + // // this.cardNodes.forEach( cardNode => { + // // cardNode.model.isDraggingProperty.value = false; + // // } ); + // model.isCardGrabbedProperty.value = false; + // + // model.focusedCardProperty.value = activeCardNodes[ 0 ].model; + // } + // } + // else { + // model.focusedCardProperty.value = activeCardNodes[ 0 ].model; + // } + // } + // } ); const focusHighlightWidthProperty = new DerivedProperty( [ model.numActiveCardsProperty ], numActiveCards => { return model.getCardPositionX( numActiveCards === 0 ? 1 : numActiveCards ); } ); - focusHighlightPath.addChild( grabReleaseCueNode ); const highlightRectangle = new Path( null ); this.addChild( highlightRectangle ); @@ -387,14 +394,11 @@ focusHighlightWidthProperty.link( focusHighlightWidth => { const marginX = 7; const focusRect = Shape.rect( -marginX, -FOCUS_HIGHLIGHT_Y_MARGIN, focusHighlightWidth + 2 * marginX, CAVConstants.CARD_DIMENSION + 2 * FOCUS_HIGHLIGHT_Y_MARGIN + 9 ); - focusHighlightPath.setShape( focusRect ); + this.groupSortInteractionView.groupFocusHighlightPath.setShape( focusRect ); highlightRectangle.setShape( focusRect ); const cueNodeWidth = grabReleaseCueNode.width; grabReleaseCueNode.centerX = Utils.clamp( focusRect.bounds.centerX, cueNodeWidth / 2, this.width - cueNodeWidth / 2 ); } ); - - this.setGroupFocusHighlight( focusHighlightPath ); - this.addInputListener( keyboardListener ); } // The listener which is linked to the cardNode.positionProperty

[zepumph]

Things are working quite well. The mouse drag indicator isn't hooked up, and there are 11 TODOs.

``` Subject: [PATCH] rename to Typescript, https://github.com/phetsims/center-and-variability/issues/605 --- Index: center-and-variability/js/median/model/InteractiveCardContainerModel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/center-and-variability/js/median/model/InteractiveCardContainerModel.ts b/center-and-variability/js/median/model/InteractiveCardContainerModel.ts --- a/center-and-variability/js/median/model/InteractiveCardContainerModel.ts (revision f5c5c53f59fd83cf9934d42fae62872377cb92f4) +++ b/center-and-variability/js/median/model/InteractiveCardContainerModel.ts (date 1705514383734) @@ -21,20 +21,17 @@ import centerAndVariability from '../../centerAndVariability.js'; import MedianModel from './MedianModel.js'; import { EmptySelfOptions } from '../../../../phet-core/js/optionize.js'; -import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; import Property from '../../../../axon/js/Property.js'; import CardModel from './CardModel.js'; import BooleanProperty from '../../../../axon/js/BooleanProperty.js'; import NumberProperty from '../../../../axon/js/NumberProperty.js'; import DerivedProperty from '../../../../axon/js/DerivedProperty.js'; -import NullableIO from '../../../../tandem/js/types/NullableIO.js'; -import ReferenceIO from '../../../../tandem/js/types/ReferenceIO.js'; -import Tandem from '../../../../tandem/js/Tandem.js'; import dotRandom from '../../../../dot/js/dotRandom.js'; import CAVQueryParameters from '../../common/CAVQueryParameters.js'; import Emitter from '../../../../axon/js/Emitter.js'; import isResettingProperty from '../../../../soccer-common/js/model/isResettingProperty.js'; import isSettingPhetioStateProperty from '../../../../tandem/js/isSettingPhetioStateProperty.js'; +import GroupSortInteractionModel from '../../../../scenery-phet/js/accessibility/group-sort/model/GroupSortInteractionModel.js'; const cardMovementSounds = [ cardMovement1_mp3, @@ -74,34 +71,25 @@ // For sonification, order the active, non-displaced cards appeared in the last step private lastStepOrder: CardModel[] = []; - // Indicates whether the user has ever dragged a card, used to determine dragIndicationCardProperty. - public readonly hasDraggedCardProperty: TReadOnlyProperty; - - // The card where the drag indicator should appear. null if no drag indicator should appear. - public readonly dragIndicationCardProperty: Property; - public readonly totalDragDistanceProperty: Property; - // KEYBOARD INPUT PROPERTIES: - // The card which currently has focus. Is part of what controls highlight visibility among other things. - public readonly focusedCardProperty: Property = new Property( null ); - + // TODO: delete https://github.com/phetsims/scenery-phet/issues/815 // Tracks when a card is currently grabbed via keyboard input. public readonly isCardGrabbedProperty = new BooleanProperty( false ); - // Visible Properties for keyboard hints - public readonly grabReleaseCueVisibleProperty: TReadOnlyProperty; - public readonly isKeyboardDragArrowVisibleProperty: TReadOnlyProperty; - // Properties that track if a certain action has ever been performed via keyboard input. - public readonly hasKeyboardMovedCardProperty: Property; + // TODO: delete https://github.com/phetsims/scenery-phet/issues/815 public readonly hasKeyboardGrabbedCardProperty = new BooleanProperty( false ); + // TODO: delete https://github.com/phetsims/scenery-phet/issues/815 public readonly hasKeyboardSelectedDifferentCardProperty = new BooleanProperty( false ); // Property that is triggered via focus and blur events in the InteractiveCardNodeContainer + // TODO: delete https://github.com/phetsims/scenery-phet/issues/815 public readonly isKeyboardFocusedProperty = new BooleanProperty( false ); public readonly manuallySortedEmitter: Emitter; + public readonly groupSortInteractionModel: GroupSortInteractionModel; + public constructor( medianModel: MedianModel, providedOptions: InteractiveCardContainerModelOptions ) { super( medianModel, providedOptions ); @@ -111,68 +99,56 @@ phetioDocumentation: 'Accumulated card drag distance, for purposes of hiding the drag indicator node' } ); - this.hasKeyboardMovedCardProperty = new BooleanProperty( false, { - tandem: providedOptions.tandem.createTandem( 'hasKeyboardMovedCardProperty' ), - phetioReadOnly: true, // controlled by the sim - phetioDocumentation: 'Whether a card been moved using the keyboard, for purposes of hiding the drag indicator node' - } ); - - this.hasDraggedCardProperty = new DerivedProperty( - [ this.totalDragDistanceProperty, this.hasKeyboardMovedCardProperty ], - ( totalDragDistance, hasKeyboardMovedCard ) => totalDragDistance > 15 || hasKeyboardMovedCard - ); - - this.dragIndicationCardProperty = new Property( null, { - phetioReadOnly: true, - phetioValueType: NullableIO( ReferenceIO( CardModel.CardModelIO ) ), - tandem: this.representationContext === 'accordion' ? providedOptions.tandem.createTandem( 'cardDragIndicatorProperty' ) : Tandem.OPT_OUT, - phetioDocumentation: 'This is for PhET-iO internal use only.' + this.groupSortInteractionModel = new GroupSortInteractionModel( { + getValueProperty: cardModel => cardModel.indexProperty, + tandem: providedOptions.tandem.createTandem( 'groupSortInteractionModel' ) } ); - this.isKeyboardDragArrowVisibleProperty = new DerivedProperty( [ this.focusedCardProperty, this.hasKeyboardMovedCardProperty, this.hasKeyboardGrabbedCardProperty, - this.isCardGrabbedProperty, this.isKeyboardFocusedProperty ], - ( focusedCard, hasKeyboardMovedCard, hasGrabbedCard, isCardGrabbed, hasKeyboardFocus ) => { - return focusedCard !== null && !hasKeyboardMovedCard && hasGrabbedCard && isCardGrabbed && hasKeyboardFocus; - } ); + this.totalDragDistanceProperty.link( totalDragDistance => { + this.groupSortInteractionModel.hasGroupItemBeenSortedProperty.value = totalDragDistance > 15 || + this.groupSortInteractionModel.hasGroupItemBeenSortedProperty.value; + } ); - this.grabReleaseCueVisibleProperty = new DerivedProperty( [ this.focusedCardProperty, this.hasKeyboardGrabbedCardProperty, this.isKeyboardFocusedProperty ], - ( focusedCard, hasGrabbedCard, hasKeyboardFocus ) => { - return focusedCard !== null && !hasGrabbedCard && hasKeyboardFocus; - } ); this.cardCellsChangedEmitter.addListener( () => { medianModel.areCardsSortedProperty.value = this.isDataSorted(); } ); - const updateDragIndicationCardProperty = () => { + const updateMouseSortCueNode = () => { + + // TODO: sloppy https://github.com/phetsims/center-and-variability/issues/605 const leftCard = this.getCardsInCellOrder()[ 0 ]; const rightCard = this.getCardsInCellOrder()[ 1 ]; // If the user has not yet dragged a card and there are multiple cards showing, add the drag indicator to leftCard. - if ( !this.hasDraggedCardProperty.value && leftCard && rightCard ) { - this.dragIndicationCardProperty.value = leftCard; + // TODO: not any interaction sorted, just for mouse, https://github.com/phetsims/center-and-variability/issues/605 + if ( !this.groupSortInteractionModel.hasGroupItemBeenSortedProperty.value && leftCard && rightCard ) { + // TODO:???? https://github.com/phetsims/center-and-variability/issues/605 + // this.groupSortInteractionModel.selectedGroupItemProperty.value = leftCard; } // If the user has dragged a card, then the drag indicator does not need to be shown. - if ( this.hasDraggedCardProperty.value || !leftCard || !rightCard ) { - this.dragIndicationCardProperty.value = null; + // TODO: not any interaction sorted, just for mouse, https://github.com/phetsims/center-and-variability/issues/605 + if ( this.groupSortInteractionModel.hasGroupItemBeenSortedProperty.value || !leftCard || !rightCard ) { + this.groupSortInteractionModel.mouseSortCueVisibleProperty.value = false; } }; - this.cardCellsChangedEmitter.addListener( updateDragIndicationCardProperty ); - this.hasDraggedCardProperty.link( updateDragIndicationCardProperty ); - this.cards.forEach( card => card.soccerBall.valueProperty.lazyLink( updateDragIndicationCardProperty ) ); + this.cardCellsChangedEmitter.addListener( updateMouseSortCueNode ); + this.groupSortInteractionModel.hasGroupItemBeenSortedProperty.link( updateMouseSortCueNode ); + this.cards.forEach( card => card.soccerBall.valueProperty.lazyLink( updateMouseSortCueNode ) ); medianModel.selectedSceneModelProperty.value.resetEmitter.addListener( () => { this.totalDragDistanceProperty.reset(); - this.hasKeyboardMovedCardProperty.reset(); + this.groupSortInteractionModel.reset(); // TODO: not everything!? https://github.com/phetsims/center-and-variability/issues/605 this.hasKeyboardGrabbedCardProperty.reset(); this.hasKeyboardSelectedDifferentCardProperty.reset(); } ); medianModel.selectedSceneModelProperty.value.preClearDataEmitter.addListener( () => { - this.focusedCardProperty.reset(); + this.groupSortInteractionModel.resetInteractionState(); + // TODO: delete https://github.com/phetsims/scenery-phet/issues/815 this.isCardGrabbedProperty.reset(); } ); Index: scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.ts b/scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.ts --- a/scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.ts (revision c909d03c4d0c7d9bdc59972ddf53bc9b71b1a01c) +++ b/scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.ts (date 1705515392182) @@ -43,6 +43,10 @@ // The available range for storing. This is the acceptable range for the valueProperty of ItemModel (see getValueProperty()). sortingRange: Range; + // Do the sort operation, allowing for custom actions, defaults to just updating the valueProperty of the selected + // group item to the new value. + sortGroupItem?: ( groupItem: ItemModel, newValue: number ) => void; + // Called when a group item is sorted. Note that this may not have changed its value. onSort?: ( groupItem: ItemModel, oldValue: number ) => void; @@ -66,12 +70,13 @@ export default class GroupSortInteractionView extends Disposable { // Update group highlight dynamically by setting the `shape` of this path. - protected readonly groupFocusHighlightPath: HighlightPath; + public readonly groupFocusHighlightPath: HighlightPath; // Emitted when the sorting cue should be repositioned. Most likely because the selection has changed. public readonly positionSortCueNodeEmitter = new Emitter(); private readonly getNodeFromModelItem: ( model: ItemModel ) => ItemNode | null; + private readonly sortGroupItem: ( groupItem: ItemModel, newValue: number ) => void; private readonly onSort: ( groupItem: ItemModel, oldValue: number ) => void; private readonly sortingRange: Range; private readonly sortStep: number; @@ -89,12 +94,16 @@ numberKeyMapper: null, onSort: _.noop, sortStep: 1, - pageSortStep: Math.ceil( providedOptions.sortingRange.getLength() / 5 ) + pageSortStep: Math.ceil( providedOptions.sortingRange.getLength() / 5 ), + sortGroupItem: ( groupItem, newValue ) => { + this.getValueProperty( groupItem ).value = newValue; + } }, providedOptions ); super( options ); this.getNodeFromModelItem = options.getNodeFromModelItem; + this.sortGroupItem = options.sortGroupItem; this.onSort = options.onSort; this.sortingRange = options.sortingRange; this.sortStep = options.sortStep; @@ -125,6 +134,8 @@ isGroupItemKeyboardGrabbedProperty.value = false; isKeyboardFocusedProperty.value = false; }, + + // TODO: this isn't part of InteractiveCardContainer? https://github.com/phetsims/scenery-phet/issues/815 over: () => { // TODO: MS!!!! this is awkward. In this situation: // 1. tab to populated node, the keyboard grab cue is shown. @@ -203,7 +214,8 @@ if ( selectedGroupItemProperty.value !== null ) { const groupItem = selectedGroupItemProperty.value; - const oldValue = this.getValueProperty( groupItem ).value!; + const valueProperty = this.getValueProperty( groupItem ); + const oldValue = valueProperty.value!; assert && assert( oldValue !== null, 'We should have a group item when responding to input?' ); // Sorting an item @@ -224,10 +236,12 @@ // TODO: DESIGN!!! This changes the behavior because now the WASD, page up/page down keys work // for the selection too - they don't on published version (Note that home and end DO work on published // version for selection), https://github.com/phetsims/scenery-phet/issues/815 - const delta = this.getDeltaForKey( keysPressed ); - if ( delta !== null ) { + const unclampedDelta = this.getDeltaForKey( keysPressed ); + if ( unclampedDelta !== null ) { this.groupSortInteractionModel.hasKeyboardSelectedDifferentGroupItemProperty.value = true; - selectedGroupItemProperty.value = options.getNextSelectedGroupItem( delta ); + + const clampedDelta = this.sortingRange.clampDelta( oldValue, unclampedDelta ); + selectedGroupItemProperty.value = options.getNextSelectedGroupItem( clampedDelta ); } } this.onGroupItemChange( groupItem ); @@ -263,7 +277,7 @@ } ); } - const defaultGroupShape = primaryFocusedNode.bounds.isFinite() ? Shape.bounds( primaryFocusedNode.visibleBounds ) : null; + const defaultGroupShape = primaryFocusedNode.visibleBounds.isFinite() ? Shape.bounds( primaryFocusedNode.visibleBounds ) : null; // Set the outer group focus highlight to surround the entire area where group items are located. this.groupFocusHighlightPath = new HighlightPath( defaultGroupShape, { @@ -305,7 +319,7 @@ private onSortedValue( groupItem: ItemModel, value: number, oldValue: number ): void { assert && assert( value !== null, 'We should have a value for the group item by the end of the listener.' ); - this.getValueProperty( groupItem ).value = this.sortingRange.constrainValue( value ); + this.sortGroupItem( groupItem, this.sortingRange.constrainValue( value ) ); // TODO: DESIGN!!! fire this even if the value didn't change? Yes likely, for the sound https://github.com/phetsims/scenery-phet/issues/815 this.onSort( groupItem, oldValue ); @@ -313,6 +327,10 @@ this.groupSortInteractionModel.hasGroupItemBeenSortedProperty.value = true; } + /** + * Get the delta to change the value given what key was pressed. The returned delta may not result in a value in range, + * please constrain value from range or provide your own defensive measures to this delta. + */ private getDeltaForKey( key: string ): number | null { const fullRange = this.sortingRange.getLength(); return key === 'home' ? -fullRange : Index: center-and-variability/js/median/view/InteractiveCardNodeContainer.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/center-and-variability/js/median/view/InteractiveCardNodeContainer.ts b/center-and-variability/js/median/view/InteractiveCardNodeContainer.ts --- a/center-and-variability/js/median/view/InteractiveCardNodeContainer.ts (revision f5c5c53f59fd83cf9934d42fae62872377cb92f4) +++ b/center-and-variability/js/median/view/InteractiveCardNodeContainer.ts (date 1705515392219) @@ -24,8 +24,9 @@ import InteractiveCardContainerModel from '../model/InteractiveCardContainerModel.js'; import Property from '../../../../axon/js/Property.js'; import CAVSoccerSceneModel from '../../common/model/CAVSoccerSceneModel.js'; -import { animatedPanZoomSingleton, HighlightFromNode, HighlightPath, KeyboardListener, NodeTranslationOptions, Path } from '../../../../scenery/js/imports.js'; +import { animatedPanZoomSingleton, Node, NodeTranslationOptions, Path } from '../../../../scenery/js/imports.js'; import Vector2 from '../../../../dot/js/Vector2.js'; +import Range from '../../../../dot/js/Range.js'; import CAVConstants from '../../common/CAVConstants.js'; import Bounds2 from '../../../../dot/js/Bounds2.js'; import CardNode, { cardDropClip, cardPickUpSoundClip, PICK_UP_DELTA_X } from './CardNode.js'; @@ -34,7 +35,6 @@ import { Shape } from '../../../../kite/js/imports.js'; import Multilink from '../../../../axon/js/Multilink.js'; import isSettingPhetioStateProperty from '../../../../tandem/js/isSettingPhetioStateProperty.js'; -import GrabReleaseCueNode from '../../../../scenery-phet/js/accessibility/nodes/GrabReleaseCueNode.js'; import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; import CelebrationNode from './CelebrationNode.js'; import checkboxCheckedSoundPlayer from '../../../../tambo/js/shared-sound-players/checkboxCheckedSoundPlayer.js'; @@ -43,6 +43,7 @@ import CardDragIndicatorNode from './CardDragIndicatorNode.js'; import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js'; import GroupSortInteractionView from '../../../../scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.js'; +import CardModel from '../model/CardModel.js'; const FOCUS_HIGHLIGHT_Y_MARGIN = CAVConstants.CARD_SPACING + 3; @@ -61,6 +62,7 @@ // The message that appears when the cards are sorted private readonly celebrationNode: CelebrationNode; + private readonly groupSortInteractionView: GroupSortInteractionView; public constructor( model: InteractiveCardContainerModel, private readonly isSortingDataProperty: Property, @@ -78,6 +80,41 @@ this.celebrationNode = new CelebrationNode( model, this.cardMap, this.sceneModel.resetEmitter ); this.addChild( this.celebrationNode ); + this.groupSortInteractionView = new GroupSortInteractionView( model.groupSortInteractionModel, this, { + getNextSelectedGroupItem: unclampedDelta => { + // TODO: we have to do this every time, perhaps a parameter? https://github.com/phetsims/center-and-variability/issues/605 + const selectedCardModel = model.groupSortInteractionModel.selectedGroupItemProperty.value!; + const currentIndex = selectedCardModel.indexProperty.value!; + assert && assert( currentIndex !== null, 'need an index to be sorted' ); + + // TODO: until range is dynamic, this could be outside of current cards https://github.com/phetsims/center-and-variability/issues/605 + const delta = new Range( 0, this.getActiveCardNodesInOrder().length - 1 ).clampDelta( currentIndex, unclampedDelta ); + const newIndex = currentIndex + delta; + const cardNodes = this.getActiveCardNodesInOrder(); + const newCardNode = cardNodes[ newIndex ]; + assert && assert( newCardNode, 'wrong index for available cards I believe' ); + return newCardNode.model; + }, + sortGroupItem: ( selectedCardModel, newValue ) => { + assert && assert( selectedCardModel.indexProperty.value !== null, 'need an index to be sorted' ); + const delta = newValue - selectedCardModel.indexProperty.value!; + swapCards( this.getActiveCardNodesInOrder(), this.cardMap.get( selectedCardModel )!, delta ); + }, + getGroupItemToSelect: () => { + const activeCards = this.getActiveCardNodesInOrder(); + return activeCards[ 0 ] ? activeCards[ 0 ].model : null; + }, + getNodeFromModelItem: cardModel => this.cardMap.get( cardModel ) || null, + sortingRange: new Range( 0, this.model.cards.length - 1 ) // TODO: Need to support Property(Range) https://github.com/phetsims/center-and-variability/issues/605 + } ); + + model.groupSortInteractionModel.isGroupItemKeyboardGrabbedProperty.lazyLink( grabbed => { + // TODO: use onRelease() option instead? https://github.com/phetsims/center-and-variability/issues/605 + this.cardNodes.forEach( cardNode => { + cardNode.model.isDraggingProperty.value = false; + } ); + } ); + this.cardMap.forEach( ( cardNode, cardModel ) => { // Update the position of all cards (via animation) whenever any card is dragged cardNode.model.positionProperty.link( this.createDragPositionListener( cardNode ) ); @@ -107,32 +144,24 @@ } ); } ); - const focusHighlightPath = new HighlightPath( null, { - outerStroke: HighlightPath.OUTER_LIGHT_GROUP_FOCUS_COLOR, - innerStroke: HighlightPath.INNER_LIGHT_GROUP_FOCUS_COLOR, - outerLineWidth: HighlightPath.GROUP_OUTER_LINE_WIDTH, - innerLineWidth: HighlightPath.GROUP_INNER_LINE_WIDTH - } ); - - const grabReleaseCueNode = new GrabReleaseCueNode( { - top: CAVConstants.CARD_DIMENSION + FOCUS_HIGHLIGHT_Y_MARGIN + 15, - visibleProperty: model.grabReleaseCueVisibleProperty + const grabReleaseCueNode = GroupSortInteractionView.createGrabReleaseCueNode( model.groupSortInteractionModel.grabReleaseCueVisibleProperty, { + top: CAVConstants.CARD_DIMENSION + FOCUS_HIGHLIGHT_Y_MARGIN + 15 } ); - const keyboardSortCueNode = GroupSortInteractionView.createSortCueNode( model.isKeyboardDragArrowVisibleProperty ); + const keyboardSortCueNode = GroupSortInteractionView.createSortCueNode( model.groupSortInteractionModel.keyboardSortCueVisibleProperty ); this.addChild( keyboardSortCueNode ); const cardDragIndicatorNode = new CardDragIndicatorNode( { centerTop: new Vector2( 0.5 * CAVConstants.CARD_DIMENSION - PICK_UP_DELTA_X, CAVConstants.CARD_DIMENSION - 10 ), visibleProperty: new DerivedProperty( - [ this.inputEnabledProperty, model.isKeyboardFocusedProperty, model.dragIndicationCardProperty ], + [ this.inputEnabledProperty, model.isKeyboardFocusedProperty, model.groupSortInteractionModel.selectedGroupItemProperty ], ( inputEnabled, hasKeyboardFocus, dragIndicationCard ) => inputEnabled && !hasKeyboardFocus && !!dragIndicationCard ) } ); this.addChild( cardDragIndicatorNode ); - model.dragIndicationCardProperty.lazyLink( ( newCard, oldCard ) => { + model.groupSortInteractionModel.selectedGroupItemProperty.lazyLink( ( newCard, oldCard ) => { if ( oldCard ) { const oldCardNode = this.cardMap.get( oldCard )!; @@ -176,7 +205,7 @@ // Needs to be pickable in accordion box. this.pickable = true; - const focusedCardNodeProperty: TReadOnlyProperty = new DerivedProperty( [ model.focusedCardProperty ], focusedCard => { + const focusedCardNodeProperty: TReadOnlyProperty = new DerivedProperty( [ model.groupSortInteractionModel.selectedGroupItemProperty ], focusedCard => { return focusedCard === null ? focusedCard : this.cardMap.get( focusedCard )!; } ); @@ -187,50 +216,52 @@ // When a user is focused on the card container but there are no cards yet, we want to ensure that a card gets focused // once there is a card. - if ( model.focusedCardProperty.value === null && this.focused && model.getActiveCards().length === 1 ) { - model.focusedCardProperty.value = activeCardNodes[ 0 ].model; + // TODO: this.focused should use the model https://github.com/phetsims/center-and-variability/issues/605 + if ( model.groupSortInteractionModel.selectedGroupItemProperty.value === null && this.focused && model.getActiveCards().length === 1 ) { + model.groupSortInteractionModel.selectedGroupItemProperty.value = activeCardNodes[ 0 ].model; } - // If the card cells changed, and we have no more active cards left, that means that all the cards were removed. + // If the card cells changed, and we have no more active cards left, that means that all the cards were removed. // Therefore, we want to set the focused card to null. else if ( model.getActiveCards().length === 0 ) { - model.focusedCardProperty.value = null; - } - } ); - - this.addInputListener( { - focus: () => { - const activeCardNodes = this.getActiveCardNodesInOrder(); - if ( model.focusedCardProperty.value === null && activeCardNodes.length > 0 ) { - model.focusedCardProperty.value = activeCardNodes[ 0 ].model; - } - - // When the group receives keyboard focus, make sure that the focused card is displayed - if ( focusedCardNodeProperty.value ) { - animatedPanZoomSingleton.listener.panToNode( focusedCardNodeProperty.value, true ); - } - model.isKeyboardFocusedProperty.value = true; - }, - blur: () => { - model.isCardGrabbedProperty.value = false; - model.isKeyboardFocusedProperty.value = false; + model.groupSortInteractionModel.selectedGroupItemProperty.value = null; } } ); + // + // this.addInputListener( { + // focus: () => { + // const activeCardNodes = this.getActiveCardNodesInOrder(); + // if ( model.focusedCardProperty.value === null && activeCardNodes.length > 0 ) { + // model.focusedCardProperty.value = activeCardNodes[ 0 ].model; + // } + // + // // When the group receives keyboard focus, make sure that the focused card is displayed + // if ( focusedCardNodeProperty.value ) { + // animatedPanZoomSingleton.listener.panToNode( focusedCardNodeProperty.value, true ); + // } + // model.isKeyboardFocusedProperty.value = true; + // }, + // blur: () => { + // model.isCardGrabbedProperty.value = false; + // model.isKeyboardFocusedProperty.value = false; + // } + // } ); // When pdomFocusHighlightsVisibleProperty become false, interaction with a mouse has begun while using // Interactive Highlighting. When that happens, clear the sim-specific state tracking 'focused' cards. + // TODO: wat? https://github.com/phetsims/center-and-variability/issues/605 phet.joist.sim.display.focusManager.pdomFocusHighlightsVisibleProperty.link( ( visible: boolean ) => { if ( !visible ) { - if ( model.focusedCardProperty.value !== null ) { + if ( model.groupSortInteractionModel.selectedGroupItemProperty.value !== null ) { // Before clearing out the focusedCardProperty the CardModel must be cleared out of it's // dragging state. - model.focusedCardProperty.value.isDraggingProperty.set( false ); + model.groupSortInteractionModel.selectedGroupItemProperty.value.isDraggingProperty.set( false ); // Clear the 'focused' card so that there isn't a flicker to a highlight around that card when // moving between the CardNode interactive highlight and the container group highlight (which has // a custom highlight around the focused card). - model.focusedCardProperty.set( null ); + model.groupSortInteractionModel.selectedGroupItemProperty.value = null; } model.isCardGrabbedProperty.value = false; @@ -244,15 +275,10 @@ Multilink.multilink( [ focusedCardNodeProperty, model.isCardGrabbedProperty ], ( focusedCardNode, isCardGrabbed ) => { if ( focusedCardNode ) { - const focusForSelectedCard = new HighlightFromNode( focusedCardNode.cardNode, { dashed: isCardGrabbed } ); - this.setFocusHighlight( focusForSelectedCard ); - focusedCardNode.model.isDraggingProperty.value = isCardGrabbed; - keyboardSortCueNode.centerBottom = new Vector2( focusedCardNode.centerX + CARD_LAYER_OFFSET + PICK_UP_DELTA_X / 2 + 1, focusForSelectedCard.bottom + 11 ); - } - else { - this.setFocusHighlight( 'invisible' ); + keyboardSortCueNode.centerBottom = new Vector2( focusedCardNode.centerX + CARD_LAYER_OFFSET + PICK_UP_DELTA_X / 2 + 1, + ( this.focusHighlight as unknown as Node ).bottom + 11 ); } } ); @@ -278,6 +304,7 @@ // Move and swap cards according to the focused card's target index. Used for alternative input. const swapCards = ( activeCards: CardNode[], focusedCard: CardNode, delta: number ) => { const currentIndex = activeCards.indexOf( focusedCard ); + assert && assert( focusedCard.model.indexProperty.value === currentIndex, 'sanity check' ); const targetIndex = Utils.clamp( currentIndex + delta, 0, activeCards.length - 1 ); if ( targetIndex !== currentIndex ) { @@ -301,85 +328,80 @@ animatedPanZoomSingleton.listener.panToNode( focusedCard, true ); model.cardCellsChangedEmitter.emit(); - - // Gets rid of the hand icon - model.hasKeyboardMovedCardProperty.value = true; } }; - - const keyboardListener = new KeyboardListener( { - fireOnHold: true, - keys: [ 'd', 'a', 'arrowRight', 'arrowLeft', 'w', 's', 'arrowUp', 'arrowDown', 'enter', 'space', 'home', 'end', 'escape', 'pageUp', 'pageDown' ], - callback: ( event, keysPressed ) => { - - const focusedCardNode = focusedCardNodeProperty.value; - const activeCardNodes = this.getActiveCardNodesInOrder(); - const numberOfActiveCards = activeCardNodes.length; - const isCardGrabbed = model.isCardGrabbedProperty.value; - - // If there are no active cards no card can be focused and no keyboard input is allowed. - if ( numberOfActiveCards === 0 ) { - model.focusedCardProperty.value = null; - return; - } - - if ( focusedCardNode ) { - const delta = this.getKeystrokeDelta( keysPressed, numberOfActiveCards ); - - if ( [ 'enter', 'space' ].includes( keysPressed ) ) { - model.isCardGrabbedProperty.value = !model.isCardGrabbedProperty.value; - model.hasKeyboardGrabbedCardProperty.value = true; - - // See if the user unsorted the data. If so, uncheck the "Sort Data" checkbox - if ( !model.isCardGrabbedProperty.value && this.isSortingDataProperty.value && !this.model.isDataSorted() ) { - this.isSortingDataProperty.value = false; - } - } - else if ( isCardGrabbed ) { - if ( keysPressed === 'escape' ) { - model.isCardGrabbedProperty.value = false; - } - - // If we have a nonNull delta and the card is grabbed we want to swap card positions. - else if ( delta !== null ) { - swapCards( activeCardNodes, focusedCardNode, delta ); - } - } - - // If we have a nonNull delta and the card is not grabbed we want to shift focus to a different card. - else if ( delta !== null ) { - - // Shift the card focus when a card is not grabbed. - const currentIndex = activeCardNodes.indexOf( focusedCardNode ); - const nextIndex = Utils.clamp( currentIndex + delta, 0, numberOfActiveCards - 1 ); - model.focusedCardProperty.value = activeCardNodes[ nextIndex ].model; - model.hasKeyboardSelectedDifferentCardProperty.value = true; - animatedPanZoomSingleton.listener.panToNode( focusedCardNode, true ); - } - else { - - // No cards are grabbed! We cleared the 'focused' card because we were using mouse input - start over with - // keyboard interaction and focus the first card AND make sure that all cards individually are no longer - // dragging. - this.cardNodes.forEach( cardNode => { - cardNode.model.isDraggingProperty.value = false; - } ); - model.isCardGrabbedProperty.value = false; - - model.focusedCardProperty.value = activeCardNodes[ 0 ].model; - } - } - else { - model.focusedCardProperty.value = activeCardNodes[ 0 ].model; - } - } - } ); + // + // const keyboardListener = new KeyboardListener( { + // fireOnHold: true, + // keys: [ 'd', 'a', 'arrowRight', 'arrowLeft', 'w', 's', 'arrowUp', 'arrowDown', 'enter', 'space', 'home', 'end', 'escape', 'pageUp', 'pageDown' ], + // callback: ( event, keysPressed ) => { + // + // const focusedCardNode = focusedCardNodeProperty.value; + // const activeCardNodes = this.getActiveCardNodesInOrder(); + // const numberOfActiveCards = activeCardNodes.length; + // const isCardGrabbed = model.isCardGrabbedProperty.value; + // + // // If there are no active cards no card can be focused and no keyboard input is allowed. + // if ( numberOfActiveCards === 0 ) { + // model.focusedCardProperty.value = null; + // return; + // } + // + // if ( focusedCardNode ) { + // const delta = this.getKeystrokeDelta( keysPressed, numberOfActiveCards ); + // + // if ( [ 'enter', 'space' ].includes( keysPressed ) ) { + // model.isCardGrabbedProperty.value = !model.isCardGrabbedProperty.value; + // model.hasKeyboardGrabbedCardProperty.value = true; + // + // // See if the user unsorted the data. If so, uncheck the "Sort Data" checkbox + // if ( !model.isCardGrabbedProperty.value && this.isSortingDataProperty.value && !this.model.isDataSorted() ) { + // this.isSortingDataProperty.value = false; + // } + // } + // else if ( isCardGrabbed ) { + // if ( keysPressed === 'escape' ) { + // model.isCardGrabbedProperty.value = false; + // } + // + // // If we have a nonNull delta and the card is grabbed we want to swap card positions. + // else if ( delta !== null ) { + // } + // } + // + // // If we have a nonNull delta and the card is not grabbed we want to shift focus to a different card. + // else if ( delta !== null ) { + // + // // Shift the card focus when a card is not grabbed. + // const currentIndex = activeCardNodes.indexOf( focusedCardNode ); + // const nextIndex = Utils.clamp( currentIndex + delta, 0, numberOfActiveCards - 1 ); + // model.focusedCardProperty.value = activeCardNodes[ nextIndex ].model; + // model.hasKeyboardSelectedDifferentCardProperty.value = true; + // animatedPanZoomSingleton.listener.panToNode( focusedCardNode, true ); + // } + // else { + // + // // No cards are grabbed! We cleared the 'focused' card because we were using mouse input - start over with + // // keyboard interaction and focus the first card AND make sure that all cards individually are no longer + // // dragging. + // // this.cardNodes.forEach( cardNode => { + // // cardNode.model.isDraggingProperty.value = false; + // // } ); + // model.isCardGrabbedProperty.value = false; + // + // model.focusedCardProperty.value = activeCardNodes[ 0 ].model; + // } + // } + // else { + // model.focusedCardProperty.value = activeCardNodes[ 0 ].model; + // } + // } + // } ); const focusHighlightWidthProperty = new DerivedProperty( [ model.numActiveCardsProperty ], numActiveCards => { return model.getCardPositionX( numActiveCards === 0 ? 1 : numActiveCards ); } ); - focusHighlightPath.addChild( grabReleaseCueNode ); - + // TODO: What is this highlightRect? https://github.com/phetsims/scenery-phet/issues/815 const highlightRectangle = new Path( null ); this.addChild( highlightRectangle ); highlightRectangle.moveToBack(); @@ -387,14 +409,11 @@ focusHighlightWidthProperty.link( focusHighlightWidth => { const marginX = 7; const focusRect = Shape.rect( -marginX, -FOCUS_HIGHLIGHT_Y_MARGIN, focusHighlightWidth + 2 * marginX, CAVConstants.CARD_DIMENSION + 2 * FOCUS_HIGHLIGHT_Y_MARGIN + 9 ); - focusHighlightPath.setShape( focusRect ); + this.groupSortInteractionView.groupFocusHighlightPath.setShape( focusRect ); highlightRectangle.setShape( focusRect ); const cueNodeWidth = grabReleaseCueNode.width; grabReleaseCueNode.centerX = Utils.clamp( focusRect.bounds.centerX, cueNodeWidth / 2, this.width - cueNodeWidth / 2 ); } ); - - this.setGroupFocusHighlight( focusHighlightPath ); - this.addInputListener( keyboardListener ); } // The listener which is linked to the cardNode.positionProperty Index: scenery-phet/js/accessibility/group-sort/model/GroupSortInteractionModel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/scenery-phet/js/accessibility/group-sort/model/GroupSortInteractionModel.ts b/scenery-phet/js/accessibility/group-sort/model/GroupSortInteractionModel.ts --- a/scenery-phet/js/accessibility/group-sort/model/GroupSortInteractionModel.ts (revision c909d03c4d0c7d9bdc59972ddf53bc9b71b1a01c) +++ b/scenery-phet/js/accessibility/group-sort/model/GroupSortInteractionModel.ts (date 1705512399665) @@ -96,6 +96,7 @@ // Whether any group item has ever been sorted to a new value, even if not by the group sort interaction. For best results, // set this to true from other interactions too (like mouse/touch). + // TODO: this should be derived, and a new Property for just mouse created. https://github.com/phetsims/scenery-phet/issues/815 public readonly hasGroupItemBeenSortedProperty: Property; public readonly getValueProperty: ( itemModel: ItemModel ) => TProperty;

[zepumph]

It is possible this is ready for commit, but I don't have time to confirm

```diff Subject: [PATCH] rename to Typescript, https://github.com/phetsims/center-and-variability/issues/605 --- Index: center-and-variability/js/median/model/InteractiveCardContainerModel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/center-and-variability/js/median/model/InteractiveCardContainerModel.ts b/center-and-variability/js/median/model/InteractiveCardContainerModel.ts --- a/center-and-variability/js/median/model/InteractiveCardContainerModel.ts (revision f5c5c53f59fd83cf9934d42fae62872377cb92f4) +++ b/center-and-variability/js/median/model/InteractiveCardContainerModel.ts (date 1705529699879) @@ -21,20 +21,17 @@ import centerAndVariability from '../../centerAndVariability.js'; import MedianModel from './MedianModel.js'; import { EmptySelfOptions } from '../../../../phet-core/js/optionize.js'; -import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; import Property from '../../../../axon/js/Property.js'; import CardModel from './CardModel.js'; import BooleanProperty from '../../../../axon/js/BooleanProperty.js'; import NumberProperty from '../../../../axon/js/NumberProperty.js'; import DerivedProperty from '../../../../axon/js/DerivedProperty.js'; -import NullableIO from '../../../../tandem/js/types/NullableIO.js'; -import ReferenceIO from '../../../../tandem/js/types/ReferenceIO.js'; -import Tandem from '../../../../tandem/js/Tandem.js'; import dotRandom from '../../../../dot/js/dotRandom.js'; import CAVQueryParameters from '../../common/CAVQueryParameters.js'; import Emitter from '../../../../axon/js/Emitter.js'; import isResettingProperty from '../../../../soccer-common/js/model/isResettingProperty.js'; import isSettingPhetioStateProperty from '../../../../tandem/js/isSettingPhetioStateProperty.js'; +import GroupSortInteractionModel from '../../../../scenery-phet/js/accessibility/group-sort/model/GroupSortInteractionModel.js'; const cardMovementSounds = [ cardMovement1_mp3, @@ -74,34 +71,12 @@ // For sonification, order the active, non-displaced cards appeared in the last step private lastStepOrder: CardModel[] = []; - // Indicates whether the user has ever dragged a card, used to determine dragIndicationCardProperty. - public readonly hasDraggedCardProperty: TReadOnlyProperty; - - // The card where the drag indicator should appear. null if no drag indicator should appear. - public readonly dragIndicationCardProperty: Property; - public readonly totalDragDistanceProperty: Property; - // KEYBOARD INPUT PROPERTIES: - // The card which currently has focus. Is part of what controls highlight visibility among other things. - public readonly focusedCardProperty: Property = new Property( null ); - - // Tracks when a card is currently grabbed via keyboard input. - public readonly isCardGrabbedProperty = new BooleanProperty( false ); - - // Visible Properties for keyboard hints - public readonly grabReleaseCueVisibleProperty: TReadOnlyProperty; - public readonly isKeyboardDragArrowVisibleProperty: TReadOnlyProperty; - - // Properties that track if a certain action has ever been performed via keyboard input. - public readonly hasKeyboardMovedCardProperty: Property; - public readonly hasKeyboardGrabbedCardProperty = new BooleanProperty( false ); - public readonly hasKeyboardSelectedDifferentCardProperty = new BooleanProperty( false ); - - // Property that is triggered via focus and blur events in the InteractiveCardNodeContainer - public readonly isKeyboardFocusedProperty = new BooleanProperty( false ); public readonly manuallySortedEmitter: Emitter; + public readonly groupSortInteractionModel: GroupSortInteractionModel; + public constructor( medianModel: MedianModel, providedOptions: InteractiveCardContainerModelOptions ) { super( medianModel, providedOptions ); @@ -111,71 +86,42 @@ phetioDocumentation: 'Accumulated card drag distance, for purposes of hiding the drag indicator node' } ); - this.hasKeyboardMovedCardProperty = new BooleanProperty( false, { - tandem: providedOptions.tandem.createTandem( 'hasKeyboardMovedCardProperty' ), - phetioReadOnly: true, // controlled by the sim - phetioDocumentation: 'Whether a card been moved using the keyboard, for purposes of hiding the drag indicator node' + this.groupSortInteractionModel = new GroupSortInteractionModel( { + getValueProperty: cardModel => cardModel.indexProperty, + tandem: providedOptions.tandem.createTandem( 'groupSortInteractionModel' ) } ); - this.hasDraggedCardProperty = new DerivedProperty( - [ this.totalDragDistanceProperty, this.hasKeyboardMovedCardProperty ], - ( totalDragDistance, hasKeyboardMovedCard ) => totalDragDistance > 15 || hasKeyboardMovedCard - ); - - this.dragIndicationCardProperty = new Property( null, { - phetioReadOnly: true, - phetioValueType: NullableIO( ReferenceIO( CardModel.CardModelIO ) ), - tandem: this.representationContext === 'accordion' ? providedOptions.tandem.createTandem( 'cardDragIndicatorProperty' ) : Tandem.OPT_OUT, - phetioDocumentation: 'This is for PhET-iO internal use only.' + this.totalDragDistanceProperty.link( totalDragDistance => { + this.groupSortInteractionModel.hasGroupItemBeenSortedProperty.value = totalDragDistance > 15 || + this.groupSortInteractionModel.hasGroupItemBeenSortedProperty.value; } ); - - this.isKeyboardDragArrowVisibleProperty = new DerivedProperty( [ this.focusedCardProperty, this.hasKeyboardMovedCardProperty, this.hasKeyboardGrabbedCardProperty, - this.isCardGrabbedProperty, this.isKeyboardFocusedProperty ], - ( focusedCard, hasKeyboardMovedCard, hasGrabbedCard, isCardGrabbed, hasKeyboardFocus ) => { - return focusedCard !== null && !hasKeyboardMovedCard && hasGrabbedCard && isCardGrabbed && hasKeyboardFocus; - } ); - - this.grabReleaseCueVisibleProperty = new DerivedProperty( [ this.focusedCardProperty, this.hasKeyboardGrabbedCardProperty, this.isKeyboardFocusedProperty ], - ( focusedCard, hasGrabbedCard, hasKeyboardFocus ) => { - return focusedCard !== null && !hasGrabbedCard && hasKeyboardFocus; - } ); this.cardCellsChangedEmitter.addListener( () => { medianModel.areCardsSortedProperty.value = this.isDataSorted(); } ); - const updateDragIndicationCardProperty = () => { - - const leftCard = this.getCardsInCellOrder()[ 0 ]; - const rightCard = this.getCardsInCellOrder()[ 1 ]; + const updateMouseSortCueNode = () => { - // If the user has not yet dragged a card and there are multiple cards showing, add the drag indicator to leftCard. - if ( !this.hasDraggedCardProperty.value && leftCard && rightCard ) { - this.dragIndicationCardProperty.value = leftCard; - } - + // If the user has not yet dragged a card and there are multiple cards showing, add the drag indicator. // If the user has dragged a card, then the drag indicator does not need to be shown. - if ( this.hasDraggedCardProperty.value || !leftCard || !rightCard ) { - this.dragIndicationCardProperty.value = null; - } + // TODO: not any interaction sorted, just for mouse, https://github.com/phetsims/center-and-variability/issues/605 + this.groupSortInteractionModel.mouseSortCueVisibleProperty.value = this.getCardsInCellOrder().length >= 2 && + !this.groupSortInteractionModel.hasGroupItemBeenSortedProperty.value && + !this.groupSortInteractionModel.isKeyboardFocusedProperty.value; }; - this.cardCellsChangedEmitter.addListener( updateDragIndicationCardProperty ); - this.hasDraggedCardProperty.link( updateDragIndicationCardProperty ); - this.cards.forEach( card => card.soccerBall.valueProperty.lazyLink( updateDragIndicationCardProperty ) ); + this.cardCellsChangedEmitter.addListener( updateMouseSortCueNode ); + this.groupSortInteractionModel.registerUpdateSortIndicatorNode( updateMouseSortCueNode ); + this.cards.forEach( card => card.soccerBall.valueProperty.lazyLink( updateMouseSortCueNode ) ); medianModel.selectedSceneModelProperty.value.resetEmitter.addListener( () => { this.totalDragDistanceProperty.reset(); - this.hasKeyboardMovedCardProperty.reset(); - this.hasKeyboardGrabbedCardProperty.reset(); - this.hasKeyboardSelectedDifferentCardProperty.reset(); + this.groupSortInteractionModel.reset(); } ); medianModel.selectedSceneModelProperty.value.preClearDataEmitter.addListener( () => { - this.focusedCardProperty.reset(); - this.isCardGrabbedProperty.reset(); - } - ); + this.groupSortInteractionModel.resetInteractionState(); + } ); this.manuallySortedEmitter = new Emitter( { tandem: providedOptions.tandem.createTandem( 'manuallySortedEmitter' ), Index: scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.ts b/scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.ts --- a/scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.ts (revision c909d03c4d0c7d9bdc59972ddf53bc9b71b1a01c) +++ b/scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.ts (date 1705531794676) @@ -43,9 +43,19 @@ // The available range for storing. This is the acceptable range for the valueProperty of ItemModel (see getValueProperty()). sortingRange: Range; + // Do the sort operation, allowing for custom actions, defaults to just updating the valueProperty of the selected + // group item to the new value. + sortGroupItem?: ( groupItem: ItemModel, newValue: number ) => void; + // Called when a group item is sorted. Note that this may not have changed its value. onSort?: ( groupItem: ItemModel, oldValue: number ) => void; + // When the selected group item is grabbed (into "sorting" mode). + onGrab?: ( groupItem: ItemModel ) => void; + + // When the selected group item is released (back into "selecting" mode). + onRelease?: ( groupItem: ItemModel ) => void; + // If provided, listen to the number keys as well. Provide the value that the number key maps to. A direct value, // not a delta. If set to null, then number keys will not be listened to for this interaction numberKeyMapper?: ( ( pressedKeys: string ) => ( number | null ) ) | null; @@ -66,12 +76,13 @@ export default class GroupSortInteractionView extends Disposable { // Update group highlight dynamically by setting the `shape` of this path. - protected readonly groupFocusHighlightPath: HighlightPath; + public readonly groupFocusHighlightPath: HighlightPath; // Emitted when the sorting cue should be repositioned. Most likely because the selection has changed. public readonly positionSortCueNodeEmitter = new Emitter(); private readonly getNodeFromModelItem: ( model: ItemModel ) => ItemNode | null; + private readonly sortGroupItem: ( groupItem: ItemModel, newValue: number ) => void; private readonly onSort: ( groupItem: ItemModel, oldValue: number ) => void; private readonly sortingRange: Range; private readonly sortStep: number; @@ -88,13 +99,19 @@ ParentOptions>()( { numberKeyMapper: null, onSort: _.noop, + onGrab: _.noop, + onRelease: _.noop, sortStep: 1, - pageSortStep: Math.ceil( providedOptions.sortingRange.getLength() / 5 ) + pageSortStep: Math.ceil( providedOptions.sortingRange.getLength() / 5 ), + sortGroupItem: ( groupItem, newValue ) => { + this.getValueProperty( groupItem ).value = newValue; + } }, providedOptions ); super( options ); this.getNodeFromModelItem = options.getNodeFromModelItem; + this.sortGroupItem = options.sortGroupItem; this.onSort = options.onSort; this.sortingRange = options.sortingRange; this.sortStep = options.sortStep; @@ -105,6 +122,22 @@ const isGroupItemKeyboardGrabbedProperty = this.groupSortInteractionModel.isGroupItemKeyboardGrabbedProperty; const hasKeyboardGrabbedGroupItemProperty = this.groupSortInteractionModel.hasKeyboardGrabbedGroupItemProperty; + const grabbedPropertyListener = ( grabbed: boolean ) => { + const selectedGroupItem = selectedGroupItemProperty.value; + if ( selectedGroupItem ) { + if ( grabbed ) { + options.onGrab( selectedGroupItem ); + } + else { + options.onRelease( selectedGroupItem ); + } + } + }; + isGroupItemKeyboardGrabbedProperty.lazyLink( grabbedPropertyListener ) + this.disposeEmitter.addListener( () => { + isGroupItemKeyboardGrabbedProperty.unlink( grabbedPropertyListener ) + } ); + const focusListener = { focus: () => { @@ -125,6 +158,8 @@ isGroupItemKeyboardGrabbedProperty.value = false; isKeyboardFocusedProperty.value = false; }, + + // TODO: this isn't part of InteractiveCardContainer? https://github.com/phetsims/scenery-phet/issues/815 over: () => { // TODO: MS!!!! this is awkward. In this situation: // 1. tab to populated node, the keyboard grab cue is shown. @@ -203,7 +238,8 @@ if ( selectedGroupItemProperty.value !== null ) { const groupItem = selectedGroupItemProperty.value; - const oldValue = this.getValueProperty( groupItem ).value!; + const valueProperty = this.getValueProperty( groupItem ); + const oldValue = valueProperty.value!; assert && assert( oldValue !== null, 'We should have a group item when responding to input?' ); // Sorting an item @@ -224,10 +260,12 @@ // TODO: DESIGN!!! This changes the behavior because now the WASD, page up/page down keys work // for the selection too - they don't on published version (Note that home and end DO work on published // version for selection), https://github.com/phetsims/scenery-phet/issues/815 - const delta = this.getDeltaForKey( keysPressed ); - if ( delta !== null ) { + const unclampedDelta = this.getDeltaForKey( keysPressed ); + if ( unclampedDelta !== null ) { this.groupSortInteractionModel.hasKeyboardSelectedDifferentGroupItemProperty.value = true; - selectedGroupItemProperty.value = options.getNextSelectedGroupItem( delta ); + + const clampedDelta = this.sortingRange.clampDelta( oldValue, unclampedDelta ); + selectedGroupItemProperty.value = options.getNextSelectedGroupItem( clampedDelta ); } } this.onGroupItemChange( groupItem ); @@ -263,7 +301,7 @@ } ); } - const defaultGroupShape = primaryFocusedNode.bounds.isFinite() ? Shape.bounds( primaryFocusedNode.visibleBounds ) : null; + const defaultGroupShape = primaryFocusedNode.visibleBounds.isFinite() ? Shape.bounds( primaryFocusedNode.visibleBounds ) : null; // Set the outer group focus highlight to surround the entire area where group items are located. this.groupFocusHighlightPath = new HighlightPath( defaultGroupShape, { @@ -305,7 +343,7 @@ private onSortedValue( groupItem: ItemModel, value: number, oldValue: number ): void { assert && assert( value !== null, 'We should have a value for the group item by the end of the listener.' ); - this.getValueProperty( groupItem ).value = this.sortingRange.constrainValue( value ); + this.sortGroupItem( groupItem, this.sortingRange.constrainValue( value ) ); // TODO: DESIGN!!! fire this even if the value didn't change? Yes likely, for the sound https://github.com/phetsims/scenery-phet/issues/815 this.onSort( groupItem, oldValue ); @@ -313,6 +351,10 @@ this.groupSortInteractionModel.hasGroupItemBeenSortedProperty.value = true; } + /** + * Get the delta to change the value given what key was pressed. The returned delta may not result in a value in range, + * please constrain value from range or provide your own defensive measures to this delta. + */ private getDeltaForKey( key: string ): number | null { const fullRange = this.sortingRange.getLength(); return key === 'home' ? -fullRange : Index: center-and-variability/js/median/view/InteractiveCardNodeContainer.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/center-and-variability/js/median/view/InteractiveCardNodeContainer.ts b/center-and-variability/js/median/view/InteractiveCardNodeContainer.ts --- a/center-and-variability/js/median/view/InteractiveCardNodeContainer.ts (revision f5c5c53f59fd83cf9934d42fae62872377cb92f4) +++ b/center-and-variability/js/median/view/InteractiveCardNodeContainer.ts (date 1705532403020) @@ -24,8 +24,9 @@ import InteractiveCardContainerModel from '../model/InteractiveCardContainerModel.js'; import Property from '../../../../axon/js/Property.js'; import CAVSoccerSceneModel from '../../common/model/CAVSoccerSceneModel.js'; -import { animatedPanZoomSingleton, HighlightFromNode, HighlightPath, KeyboardListener, NodeTranslationOptions, Path } from '../../../../scenery/js/imports.js'; +import { animatedPanZoomSingleton, Node, NodeTranslationOptions } from '../../../../scenery/js/imports.js'; import Vector2 from '../../../../dot/js/Vector2.js'; +import Range from '../../../../dot/js/Range.js'; import CAVConstants from '../../common/CAVConstants.js'; import Bounds2 from '../../../../dot/js/Bounds2.js'; import CardNode, { cardDropClip, cardPickUpSoundClip, PICK_UP_DELTA_X } from './CardNode.js'; @@ -34,7 +35,6 @@ import { Shape } from '../../../../kite/js/imports.js'; import Multilink from '../../../../axon/js/Multilink.js'; import isSettingPhetioStateProperty from '../../../../tandem/js/isSettingPhetioStateProperty.js'; -import GrabReleaseCueNode from '../../../../scenery-phet/js/accessibility/nodes/GrabReleaseCueNode.js'; import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; import CelebrationNode from './CelebrationNode.js'; import checkboxCheckedSoundPlayer from '../../../../tambo/js/shared-sound-players/checkboxCheckedSoundPlayer.js'; @@ -43,6 +43,7 @@ import CardDragIndicatorNode from './CardDragIndicatorNode.js'; import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js'; import GroupSortInteractionView from '../../../../scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.js'; +import CardModel from '../model/CardModel.js'; const FOCUS_HIGHLIGHT_Y_MARGIN = CAVConstants.CARD_SPACING + 3; @@ -61,6 +62,7 @@ // The message that appears when the cards are sorted private readonly celebrationNode: CelebrationNode; + private readonly groupSortInteractionView: GroupSortInteractionView; public constructor( model: InteractiveCardContainerModel, private readonly isSortingDataProperty: Property, @@ -78,6 +80,47 @@ this.celebrationNode = new CelebrationNode( model, this.cardMap, this.sceneModel.resetEmitter ); this.addChild( this.celebrationNode ); + this.groupSortInteractionView = new GroupSortInteractionView( model.groupSortInteractionModel, this, { + getNextSelectedGroupItem: unclampedDelta => { + // TODO: we have to do this every time, perhaps a parameter? https://github.com/phetsims/center-and-variability/issues/605 + const selectedCardModel = model.groupSortInteractionModel.selectedGroupItemProperty.value!; + const currentIndex = selectedCardModel.indexProperty.value!; + assert && assert( currentIndex !== null, 'need an index to be sorted' ); + + // TODO: until range is dynamic, this could be outside of current cards https://github.com/phetsims/center-and-variability/issues/605 + const delta = new Range( 0, this.getActiveCardNodesInOrder().length - 1 ).clampDelta( currentIndex, unclampedDelta ); + const newIndex = currentIndex + delta; + const cardNodes = this.getActiveCardNodesInOrder(); + const newCardNode = cardNodes[ newIndex ]; + assert && assert( newCardNode, 'wrong index for available cards I believe' ); + return newCardNode.model; + }, + onGrab: groupItem => { + groupItem.isDraggingProperty.value = true; + }, + onRelease: groupItem => { + groupItem.isDraggingProperty.value = false; + }, + sortGroupItem: ( selectedCardModel, newValue ) => { + assert && assert( selectedCardModel.indexProperty.value !== null, 'need an index to be sorted' ); + const delta = newValue - selectedCardModel.indexProperty.value!; + swapCards( this.getActiveCardNodesInOrder(), this.cardMap.get( selectedCardModel )!, delta ); + }, + onSort: () => { + + // See if the user unsorted the data. If so, uncheck the "Sort Data" checkbox + if ( this.isSortingDataProperty.value && !this.model.isDataSorted() ) { + this.isSortingDataProperty.value = false; + } + }, + getGroupItemToSelect: () => { + const activeCards = this.getActiveCardNodesInOrder(); + return activeCards[ 0 ] ? activeCards[ 0 ].model : null; + }, + getNodeFromModelItem: cardModel => this.cardMap.get( cardModel ) || null, + sortingRange: new Range( 0, this.model.cards.length - 1 ) // TODO: Need to support Property(Range) https://github.com/phetsims/center-and-variability/issues/605 + } ); + this.cardMap.forEach( ( cardNode, cardModel ) => { // Update the position of all cards (via animation) whenever any card is dragged cardNode.model.positionProperty.link( this.createDragPositionListener( cardNode ) ); @@ -107,32 +150,20 @@ } ); } ); - const focusHighlightPath = new HighlightPath( null, { - outerStroke: HighlightPath.OUTER_LIGHT_GROUP_FOCUS_COLOR, - innerStroke: HighlightPath.INNER_LIGHT_GROUP_FOCUS_COLOR, - outerLineWidth: HighlightPath.GROUP_OUTER_LINE_WIDTH, - innerLineWidth: HighlightPath.GROUP_INNER_LINE_WIDTH - } ); - - const grabReleaseCueNode = new GrabReleaseCueNode( { - top: CAVConstants.CARD_DIMENSION + FOCUS_HIGHLIGHT_Y_MARGIN + 15, - visibleProperty: model.grabReleaseCueVisibleProperty - } ); - - const keyboardSortCueNode = GroupSortInteractionView.createSortCueNode( model.isKeyboardDragArrowVisibleProperty ); + const keyboardSortCueNode = GroupSortInteractionView.createSortCueNode( model.groupSortInteractionModel.keyboardSortCueVisibleProperty ); this.addChild( keyboardSortCueNode ); const cardDragIndicatorNode = new CardDragIndicatorNode( { centerTop: new Vector2( 0.5 * CAVConstants.CARD_DIMENSION - PICK_UP_DELTA_X, CAVConstants.CARD_DIMENSION - 10 ), visibleProperty: new DerivedProperty( - [ this.inputEnabledProperty, model.isKeyboardFocusedProperty, model.dragIndicationCardProperty ], - ( inputEnabled, hasKeyboardFocus, dragIndicationCard ) => inputEnabled && !hasKeyboardFocus && !!dragIndicationCard ) + [ this.inputEnabledProperty, model.groupSortInteractionModel.mouseSortCueVisibleProperty ], + ( inputEnabled, mouseSortCueVisible ) => inputEnabled && mouseSortCueVisible ) } ); this.addChild( cardDragIndicatorNode ); - model.dragIndicationCardProperty.lazyLink( ( newCard, oldCard ) => { + model.groupSortInteractionModel.selectedGroupItemProperty.lazyLink( ( newCard, oldCard ) => { if ( oldCard ) { const oldCardNode = this.cardMap.get( oldCard )!; @@ -176,7 +207,7 @@ // Needs to be pickable in accordion box. this.pickable = true; - const focusedCardNodeProperty: TReadOnlyProperty = new DerivedProperty( [ model.focusedCardProperty ], focusedCard => { + const focusedCardNodeProperty: TReadOnlyProperty = new DerivedProperty( [ model.groupSortInteractionModel.selectedGroupItemProperty ], focusedCard => { return focusedCard === null ? focusedCard : this.cardMap.get( focusedCard )!; } ); @@ -187,77 +218,56 @@ // When a user is focused on the card container but there are no cards yet, we want to ensure that a card gets focused // once there is a card. - if ( model.focusedCardProperty.value === null && this.focused && model.getActiveCards().length === 1 ) { - model.focusedCardProperty.value = activeCardNodes[ 0 ].model; + // TODO: this.focused should use the model https://github.com/phetsims/center-and-variability/issues/605 + if ( model.groupSortInteractionModel.selectedGroupItemProperty.value === null && this.focused && model.getActiveCards().length === 1 ) { + model.groupSortInteractionModel.selectedGroupItemProperty.value = activeCardNodes[ 0 ].model; } - // If the card cells changed, and we have no more active cards left, that means that all the cards were removed. + // If the card cells changed, and we have no more active cards left, that means that all the cards were removed. // Therefore, we want to set the focused card to null. else if ( model.getActiveCards().length === 0 ) { - model.focusedCardProperty.value = null; - } - } ); - - this.addInputListener( { - focus: () => { - const activeCardNodes = this.getActiveCardNodesInOrder(); - if ( model.focusedCardProperty.value === null && activeCardNodes.length > 0 ) { - model.focusedCardProperty.value = activeCardNodes[ 0 ].model; - } - - // When the group receives keyboard focus, make sure that the focused card is displayed - if ( focusedCardNodeProperty.value ) { - animatedPanZoomSingleton.listener.panToNode( focusedCardNodeProperty.value, true ); - } - model.isKeyboardFocusedProperty.value = true; - }, - blur: () => { - model.isCardGrabbedProperty.value = false; - model.isKeyboardFocusedProperty.value = false; + model.groupSortInteractionModel.selectedGroupItemProperty.value = null; } } ); // When pdomFocusHighlightsVisibleProperty become false, interaction with a mouse has begun while using // Interactive Highlighting. When that happens, clear the sim-specific state tracking 'focused' cards. + // TODO: wat? https://github.com/phetsims/center-and-variability/issues/605 phet.joist.sim.display.focusManager.pdomFocusHighlightsVisibleProperty.link( ( visible: boolean ) => { if ( !visible ) { - if ( model.focusedCardProperty.value !== null ) { + if ( model.groupSortInteractionModel.selectedGroupItemProperty.value !== null ) { // Before clearing out the focusedCardProperty the CardModel must be cleared out of it's // dragging state. - model.focusedCardProperty.value.isDraggingProperty.set( false ); + model.groupSortInteractionModel.selectedGroupItemProperty.value.isDraggingProperty.set( false ); // Clear the 'focused' card so that there isn't a flicker to a highlight around that card when // moving between the CardNode interactive highlight and the container group highlight (which has // a custom highlight around the focused card). - model.focusedCardProperty.set( null ); + model.groupSortInteractionModel.selectedGroupItemProperty.value = null; } - model.isCardGrabbedProperty.value = false; + model.groupSortInteractionModel.isGroupItemKeyboardGrabbedProperty.value = false; // This controls the visibility of interaction cues (keyboard vs mouse), so we need to clear it when // switching interaction modes. - model.isKeyboardFocusedProperty.value = false; + // TODO: move to common code? https://github.com/phetsims/center-and-variability/issues/605 + model.groupSortInteractionModel.isKeyboardFocusedProperty.value = false; } } ); - Multilink.multilink( [ focusedCardNodeProperty, model.isCardGrabbedProperty ], ( focusedCardNode, isCardGrabbed ) => { + Multilink.multilink( [ focusedCardNodeProperty, model.groupSortInteractionModel.isGroupItemKeyboardGrabbedProperty ], + ( focusedCardNode, isCardGrabbed ) => { if ( focusedCardNode ) { - const focusForSelectedCard = new HighlightFromNode( focusedCardNode.cardNode, { dashed: isCardGrabbed } ); - this.setFocusHighlight( focusForSelectedCard ); - focusedCardNode.model.isDraggingProperty.value = isCardGrabbed; - keyboardSortCueNode.centerBottom = new Vector2( focusedCardNode.centerX + CARD_LAYER_OFFSET + PICK_UP_DELTA_X / 2 + 1, focusForSelectedCard.bottom + 11 ); - } - else { - this.setFocusHighlight( 'invisible' ); + keyboardSortCueNode.centerBottom = new Vector2( focusedCardNode.centerX + CARD_LAYER_OFFSET + PICK_UP_DELTA_X / 2 + 1, + ( this.focusHighlight as unknown as Node ).bottom + 11 ); } - } - ); + } ); - model.isCardGrabbedProperty.link( isCardGrabbed => { + model.groupSortInteractionModel.isGroupItemKeyboardGrabbedProperty.link( isCardGrabbed => { if ( isCardGrabbed ) { this.wasSortedBefore = model.isDataSorted(); } @@ -278,6 +288,7 @@ // Move and swap cards according to the focused card's target index. Used for alternative input. const swapCards = ( activeCards: CardNode[], focusedCard: CardNode, delta: number ) => { const currentIndex = activeCards.indexOf( focusedCard ); + assert && assert( focusedCard.model.indexProperty.value === currentIndex, 'sanity check' ); const targetIndex = Utils.clamp( currentIndex + delta, 0, activeCards.length - 1 ); if ( targetIndex !== currentIndex ) { @@ -301,100 +312,28 @@ animatedPanZoomSingleton.listener.panToNode( focusedCard, true ); model.cardCellsChangedEmitter.emit(); - - // Gets rid of the hand icon - model.hasKeyboardMovedCardProperty.value = true; } }; - const keyboardListener = new KeyboardListener( { - fireOnHold: true, - keys: [ 'd', 'a', 'arrowRight', 'arrowLeft', 'w', 's', 'arrowUp', 'arrowDown', 'enter', 'space', 'home', 'end', 'escape', 'pageUp', 'pageDown' ], - callback: ( event, keysPressed ) => { - - const focusedCardNode = focusedCardNodeProperty.value; - const activeCardNodes = this.getActiveCardNodesInOrder(); - const numberOfActiveCards = activeCardNodes.length; - const isCardGrabbed = model.isCardGrabbedProperty.value; - - // If there are no active cards no card can be focused and no keyboard input is allowed. - if ( numberOfActiveCards === 0 ) { - model.focusedCardProperty.value = null; - return; - } - - if ( focusedCardNode ) { - const delta = this.getKeystrokeDelta( keysPressed, numberOfActiveCards ); - - if ( [ 'enter', 'space' ].includes( keysPressed ) ) { - model.isCardGrabbedProperty.value = !model.isCardGrabbedProperty.value; - model.hasKeyboardGrabbedCardProperty.value = true; - - // See if the user unsorted the data. If so, uncheck the "Sort Data" checkbox - if ( !model.isCardGrabbedProperty.value && this.isSortingDataProperty.value && !this.model.isDataSorted() ) { - this.isSortingDataProperty.value = false; - } - } - else if ( isCardGrabbed ) { - if ( keysPressed === 'escape' ) { - model.isCardGrabbedProperty.value = false; - } - - // If we have a nonNull delta and the card is grabbed we want to swap card positions. - else if ( delta !== null ) { - swapCards( activeCardNodes, focusedCardNode, delta ); - } - } - - // If we have a nonNull delta and the card is not grabbed we want to shift focus to a different card. - else if ( delta !== null ) { - // Shift the card focus when a card is not grabbed. - const currentIndex = activeCardNodes.indexOf( focusedCardNode ); - const nextIndex = Utils.clamp( currentIndex + delta, 0, numberOfActiveCards - 1 ); - model.focusedCardProperty.value = activeCardNodes[ nextIndex ].model; - model.hasKeyboardSelectedDifferentCardProperty.value = true; - animatedPanZoomSingleton.listener.panToNode( focusedCardNode, true ); - } - else { + const grabReleaseCueNode = GroupSortInteractionView.createGrabReleaseCueNode( model.groupSortInteractionModel.grabReleaseCueVisibleProperty, { + top: CAVConstants.CARD_DIMENSION + FOCUS_HIGHLIGHT_Y_MARGIN + 15 + } ); - // No cards are grabbed! We cleared the 'focused' card because we were using mouse input - start over with - // keyboard interaction and focus the first card AND make sure that all cards individually are no longer - // dragging. - this.cardNodes.forEach( cardNode => { - cardNode.model.isDraggingProperty.value = false; - } ); - model.isCardGrabbedProperty.value = false; + // TODO: MS! Discuss this as a design patter vs. handling visibility manually, https://github.com/phetsims/center-and-variability/issues/605 + this.groupSortInteractionView.groupFocusHighlightPath.addChild( grabReleaseCueNode ); - model.focusedCardProperty.value = activeCardNodes[ 0 ].model; - } - } - else { - model.focusedCardProperty.value = activeCardNodes[ 0 ].model; - } - } - } ); const focusHighlightWidthProperty = new DerivedProperty( [ model.numActiveCardsProperty ], numActiveCards => { return model.getCardPositionX( numActiveCards === 0 ? 1 : numActiveCards ); } ); - focusHighlightPath.addChild( grabReleaseCueNode ); - - const highlightRectangle = new Path( null ); - this.addChild( highlightRectangle ); - highlightRectangle.moveToBack(); - + const marginX = 7; focusHighlightWidthProperty.link( focusHighlightWidth => { - const marginX = 7; const focusRect = Shape.rect( -marginX, -FOCUS_HIGHLIGHT_Y_MARGIN, focusHighlightWidth + 2 * marginX, CAVConstants.CARD_DIMENSION + 2 * FOCUS_HIGHLIGHT_Y_MARGIN + 9 ); - focusHighlightPath.setShape( focusRect ); - highlightRectangle.setShape( focusRect ); + this.groupSortInteractionView.groupFocusHighlightPath.setShape( focusRect ); const cueNodeWidth = grabReleaseCueNode.width; - grabReleaseCueNode.centerX = Utils.clamp( focusRect.bounds.centerX, cueNodeWidth / 2, this.width - cueNodeWidth / 2 ); + grabReleaseCueNode.centerX = Utils.clamp( focusRect.bounds.centerX, cueNodeWidth / 2, Math.max( this.width - cueNodeWidth / 2, cueNodeWidth ) ); } ); - - this.setGroupFocusHighlight( focusHighlightPath ); - this.addInputListener( keyboardListener ); } // The listener which is linked to the cardNode.positionProperty @@ -441,19 +380,6 @@ } }; } - - private getKeystrokeDelta( keysPressed: string, numberOfActiveCards: number ): number | null { - if ( [ 'arrowRight', 'arrowLeft', 'a', 'd', 'arrowUp', 'arrowDown', 'w', 's' ].includes( keysPressed ) ) { - return [ 'arrowRight', 'd', 'arrowUp', 'w' ].includes( keysPressed ) ? 1 : -1; - } - else if ( [ 'pageUp', 'pageDown' ].includes( keysPressed ) ) { - return keysPressed === 'pageUp' ? 3 : -3; - } - else if ( [ 'home', 'end' ].includes( keysPressed ) ) { - return keysPressed === 'end' ? numberOfActiveCards : -numberOfActiveCards; - } - return null; - } } centerAndVariability.register( 'InteractiveCardNodeContainer', InteractiveCardNodeContainer ); \ No newline at end of file Index: scenery-phet/js/accessibility/group-sort/model/GroupSortInteractionModel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/scenery-phet/js/accessibility/group-sort/model/GroupSortInteractionModel.ts b/scenery-phet/js/accessibility/group-sort/model/GroupSortInteractionModel.ts --- a/scenery-phet/js/accessibility/group-sort/model/GroupSortInteractionModel.ts (revision c909d03c4d0c7d9bdc59972ddf53bc9b71b1a01c) +++ b/scenery-phet/js/accessibility/group-sort/model/GroupSortInteractionModel.ts (date 1705529838864) @@ -96,6 +96,8 @@ // Whether any group item has ever been sorted to a new value, even if not by the group sort interaction. For best results, // set this to true from other interactions too (like mouse/touch). + // TODO: DESIGN?!?!? this should be derived, and a new Property for just mouse created. Basically how important is + // it to have a mouse drag indicator after successful keyboard sort (but no successful mouse) https://github.com/phetsims/scenery-phet/issues/815 public readonly hasGroupItemBeenSortedProperty: Property; public readonly getValueProperty: ( itemModel: ItemModel ) => TProperty; @@ -132,8 +134,8 @@ this.hasKeyboardGrabbedGroupItemProperty, this.isKeyboardFocusedProperty, this.enabledProperty - ], ( selectedGroupItem, hasGrabbedBall, hasKeyboardFocus, enabled ) => { - return selectedGroupItem !== null && !hasGrabbedBall && hasKeyboardFocus && enabled; + ], ( selectedGroupItem, hasGrabbedGroupItem, hasKeyboardFocus, enabled ) => { + return selectedGroupItem !== null && !hasGrabbedGroupItem && hasKeyboardFocus && enabled; } ); this.keyboardSortCueVisibleProperty = new DerivedProperty( [ @@ -173,8 +175,13 @@ public registerUpdateSortIndicatorNode( updateSortIndicatorNode: () => void ): void { this.mouseSortCueVisibleProperty.link( updateSortIndicatorNode ); this.selectedGroupItemProperty.link( updateSortIndicatorNode ); + this.hasGroupItemBeenSortedProperty.link( updateSortIndicatorNode ); + this.isKeyboardFocusedProperty.link( updateSortIndicatorNode ); + this.disposeEmitter.addListener( () => { + this.isKeyboardFocusedProperty.unlink( updateSortIndicatorNode ); + this.hasGroupItemBeenSortedProperty.unlink( updateSortIndicatorNode ); this.mouseSortCueVisibleProperty.unlink( updateSortIndicatorNode ); this.selectedGroupItemProperty.unlink( updateSortIndicatorNode ); } );

[zepumph]

I was about to commit this patch, but then I realized I need to update the PhET-iO API and migration rules. I'll wait.

```diff Subject: [PATCH] Use GroupSortInteraction for InteractiveCardContainer, https://github.com/phetsims/center-and-variability/issues/605 --- Index: center-and-variability/js/median/model/InteractiveCardContainerModel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/center-and-variability/js/median/model/InteractiveCardContainerModel.ts b/center-and-variability/js/median/model/InteractiveCardContainerModel.ts --- a/center-and-variability/js/median/model/InteractiveCardContainerModel.ts (revision f5c5c53f59fd83cf9934d42fae62872377cb92f4) +++ b/center-and-variability/js/median/model/InteractiveCardContainerModel.ts (date 1705533202021) @@ -21,20 +21,17 @@ import centerAndVariability from '../../centerAndVariability.js'; import MedianModel from './MedianModel.js'; import { EmptySelfOptions } from '../../../../phet-core/js/optionize.js'; -import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; import Property from '../../../../axon/js/Property.js'; import CardModel from './CardModel.js'; import BooleanProperty from '../../../../axon/js/BooleanProperty.js'; import NumberProperty from '../../../../axon/js/NumberProperty.js'; import DerivedProperty from '../../../../axon/js/DerivedProperty.js'; -import NullableIO from '../../../../tandem/js/types/NullableIO.js'; -import ReferenceIO from '../../../../tandem/js/types/ReferenceIO.js'; -import Tandem from '../../../../tandem/js/Tandem.js'; import dotRandom from '../../../../dot/js/dotRandom.js'; import CAVQueryParameters from '../../common/CAVQueryParameters.js'; import Emitter from '../../../../axon/js/Emitter.js'; import isResettingProperty from '../../../../soccer-common/js/model/isResettingProperty.js'; import isSettingPhetioStateProperty from '../../../../tandem/js/isSettingPhetioStateProperty.js'; +import GroupSortInteractionModel from '../../../../scenery-phet/js/accessibility/group-sort/model/GroupSortInteractionModel.js'; const cardMovementSounds = [ cardMovement1_mp3, @@ -74,34 +71,12 @@ // For sonification, order the active, non-displaced cards appeared in the last step private lastStepOrder: CardModel[] = []; - // Indicates whether the user has ever dragged a card, used to determine dragIndicationCardProperty. - public readonly hasDraggedCardProperty: TReadOnlyProperty; - - // The card where the drag indicator should appear. null if no drag indicator should appear. - public readonly dragIndicationCardProperty: Property; - public readonly totalDragDistanceProperty: Property; - // KEYBOARD INPUT PROPERTIES: - // The card which currently has focus. Is part of what controls highlight visibility among other things. - public readonly focusedCardProperty: Property = new Property( null ); - - // Tracks when a card is currently grabbed via keyboard input. - public readonly isCardGrabbedProperty = new BooleanProperty( false ); - - // Visible Properties for keyboard hints - public readonly grabReleaseCueVisibleProperty: TReadOnlyProperty; - public readonly isKeyboardDragArrowVisibleProperty: TReadOnlyProperty; - - // Properties that track if a certain action has ever been performed via keyboard input. - public readonly hasKeyboardMovedCardProperty: Property; - public readonly hasKeyboardGrabbedCardProperty = new BooleanProperty( false ); - public readonly hasKeyboardSelectedDifferentCardProperty = new BooleanProperty( false ); - - // Property that is triggered via focus and blur events in the InteractiveCardNodeContainer - public readonly isKeyboardFocusedProperty = new BooleanProperty( false ); public readonly manuallySortedEmitter: Emitter; + public readonly groupSortInteractionModel: GroupSortInteractionModel; + public constructor( medianModel: MedianModel, providedOptions: InteractiveCardContainerModelOptions ) { super( medianModel, providedOptions ); @@ -111,71 +86,42 @@ phetioDocumentation: 'Accumulated card drag distance, for purposes of hiding the drag indicator node' } ); - this.hasKeyboardMovedCardProperty = new BooleanProperty( false, { - tandem: providedOptions.tandem.createTandem( 'hasKeyboardMovedCardProperty' ), - phetioReadOnly: true, // controlled by the sim - phetioDocumentation: 'Whether a card been moved using the keyboard, for purposes of hiding the drag indicator node' + this.groupSortInteractionModel = new GroupSortInteractionModel( { + getValueProperty: cardModel => cardModel.indexProperty, + tandem: providedOptions.tandem.createTandem( 'groupSortInteractionModel' ) } ); - this.hasDraggedCardProperty = new DerivedProperty( - [ this.totalDragDistanceProperty, this.hasKeyboardMovedCardProperty ], - ( totalDragDistance, hasKeyboardMovedCard ) => totalDragDistance > 15 || hasKeyboardMovedCard - ); - - this.dragIndicationCardProperty = new Property( null, { - phetioReadOnly: true, - phetioValueType: NullableIO( ReferenceIO( CardModel.CardModelIO ) ), - tandem: this.representationContext === 'accordion' ? providedOptions.tandem.createTandem( 'cardDragIndicatorProperty' ) : Tandem.OPT_OUT, - phetioDocumentation: 'This is for PhET-iO internal use only.' + this.totalDragDistanceProperty.link( totalDragDistance => { + this.groupSortInteractionModel.hasGroupItemBeenSortedProperty.value = totalDragDistance > 15 || + this.groupSortInteractionModel.hasGroupItemBeenSortedProperty.value; } ); - - this.isKeyboardDragArrowVisibleProperty = new DerivedProperty( [ this.focusedCardProperty, this.hasKeyboardMovedCardProperty, this.hasKeyboardGrabbedCardProperty, - this.isCardGrabbedProperty, this.isKeyboardFocusedProperty ], - ( focusedCard, hasKeyboardMovedCard, hasGrabbedCard, isCardGrabbed, hasKeyboardFocus ) => { - return focusedCard !== null && !hasKeyboardMovedCard && hasGrabbedCard && isCardGrabbed && hasKeyboardFocus; - } ); - - this.grabReleaseCueVisibleProperty = new DerivedProperty( [ this.focusedCardProperty, this.hasKeyboardGrabbedCardProperty, this.isKeyboardFocusedProperty ], - ( focusedCard, hasGrabbedCard, hasKeyboardFocus ) => { - return focusedCard !== null && !hasGrabbedCard && hasKeyboardFocus; - } ); this.cardCellsChangedEmitter.addListener( () => { medianModel.areCardsSortedProperty.value = this.isDataSorted(); } ); - const updateDragIndicationCardProperty = () => { - - const leftCard = this.getCardsInCellOrder()[ 0 ]; - const rightCard = this.getCardsInCellOrder()[ 1 ]; + const updateMouseSortCueNode = () => { - // If the user has not yet dragged a card and there are multiple cards showing, add the drag indicator to leftCard. - if ( !this.hasDraggedCardProperty.value && leftCard && rightCard ) { - this.dragIndicationCardProperty.value = leftCard; - } - + // If the user has not yet dragged a card and there are multiple cards showing, add the drag indicator. // If the user has dragged a card, then the drag indicator does not need to be shown. - if ( this.hasDraggedCardProperty.value || !leftCard || !rightCard ) { - this.dragIndicationCardProperty.value = null; - } + // TODO: DESIGN! not any interaction sorted, just for mouse, https://github.com/phetsims/center-and-variability/issues/605 + this.groupSortInteractionModel.mouseSortCueVisibleProperty.value = this.getCardsInCellOrder().length >= 2 && + !this.groupSortInteractionModel.hasGroupItemBeenSortedProperty.value && + !this.groupSortInteractionModel.isKeyboardFocusedProperty.value; }; - this.cardCellsChangedEmitter.addListener( updateDragIndicationCardProperty ); - this.hasDraggedCardProperty.link( updateDragIndicationCardProperty ); - this.cards.forEach( card => card.soccerBall.valueProperty.lazyLink( updateDragIndicationCardProperty ) ); + this.cardCellsChangedEmitter.addListener( updateMouseSortCueNode ); + this.groupSortInteractionModel.registerUpdateSortIndicatorNode( updateMouseSortCueNode ); + this.cards.forEach( card => card.soccerBall.valueProperty.lazyLink( updateMouseSortCueNode ) ); medianModel.selectedSceneModelProperty.value.resetEmitter.addListener( () => { this.totalDragDistanceProperty.reset(); - this.hasKeyboardMovedCardProperty.reset(); - this.hasKeyboardGrabbedCardProperty.reset(); - this.hasKeyboardSelectedDifferentCardProperty.reset(); + this.groupSortInteractionModel.reset(); } ); medianModel.selectedSceneModelProperty.value.preClearDataEmitter.addListener( () => { - this.focusedCardProperty.reset(); - this.isCardGrabbedProperty.reset(); - } - ); + this.groupSortInteractionModel.resetInteractionState(); + } ); this.manuallySortedEmitter = new Emitter( { tandem: providedOptions.tandem.createTandem( 'manuallySortedEmitter' ), Index: scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.ts b/scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.ts --- a/scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.ts (revision c909d03c4d0c7d9bdc59972ddf53bc9b71b1a01c) +++ b/scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.ts (date 1705533202013) @@ -43,9 +43,19 @@ // The available range for storing. This is the acceptable range for the valueProperty of ItemModel (see getValueProperty()). sortingRange: Range; + // Do the sort operation, allowing for custom actions, defaults to just updating the valueProperty of the selected + // group item to the new value. + sortGroupItem?: ( groupItem: ItemModel, newValue: number ) => void; + // Called when a group item is sorted. Note that this may not have changed its value. onSort?: ( groupItem: ItemModel, oldValue: number ) => void; + // When the selected group item is grabbed (into "sorting" mode). + onGrab?: ( groupItem: ItemModel ) => void; + + // When the selected group item is released (back into "selecting" mode). + onRelease?: ( groupItem: ItemModel ) => void; + // If provided, listen to the number keys as well. Provide the value that the number key maps to. A direct value, // not a delta. If set to null, then number keys will not be listened to for this interaction numberKeyMapper?: ( ( pressedKeys: string ) => ( number | null ) ) | null; @@ -66,12 +76,13 @@ export default class GroupSortInteractionView extends Disposable { // Update group highlight dynamically by setting the `shape` of this path. - protected readonly groupFocusHighlightPath: HighlightPath; + public readonly groupFocusHighlightPath: HighlightPath; // Emitted when the sorting cue should be repositioned. Most likely because the selection has changed. public readonly positionSortCueNodeEmitter = new Emitter(); private readonly getNodeFromModelItem: ( model: ItemModel ) => ItemNode | null; + private readonly sortGroupItem: ( groupItem: ItemModel, newValue: number ) => void; private readonly onSort: ( groupItem: ItemModel, oldValue: number ) => void; private readonly sortingRange: Range; private readonly sortStep: number; @@ -88,13 +99,19 @@ ParentOptions>()( { numberKeyMapper: null, onSort: _.noop, + onGrab: _.noop, + onRelease: _.noop, sortStep: 1, - pageSortStep: Math.ceil( providedOptions.sortingRange.getLength() / 5 ) + pageSortStep: Math.ceil( providedOptions.sortingRange.getLength() / 5 ), + sortGroupItem: ( groupItem, newValue ) => { + this.getValueProperty( groupItem ).value = newValue; + } }, providedOptions ); super( options ); this.getNodeFromModelItem = options.getNodeFromModelItem; + this.sortGroupItem = options.sortGroupItem; this.onSort = options.onSort; this.sortingRange = options.sortingRange; this.sortStep = options.sortStep; @@ -105,6 +122,22 @@ const isGroupItemKeyboardGrabbedProperty = this.groupSortInteractionModel.isGroupItemKeyboardGrabbedProperty; const hasKeyboardGrabbedGroupItemProperty = this.groupSortInteractionModel.hasKeyboardGrabbedGroupItemProperty; + const grabbedPropertyListener = ( grabbed: boolean ) => { + const selectedGroupItem = selectedGroupItemProperty.value; + if ( selectedGroupItem ) { + if ( grabbed ) { + options.onGrab( selectedGroupItem ); + } + else { + options.onRelease( selectedGroupItem ); + } + } + }; + isGroupItemKeyboardGrabbedProperty.lazyLink( grabbedPropertyListener ); + this.disposeEmitter.addListener( () => { + isGroupItemKeyboardGrabbedProperty.unlink( grabbedPropertyListener ); + } ); + const focusListener = { focus: () => { @@ -125,6 +158,8 @@ isGroupItemKeyboardGrabbedProperty.value = false; isKeyboardFocusedProperty.value = false; }, + + // TODO: this isn't part of InteractiveCardContainer? https://github.com/phetsims/scenery-phet/issues/815 over: () => { // TODO: MS!!!! this is awkward. In this situation: // 1. tab to populated node, the keyboard grab cue is shown. @@ -203,7 +238,8 @@ if ( selectedGroupItemProperty.value !== null ) { const groupItem = selectedGroupItemProperty.value; - const oldValue = this.getValueProperty( groupItem ).value!; + const valueProperty = this.getValueProperty( groupItem ); + const oldValue = valueProperty.value!; assert && assert( oldValue !== null, 'We should have a group item when responding to input?' ); // Sorting an item @@ -224,10 +260,12 @@ // TODO: DESIGN!!! This changes the behavior because now the WASD, page up/page down keys work // for the selection too - they don't on published version (Note that home and end DO work on published // version for selection), https://github.com/phetsims/scenery-phet/issues/815 - const delta = this.getDeltaForKey( keysPressed ); - if ( delta !== null ) { + const unclampedDelta = this.getDeltaForKey( keysPressed ); + if ( unclampedDelta !== null ) { this.groupSortInteractionModel.hasKeyboardSelectedDifferentGroupItemProperty.value = true; - selectedGroupItemProperty.value = options.getNextSelectedGroupItem( delta ); + + const clampedDelta = this.sortingRange.clampDelta( oldValue, unclampedDelta ); + selectedGroupItemProperty.value = options.getNextSelectedGroupItem( clampedDelta ); } } this.onGroupItemChange( groupItem ); @@ -263,7 +301,7 @@ } ); } - const defaultGroupShape = primaryFocusedNode.bounds.isFinite() ? Shape.bounds( primaryFocusedNode.visibleBounds ) : null; + const defaultGroupShape = primaryFocusedNode.visibleBounds.isFinite() ? Shape.bounds( primaryFocusedNode.visibleBounds ) : null; // Set the outer group focus highlight to surround the entire area where group items are located. this.groupFocusHighlightPath = new HighlightPath( defaultGroupShape, { @@ -305,7 +343,7 @@ private onSortedValue( groupItem: ItemModel, value: number, oldValue: number ): void { assert && assert( value !== null, 'We should have a value for the group item by the end of the listener.' ); - this.getValueProperty( groupItem ).value = this.sortingRange.constrainValue( value ); + this.sortGroupItem( groupItem, this.sortingRange.constrainValue( value ) ); // TODO: DESIGN!!! fire this even if the value didn't change? Yes likely, for the sound https://github.com/phetsims/scenery-phet/issues/815 this.onSort( groupItem, oldValue ); @@ -313,6 +351,10 @@ this.groupSortInteractionModel.hasGroupItemBeenSortedProperty.value = true; } + /** + * Get the delta to change the value given what key was pressed. The returned delta may not result in a value in range, + * please constrain value from range or provide your own defensive measures to this delta. + */ private getDeltaForKey( key: string ): number | null { const fullRange = this.sortingRange.getLength(); return key === 'home' ? -fullRange : Index: center-and-variability/js/median/view/InteractiveCardNodeContainer.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/center-and-variability/js/median/view/InteractiveCardNodeContainer.ts b/center-and-variability/js/median/view/InteractiveCardNodeContainer.ts --- a/center-and-variability/js/median/view/InteractiveCardNodeContainer.ts (revision f5c5c53f59fd83cf9934d42fae62872377cb92f4) +++ b/center-and-variability/js/median/view/InteractiveCardNodeContainer.ts (date 1705533202030) @@ -24,8 +24,9 @@ import InteractiveCardContainerModel from '../model/InteractiveCardContainerModel.js'; import Property from '../../../../axon/js/Property.js'; import CAVSoccerSceneModel from '../../common/model/CAVSoccerSceneModel.js'; -import { animatedPanZoomSingleton, HighlightFromNode, HighlightPath, KeyboardListener, NodeTranslationOptions, Path } from '../../../../scenery/js/imports.js'; +import { animatedPanZoomSingleton, Node, NodeTranslationOptions } from '../../../../scenery/js/imports.js'; import Vector2 from '../../../../dot/js/Vector2.js'; +import Range from '../../../../dot/js/Range.js'; import CAVConstants from '../../common/CAVConstants.js'; import Bounds2 from '../../../../dot/js/Bounds2.js'; import CardNode, { cardDropClip, cardPickUpSoundClip, PICK_UP_DELTA_X } from './CardNode.js'; @@ -34,7 +35,6 @@ import { Shape } from '../../../../kite/js/imports.js'; import Multilink from '../../../../axon/js/Multilink.js'; import isSettingPhetioStateProperty from '../../../../tandem/js/isSettingPhetioStateProperty.js'; -import GrabReleaseCueNode from '../../../../scenery-phet/js/accessibility/nodes/GrabReleaseCueNode.js'; import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; import CelebrationNode from './CelebrationNode.js'; import checkboxCheckedSoundPlayer from '../../../../tambo/js/shared-sound-players/checkboxCheckedSoundPlayer.js'; @@ -43,6 +43,7 @@ import CardDragIndicatorNode from './CardDragIndicatorNode.js'; import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js'; import GroupSortInteractionView from '../../../../scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.js'; +import CardModel from '../model/CardModel.js'; const FOCUS_HIGHLIGHT_Y_MARGIN = CAVConstants.CARD_SPACING + 3; @@ -61,6 +62,7 @@ // The message that appears when the cards are sorted private readonly celebrationNode: CelebrationNode; + private readonly groupSortInteractionView: GroupSortInteractionView; public constructor( model: InteractiveCardContainerModel, private readonly isSortingDataProperty: Property, @@ -78,6 +80,47 @@ this.celebrationNode = new CelebrationNode( model, this.cardMap, this.sceneModel.resetEmitter ); this.addChild( this.celebrationNode ); + this.groupSortInteractionView = new GroupSortInteractionView( model.groupSortInteractionModel, this, { + getNextSelectedGroupItem: unclampedDelta => { + // TODO: we have to do this every time, perhaps a parameter? https://github.com/phetsims/center-and-variability/issues/605 + const selectedCardModel = model.groupSortInteractionModel.selectedGroupItemProperty.value!; + const currentIndex = selectedCardModel.indexProperty.value!; + assert && assert( currentIndex !== null, 'need an index to be sorted' ); + + // TODO: until range is dynamic, this could be outside of current cards https://github.com/phetsims/center-and-variability/issues/605 + const delta = new Range( 0, this.getActiveCardNodesInOrder().length - 1 ).clampDelta( currentIndex, unclampedDelta ); + const newIndex = currentIndex + delta; + const cardNodes = this.getActiveCardNodesInOrder(); + const newCardNode = cardNodes[ newIndex ]; + assert && assert( newCardNode, 'wrong index for available cards I believe' ); + return newCardNode.model; + }, + onGrab: groupItem => { + groupItem.isDraggingProperty.value = true; + }, + onRelease: groupItem => { + groupItem.isDraggingProperty.value = false; + }, + sortGroupItem: ( selectedCardModel, newValue ) => { + assert && assert( selectedCardModel.indexProperty.value !== null, 'need an index to be sorted' ); + const delta = newValue - selectedCardModel.indexProperty.value!; + swapCards( this.getActiveCardNodesInOrder(), this.cardMap.get( selectedCardModel )!, delta ); + }, + onSort: () => { + + // See if the user unsorted the data. If so, uncheck the "Sort Data" checkbox + if ( this.isSortingDataProperty.value && !this.model.isDataSorted() ) { + this.isSortingDataProperty.value = false; + } + }, + getGroupItemToSelect: () => { + const activeCards = this.getActiveCardNodesInOrder(); + return activeCards[ 0 ] ? activeCards[ 0 ].model : null; + }, + getNodeFromModelItem: cardModel => this.cardMap.get( cardModel ) || null, + sortingRange: new Range( 0, this.model.cards.length - 1 ) // TODO: Need to support Property(Range) https://github.com/phetsims/center-and-variability/issues/605 + } ); + this.cardMap.forEach( ( cardNode, cardModel ) => { // Update the position of all cards (via animation) whenever any card is dragged cardNode.model.positionProperty.link( this.createDragPositionListener( cardNode ) ); @@ -107,32 +150,20 @@ } ); } ); - const focusHighlightPath = new HighlightPath( null, { - outerStroke: HighlightPath.OUTER_LIGHT_GROUP_FOCUS_COLOR, - innerStroke: HighlightPath.INNER_LIGHT_GROUP_FOCUS_COLOR, - outerLineWidth: HighlightPath.GROUP_OUTER_LINE_WIDTH, - innerLineWidth: HighlightPath.GROUP_INNER_LINE_WIDTH - } ); - - const grabReleaseCueNode = new GrabReleaseCueNode( { - top: CAVConstants.CARD_DIMENSION + FOCUS_HIGHLIGHT_Y_MARGIN + 15, - visibleProperty: model.grabReleaseCueVisibleProperty - } ); - - const keyboardSortCueNode = GroupSortInteractionView.createSortCueNode( model.isKeyboardDragArrowVisibleProperty ); + const keyboardSortCueNode = GroupSortInteractionView.createSortCueNode( model.groupSortInteractionModel.keyboardSortCueVisibleProperty ); this.addChild( keyboardSortCueNode ); const cardDragIndicatorNode = new CardDragIndicatorNode( { centerTop: new Vector2( 0.5 * CAVConstants.CARD_DIMENSION - PICK_UP_DELTA_X, CAVConstants.CARD_DIMENSION - 10 ), visibleProperty: new DerivedProperty( - [ this.inputEnabledProperty, model.isKeyboardFocusedProperty, model.dragIndicationCardProperty ], - ( inputEnabled, hasKeyboardFocus, dragIndicationCard ) => inputEnabled && !hasKeyboardFocus && !!dragIndicationCard ) + [ this.inputEnabledProperty, model.groupSortInteractionModel.mouseSortCueVisibleProperty ], + ( inputEnabled, mouseSortCueVisible ) => inputEnabled && mouseSortCueVisible ) } ); this.addChild( cardDragIndicatorNode ); - model.dragIndicationCardProperty.lazyLink( ( newCard, oldCard ) => { + model.groupSortInteractionModel.selectedGroupItemProperty.lazyLink( ( newCard, oldCard ) => { if ( oldCard ) { const oldCardNode = this.cardMap.get( oldCard )!; @@ -176,7 +207,7 @@ // Needs to be pickable in accordion box. this.pickable = true; - const focusedCardNodeProperty: TReadOnlyProperty = new DerivedProperty( [ model.focusedCardProperty ], focusedCard => { + const focusedCardNodeProperty: TReadOnlyProperty = new DerivedProperty( [ model.groupSortInteractionModel.selectedGroupItemProperty ], focusedCard => { return focusedCard === null ? focusedCard : this.cardMap.get( focusedCard )!; } ); @@ -187,77 +218,57 @@ // When a user is focused on the card container but there are no cards yet, we want to ensure that a card gets focused // once there is a card. - if ( model.focusedCardProperty.value === null && this.focused && model.getActiveCards().length === 1 ) { - model.focusedCardProperty.value = activeCardNodes[ 0 ].model; + // TODO: this.focused should use the model https://github.com/phetsims/center-and-variability/issues/605 + if ( model.groupSortInteractionModel.selectedGroupItemProperty.value === null && this.focused && model.getActiveCards().length === 1 ) { + model.groupSortInteractionModel.selectedGroupItemProperty.value = activeCardNodes[ 0 ].model; } - // If the card cells changed, and we have no more active cards left, that means that all the cards were removed. + // If the card cells changed, and we have no more active cards left, that means that all the cards were removed. // Therefore, we want to set the focused card to null. else if ( model.getActiveCards().length === 0 ) { - model.focusedCardProperty.value = null; - } - } ); - - this.addInputListener( { - focus: () => { - const activeCardNodes = this.getActiveCardNodesInOrder(); - if ( model.focusedCardProperty.value === null && activeCardNodes.length > 0 ) { - model.focusedCardProperty.value = activeCardNodes[ 0 ].model; - } - - // When the group receives keyboard focus, make sure that the focused card is displayed - if ( focusedCardNodeProperty.value ) { - animatedPanZoomSingleton.listener.panToNode( focusedCardNodeProperty.value, true ); - } - model.isKeyboardFocusedProperty.value = true; - }, - blur: () => { - model.isCardGrabbedProperty.value = false; - model.isKeyboardFocusedProperty.value = false; + model.groupSortInteractionModel.selectedGroupItemProperty.value = null; } } ); // When pdomFocusHighlightsVisibleProperty become false, interaction with a mouse has begun while using // Interactive Highlighting. When that happens, clear the sim-specific state tracking 'focused' cards. + // TODO: MS: This seems similar to the "over" strategy inside the group sort view, let's talk https://github.com/phetsims/center-and-variability/issues/605 phet.joist.sim.display.focusManager.pdomFocusHighlightsVisibleProperty.link( ( visible: boolean ) => { if ( !visible ) { - if ( model.focusedCardProperty.value !== null ) { + if ( model.groupSortInteractionModel.selectedGroupItemProperty.value !== null ) { // Before clearing out the focusedCardProperty the CardModel must be cleared out of it's // dragging state. - model.focusedCardProperty.value.isDraggingProperty.set( false ); + model.groupSortInteractionModel.selectedGroupItemProperty.value.isDraggingProperty.set( false ); // Clear the 'focused' card so that there isn't a flicker to a highlight around that card when // moving between the CardNode interactive highlight and the container group highlight (which has // a custom highlight around the focused card). - model.focusedCardProperty.set( null ); + model.groupSortInteractionModel.selectedGroupItemProperty.value = null; } - model.isCardGrabbedProperty.value = false; + model.groupSortInteractionModel.isGroupItemKeyboardGrabbedProperty.value = false; // This controls the visibility of interaction cues (keyboard vs mouse), so we need to clear it when // switching interaction modes. - model.isKeyboardFocusedProperty.value = false; + // TODO: move to common code? https://github.com/phetsims/center-and-variability/issues/605 + model.groupSortInteractionModel.isKeyboardFocusedProperty.value = false; } } ); - Multilink.multilink( [ focusedCardNodeProperty, model.isCardGrabbedProperty ], ( focusedCardNode, isCardGrabbed ) => { + Multilink.multilink( [ focusedCardNodeProperty, model.groupSortInteractionModel.isGroupItemKeyboardGrabbedProperty ], + ( focusedCardNode, isCardGrabbed ) => { if ( focusedCardNode ) { - const focusForSelectedCard = new HighlightFromNode( focusedCardNode.cardNode, { dashed: isCardGrabbed } ); - this.setFocusHighlight( focusForSelectedCard ); - focusedCardNode.model.isDraggingProperty.value = isCardGrabbed; - keyboardSortCueNode.centerBottom = new Vector2( focusedCardNode.centerX + CARD_LAYER_OFFSET + PICK_UP_DELTA_X / 2 + 1, focusForSelectedCard.bottom + 11 ); - } - else { - this.setFocusHighlight( 'invisible' ); + keyboardSortCueNode.centerBottom = new Vector2( focusedCardNode.centerX + CARD_LAYER_OFFSET + PICK_UP_DELTA_X / 2 + 1, + // TODO: MS: Help? https://github.com/phetsims/center-and-variability/issues/605 + ( this.focusHighlight as unknown as Node ).bottom + 11 ); } - } - ); + } ); - model.isCardGrabbedProperty.link( isCardGrabbed => { + model.groupSortInteractionModel.isGroupItemKeyboardGrabbedProperty.link( isCardGrabbed => { if ( isCardGrabbed ) { this.wasSortedBefore = model.isDataSorted(); } @@ -278,6 +289,7 @@ // Move and swap cards according to the focused card's target index. Used for alternative input. const swapCards = ( activeCards: CardNode[], focusedCard: CardNode, delta: number ) => { const currentIndex = activeCards.indexOf( focusedCard ); + assert && assert( focusedCard.model.indexProperty.value === currentIndex, 'sanity check' ); const targetIndex = Utils.clamp( currentIndex + delta, 0, activeCards.length - 1 ); if ( targetIndex !== currentIndex ) { @@ -301,100 +313,28 @@ animatedPanZoomSingleton.listener.panToNode( focusedCard, true ); model.cardCellsChangedEmitter.emit(); - - // Gets rid of the hand icon - model.hasKeyboardMovedCardProperty.value = true; } }; - const keyboardListener = new KeyboardListener( { - fireOnHold: true, - keys: [ 'd', 'a', 'arrowRight', 'arrowLeft', 'w', 's', 'arrowUp', 'arrowDown', 'enter', 'space', 'home', 'end', 'escape', 'pageUp', 'pageDown' ], - callback: ( event, keysPressed ) => { - - const focusedCardNode = focusedCardNodeProperty.value; - const activeCardNodes = this.getActiveCardNodesInOrder(); - const numberOfActiveCards = activeCardNodes.length; - const isCardGrabbed = model.isCardGrabbedProperty.value; - - // If there are no active cards no card can be focused and no keyboard input is allowed. - if ( numberOfActiveCards === 0 ) { - model.focusedCardProperty.value = null; - return; - } - - if ( focusedCardNode ) { - const delta = this.getKeystrokeDelta( keysPressed, numberOfActiveCards ); - - if ( [ 'enter', 'space' ].includes( keysPressed ) ) { - model.isCardGrabbedProperty.value = !model.isCardGrabbedProperty.value; - model.hasKeyboardGrabbedCardProperty.value = true; - - // See if the user unsorted the data. If so, uncheck the "Sort Data" checkbox - if ( !model.isCardGrabbedProperty.value && this.isSortingDataProperty.value && !this.model.isDataSorted() ) { - this.isSortingDataProperty.value = false; - } - } - else if ( isCardGrabbed ) { - if ( keysPressed === 'escape' ) { - model.isCardGrabbedProperty.value = false; - } - - // If we have a nonNull delta and the card is grabbed we want to swap card positions. - else if ( delta !== null ) { - swapCards( activeCardNodes, focusedCardNode, delta ); - } - } - - // If we have a nonNull delta and the card is not grabbed we want to shift focus to a different card. - else if ( delta !== null ) { - // Shift the card focus when a card is not grabbed. - const currentIndex = activeCardNodes.indexOf( focusedCardNode ); - const nextIndex = Utils.clamp( currentIndex + delta, 0, numberOfActiveCards - 1 ); - model.focusedCardProperty.value = activeCardNodes[ nextIndex ].model; - model.hasKeyboardSelectedDifferentCardProperty.value = true; - animatedPanZoomSingleton.listener.panToNode( focusedCardNode, true ); - } - else { + const grabReleaseCueNode = GroupSortInteractionView.createGrabReleaseCueNode( model.groupSortInteractionModel.grabReleaseCueVisibleProperty, { + top: CAVConstants.CARD_DIMENSION + FOCUS_HIGHLIGHT_Y_MARGIN + 15 + } ); - // No cards are grabbed! We cleared the 'focused' card because we were using mouse input - start over with - // keyboard interaction and focus the first card AND make sure that all cards individually are no longer - // dragging. - this.cardNodes.forEach( cardNode => { - cardNode.model.isDraggingProperty.value = false; - } ); - model.isCardGrabbedProperty.value = false; + // TODO: MS! Discuss this as a design patter vs. handling visibility manually, https://github.com/phetsims/center-and-variability/issues/605 + this.groupSortInteractionView.groupFocusHighlightPath.addChild( grabReleaseCueNode ); - model.focusedCardProperty.value = activeCardNodes[ 0 ].model; - } - } - else { - model.focusedCardProperty.value = activeCardNodes[ 0 ].model; - } - } - } ); const focusHighlightWidthProperty = new DerivedProperty( [ model.numActiveCardsProperty ], numActiveCards => { return model.getCardPositionX( numActiveCards === 0 ? 1 : numActiveCards ); } ); - focusHighlightPath.addChild( grabReleaseCueNode ); - - const highlightRectangle = new Path( null ); - this.addChild( highlightRectangle ); - highlightRectangle.moveToBack(); - + const marginX = 7; focusHighlightWidthProperty.link( focusHighlightWidth => { - const marginX = 7; const focusRect = Shape.rect( -marginX, -FOCUS_HIGHLIGHT_Y_MARGIN, focusHighlightWidth + 2 * marginX, CAVConstants.CARD_DIMENSION + 2 * FOCUS_HIGHLIGHT_Y_MARGIN + 9 ); - focusHighlightPath.setShape( focusRect ); - highlightRectangle.setShape( focusRect ); + this.groupSortInteractionView.groupFocusHighlightPath.setShape( focusRect ); const cueNodeWidth = grabReleaseCueNode.width; - grabReleaseCueNode.centerX = Utils.clamp( focusRect.bounds.centerX, cueNodeWidth / 2, this.width - cueNodeWidth / 2 ); + grabReleaseCueNode.centerX = Utils.clamp( focusRect.bounds.centerX, cueNodeWidth / 2, Math.max( this.width - cueNodeWidth / 2, cueNodeWidth ) ); } ); - - this.setGroupFocusHighlight( focusHighlightPath ); - this.addInputListener( keyboardListener ); } // The listener which is linked to the cardNode.positionProperty @@ -441,19 +381,6 @@ } }; } - - private getKeystrokeDelta( keysPressed: string, numberOfActiveCards: number ): number | null { - if ( [ 'arrowRight', 'arrowLeft', 'a', 'd', 'arrowUp', 'arrowDown', 'w', 's' ].includes( keysPressed ) ) { - return [ 'arrowRight', 'd', 'arrowUp', 'w' ].includes( keysPressed ) ? 1 : -1; - } - else if ( [ 'pageUp', 'pageDown' ].includes( keysPressed ) ) { - return keysPressed === 'pageUp' ? 3 : -3; - } - else if ( [ 'home', 'end' ].includes( keysPressed ) ) { - return keysPressed === 'end' ? numberOfActiveCards : -numberOfActiveCards; - } - return null; - } } centerAndVariability.register( 'InteractiveCardNodeContainer', InteractiveCardNodeContainer ); \ No newline at end of file Index: scenery-phet/js/accessibility/group-sort/model/GroupSortInteractionModel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/scenery-phet/js/accessibility/group-sort/model/GroupSortInteractionModel.ts b/scenery-phet/js/accessibility/group-sort/model/GroupSortInteractionModel.ts --- a/scenery-phet/js/accessibility/group-sort/model/GroupSortInteractionModel.ts (revision c909d03c4d0c7d9bdc59972ddf53bc9b71b1a01c) +++ b/scenery-phet/js/accessibility/group-sort/model/GroupSortInteractionModel.ts (date 1705529838864) @@ -96,6 +96,8 @@ // Whether any group item has ever been sorted to a new value, even if not by the group sort interaction. For best results, // set this to true from other interactions too (like mouse/touch). + // TODO: DESIGN?!?!? this should be derived, and a new Property for just mouse created. Basically how important is + // it to have a mouse drag indicator after successful keyboard sort (but no successful mouse) https://github.com/phetsims/scenery-phet/issues/815 public readonly hasGroupItemBeenSortedProperty: Property; public readonly getValueProperty: ( itemModel: ItemModel ) => TProperty; @@ -132,8 +134,8 @@ this.hasKeyboardGrabbedGroupItemProperty, this.isKeyboardFocusedProperty, this.enabledProperty - ], ( selectedGroupItem, hasGrabbedBall, hasKeyboardFocus, enabled ) => { - return selectedGroupItem !== null && !hasGrabbedBall && hasKeyboardFocus && enabled; + ], ( selectedGroupItem, hasGrabbedGroupItem, hasKeyboardFocus, enabled ) => { + return selectedGroupItem !== null && !hasGrabbedGroupItem && hasKeyboardFocus && enabled; } ); this.keyboardSortCueVisibleProperty = new DerivedProperty( [ @@ -173,8 +175,13 @@ public registerUpdateSortIndicatorNode( updateSortIndicatorNode: () => void ): void { this.mouseSortCueVisibleProperty.link( updateSortIndicatorNode ); this.selectedGroupItemProperty.link( updateSortIndicatorNode ); + this.hasGroupItemBeenSortedProperty.link( updateSortIndicatorNode ); + this.isKeyboardFocusedProperty.link( updateSortIndicatorNode ); + this.disposeEmitter.addListener( () => { + this.isKeyboardFocusedProperty.unlink( updateSortIndicatorNode ); + this.hasGroupItemBeenSortedProperty.unlink( updateSortIndicatorNode ); this.mouseSortCueVisibleProperty.unlink( updateSortIndicatorNode ); this.selectedGroupItemProperty.unlink( updateSortIndicatorNode ); } );

[zepumph]

I had a productive meeting with @marlitas and @jbphet today. We went through all TODOs pointing to this issue and determined how to proceed. The implementations are above. @marlitas, would you like to give this a review? I didn't want to just close it, but I'm feeling pretty confident that we are moving in the right direction.

1. Can you poke around the card interaction (in the sim) and see if anything seems off to you.
2. Note that PhET-iO won't be handled here, but rather in https://github.com/phetsims/scenery-phet/issues/815.
3. Look through the above commits and let me know if you think of anything to improve.

Thanks!

[marlitas]

I poked around the card interaction and all looks well. I tried going through some edge cases I remembered from the most recent publication, and those seemed to be handling things appropriately. I also glanced through InteractiveCardContainer specific code and the implementation of GroupSortInteraction really did clean some things up and the documentation felt appropriate as well. A deeper code review of GroupSortInteraction is happening here: https://github.com/phetsims/scenery-phet/issues/841, so I didn't dive too deep into that specific code, rather the implementation methods related to cards in CAV.

I found nothing that stood out as buggy or that needed clarification. The changes look really good. Thanks for doing all this work @zepumph!