Generalize ToggleNode

[samreid]

From slack:

Sam Reid [12:53 PM]
Do we have something like ToggleNode.js but that works for an arbitrary number of nodes?

Chris Malley [1:02 PM]
RadioButtonGroup
You want a toggle with N states?

Sam Reid [1:03 PM]
I was asking about a generalization of ToggleNode, not ToggleButtonNode.
right (edited)

Michael Kauzmann [1:04 PM]
like you keep clicking the same node and it goes into state 1, then 2, then 3, then n, then back to 1?

Sam Reid [1:04 PM]
take a look at ToggleNode

Chris Malley [1:04 PM]
Not that I know of. Shouldn't be too difficult to generalize (and rename) ToggleNode
You want to display 1 of N nodes based on the value of a Property, yes?

Sam Reid [1:05 PM]
ToggleNode is used in 9 spots and has a very clean API. Perhaps if I generalize it, we would still keep ToggleNode, but it would be a subtype.

You want to display 1 of N nodes based on the value of a Property, yes?

That’s right.

Chris Malley [1:29 PM]
+1 subtype

[samreid]

I'm planning to rename ToggleNode => BooleanToggleNode and use the name ToggleNode to refer to the new general (out of N) type.

[samreid]

I've prepared an initial draft. I tested Equality Explorer (uses the lock icon), Balloons and Static Electricity, Energy Skate Park: Basics and Plinko Probability, and haven't seen any problems so far. Committing shortly, will affect several repos. Also, lint is passing and there are no systemic problems observed in aqua so far.

[samreid]

Commits are pushed above, @pixelzoom you were listed as a co-author of the original ToggleNode (now renamed to BooleanToggleNode), and LockControl uses BooleanToggleNode, would you like to review?

[pixelzoom]

Looks good. One question about this comment in ToggleNode:

39 // NOTE this is different than in ToggleNode
40 alignChildren: ToggleNode.CENTER

Should this be "different that BooleanToggleNode" ?

[pixelzoom]

Also wondering about this comment in BooleanToggleNode:

      // For compatibility with prior usage, we align the x coordinate
      alignChildren: ToggleNode.HORIZONTAL

I'm curious about which prior usage(s) rely on horizontal alignment. Would it be advantageous to make those usages specify ToggleNode.HORIZONTAL, rather than using what seems like an odd default?

[samreid]

I reviewed usages of BooleanToggleNode, and checked if BooleanToggleNode defaulting to CENTER gives identical results.

Balloons and Static Electricity: wall button, looks the same in either case (in english).
Forces and Motion: Basics: Go/Pause button looks the same in either case
Equality Explorer: supplies an override anyways.
Plinko Probability: Play/Pause Button looks the same in either case.

It seems reasonable that icons in BooleanRectangularToggleButton and BooleanRoundToggleButton should be vertically centered. I tested Area Model Multiplication/Game and the sound toggle icon looked good. However, I did not exhaustively test all occurrences of BooleanRectangularToggleButton or BooleanRoundToggleButton.

I'm inclined to commit this simplification.

[pixelzoom]

Equality Explorer: supplies an override anyways.

Override was removed in phetsims/equality-explorer#106, but I believe that CENTER would work fine.

[samreid]

I pushed the commit that defaults BooleanToggleNode to CENTER (as specified in ToggleNode). @pixelzoom back to you for review.