Create a broader supertype for GroupSort view

[marlitas]

I discussed the alt-input interaction for the location based objects in Number Pairs with @zepumph. There are ways in which the newly created GroupSortInteraction would work well for the use case, and ways in which it would not. Mainly, GroupSortInteraction is really only made to handle 1D movement of items, while the location based objects in Number Pairs require 2D movement.

We both agreed that for navigating a group of objects within one tab stop is a common PhET pattern, therefore creating common code support for this pattern is appropriate.

After looking through GroupSortInteractionModel together we both agreed the class is general enough to cover both 1D and 2D scenarios. Therefore GroupSortInteractionModel can remain as-is.

GroupSortInteractionView will become a subtype of GroupSelectView that adds on the keyboard listeners, as well as other logic that constricts use cases to 1D scenarios.

[marlitas]

I did a first pass at this. So far I have not scene any buggy behavior. The use cases so far are in Center and Variability, and Mean: Share and Balance.

I also renamed GroupSortInteractionModel to GroupSelectModel since it is now more broad. However, there are a couple of class properties that I feel should also be renamed, for example hasMouseSortedGroupItem no longer feels appropriate if perhaps the action is not to sort but to drag, or to press, or to etc. I had difficulty thinking of an term to swap "sort" out with. The first thing that came to mind was "interact" but that also felt potentially overloaded with the name that's been adopted of "Group Sort Interaction" which is now a subtype interaction.

When chatting with @zepumph he request he not be the main reviewer on this since he is busy with some other work. It feels appropriate for @pixelzoom to review and be part of continued discussions. I am assuming this is @pixelzoom's first time working with Group Sort Interaction since it has only been implemented in two sims, so I am happy to connect synchronously first if that would be helpful.

Either way I think this is now in a place where the super type is usable for Number Pairs while still allowing space for changes.

[pixelzoom]

I looked at this briefly, but (even with the extensive doc in GroupSelectModel) it's overwhelming. I think it would be most efficient if I got a walkthrough from either @marlitas or @zepumph. Do I have a volunteer?

[zepumph]

When chatting with @zepumph he request he not be the main reviewer on this since he is busy with some other work. It feels appropriate for @pixelzoom to review and be part of continued discussions.

I was very concerned about my highest priority task last iteration, but it has been largely accomplished and I now feel able to review these changes. I'll take a look now.

[zepumph]

• Let's delete the create() functions until we need them. When I looked for usages, it got a lot of false possitives, but I didn't see any actual usages. If this seems correct to you, let's clean it up. If there is a usages for GroupSortInteractionView, then let's still delete the GroupSelectView.
• For your number pairs usage, you probably already know these things, but just in case:
  • I see that the Grab cue node probably needs to be better positioned.
  • Make sure to reset your GroupSelectModel in reset all to get the cue node back on reset.
  • Interactive highlights is not working on the apples. Likely we want to implement this before calling this refactor complete, just in case. That said, CAV interactive highlights is looking good still, so probably not too bad.
• This block should most likely be in the supertype. The actual strings still make sense to me, but if the context is greatly different, we could also make the strings options later,

  scenery-phet/js/accessibility/group-sort/view/GroupSortInteractionView.ts

  Lines 186 to 190 in 044b746

  	Multilink.multilink( [
  	model.isGroupItemKeyboardGrabbedProperty
  	], isGrabbed => {
  	primaryFocusedNode.setPDOMAttribute( 'aria-roledescription', isGrabbed ? sortableStringProperty : navigableStringProperty );
  	} );

  .
• createSortCueNode should probably be in the subtype
• I'm confused by how onGroupItemChange is declared in the super, but only used in group sort. It makes me wonder if group select supports pan/zoom still. (I just tested, it isn't working).
• Agreed, maybe "move" is best, "select and move". No matter, many variables and doc in GroupSelectView also need this updating to whatever term you decide. Make sure to note Migration Rules when changing instrumented Properties in GroupSelectModel.

  I had difficulty thinking of an term to swap "sort" out with.

• RE the naming, it is strange that we have a supertype named for the "selecting" state, and then the subtype named for the "selected" state. It isn't obvious how both of these have both a selecting and selection state. Can we improve on this at all?
• When selecting in NumberPairs and in CAV, interaction keys don't "wrap" back to the other side. I understand why, and this is probably best, but I wanted to humor the design decision that an input key ALWAYS moves the selection to something. This may be confusing if you press the down arrow and end up at the top, but it also seems a bit awkward in general when an input has no change on the sim. If we like having a no-op at the edge, in some spots we implemented a "boundary sound" boop that plays when the selection doesn't change. That may be a nice improvement. In CAV, the sound repeats when sorting at a min/max, but there is no sound for selection. What do you think?
• I couldn't really find exactly where the detection algorithm was for finding the next apple given an input key, but I think this should potentially be factored out, perhaps given a current position, input key + list of positionProperties, I can imagine it being pretty universal to find the next item, and consistency across 2d selection would be nice to have, especially if we want things to get extra nuanced about how we find the next item.

This is kinda a lot, but I also think it is really important, and in scope for Number Pairs work. From what I've discussed with @marlitas, this is a common interaction that designers are continuing to ask for. We should devote time to make this really excellent for all our use cases. Likely we should sync up next time we have questions/review instead of doing more async messaging.