convert sims to use AquaRadioButtonGroup

[pixelzoom]

Over in https://github.com/phetsims/sun/issues/549, we decided to change how AquaRadioButtons are instrumented. Instead of associating the linked Property with each button, the linked Property will be associated with the group (as is the case for RadioButtonGroup). As noted in https://github.com/phetsims/sun/issues/549#issuecomment-568003921:

This also means that all AquaRadioButtons will need to be instantiated via VerticalAquaRadioButtonGroup, not instantiated directly and added to a custom "group" implementation. This is probably a good constraint, since the group needs to support both phet-io and a11y.

So all sims that are explicitly instantiating AquaRadioButtons will need to be changed to use VerticalAquaRadioButtonGroup. This presumably needs to be done immediately for instrumented sims, but could be a "chip away" for non-instrumented sims.

Here's are the lists of sims that are calling new AquaRadioButton, with responsible developers indicated.

Instrumented, or in progress:

• [x] beers-law-lab @pixelzoom - see https://github.com/phetsims/beers-law-lab/issues/245
• [ ] build-an-atom @jbphet
• [ ] circuit-construction-kit-common @samreid
• [ ] energy-forms-and-changes @chrisklus
• [ ] energy-skate-park @jessegreenberg
• [x] gas-properties @pixelzoom - ⚠️ see #569
• [ ] gravity-and-orbits @jessegreenberg @samreid)
• [ ] hookes-law @pixelzoom -❌ see https://github.com/phetsims/hookes-law/issues/76
• [ ] projectile-motion @jbphet (@zepumph)

Not instrumented:

• [ ] acid-base-solutions @pixelzoom - ❌ see https://github.com/phetsims/acid-base-solutions/issues/161
• [x] balancing-chemical-equations @pixelzoom
• [ ] bending-light @samreid
• [ ] build-a-molecule @jonathanolson
• [ ] curve-fitting @jonathanolson
• [ ] density-and-buoyancy @jonathanolson
• [ ] fluid-pressure-and-flow @zepumph
• [ ] isotopes-and-atomic-mass @jbphet
• [ ] masses-and-springs @Denz1994
• [x] molecule-polarity @pixelzoom
• [ ] optics-lab @veillette
• [ ] pendulum-lab @jonathanolson
• [ ] reactants-products-and-leftovers @pixelzoom - ❌ see https://github.com/phetsims/reactants-products-and-leftovers/issues/68
• [x] states-of-matter @jbphet
• [ ] tambo @jbphet
• [ ] trig-tour @jessegreenberg

And perhaps we should add "new AquaRadioButton" to lint rule bad-text.js?

Labeling for dev meeting to discuss.

[zepumph]

The PhET-iO team, especially in relation to https://github.com/phetsims/sun/issues/550, is wondering if we don't want to try to move AquaRadioButton's functionality into RadioButtonGroup. This would allow us to deprecate VerticalAquaRadioButtonGroup, which is largely duplicated layout code with RadioButtonGroup.

What if there was an option like: stye: 'normal', {'aqua'|'normal'}. Since we may be touching all AquaRadioButton occurrences in this issue anyways, wouldn't it be nicer to end up with one implementation in the end?

More generally rephrasing, the above sentiment: what do we want our long term radio button api to look like (like https://github.com/phetsims/sun/issues/523)? Ideally we would have just one implementation under the hood, but we still could have two subtypes if desired.

[pixelzoom]

@zepumph I believe that's what will need to happen in order to address https://github.com/phetsims/sun/issues/523 (unify radio button group implementations), currently assigned to @chrisklus with low priority.

[pixelzoom]

Uh oh, we need to rethink this.

I thought I'd try this conversion to see what problems I might run into. I immediately ran into this situation in acid-base-solutions:

This is a group of 3 radio buttons. The first radio button has a related checkbox. I think this is a perfectly valid UI design, and I don't think this is an isolated example. And it's impossible to accomplish with VerticalAquaRadioButtonGroup. We could address this by moving the "Molecules" radio button to the bottom of the group. But is that a design constraint that PhET would (or should have to) live with? And what if more than one radio button had an associated checkbox?

hookes-law is similar to acid-base-solutions, where a radio button has an associated UI component. We lucked out in this case because it's associated with the bottom radio button.

The other type of problem is layout. There are a number of sims that use a horizontal layout, not supported by VerticalAquaRadioButtonGroup. This is an easier problem to solve -- generalize VerticalAquaRadioButtonGroup to AquaRadioButtonGroup with orientation option. Examples:

balancing-chemical-equations:

beers-law-lab:

gravity-and-orbits:

fluid-pressure-and-flow:

I said this long ago when VerticalAquaRadioButtonGroup was first created - I don't like it. I don't like the fact that it has responsibility for creating the radio buttons. It should be responsible for the "groupness" and (perhaps) layout. Java's ButtonGroup pattern would be a good place to start: http://www.fredosaurus.com/notes-java/GUI/components/50radio_buttons/25radiobuttons.html

[chrisklus]

Should we discuss next steps at an upcoming developer meeting? Perhaps I should be addressing #523 at high priority.

[zepumph]

After discussing in the dev meeting today:

• We don't think it is worth design time on complicated cases until we actually get back to those sims.
• We could rename VerticalAquaRadioButtonGroup to AquaRadioButtonGroup and add an 'orientation' option, like in RadioButtonGroup. That way a few more cases above could be converted.
• Still it seemed like there were no objections to converting to this type when appropriate (easy). Adding "chip-away" to this issue.

[pixelzoom]

From https://github.com/phetsims/sun/issues/555#issuecomment-585958226:

• We could rename VerticalAquaRadioButtonGroup to AquaRadioButtonGroup and add an 'orientation' option, like in RadioButtonGroup. That way a few more cases above could be converted.

This is in fact necessary to address any of the sims that have horizontal layout of AquaRadioButtons. So I told a stab at it. The above commits are related to factoring out AquaRadioButtonGroup, to support both vertical and horizontal orientations. While I started that work here, it blossomed into its own issue, and was moved to https://github.com/phetsims/sun/issues/566.

[pixelzoom]

I ran into a problem when trying to use VerticalAquaRadioButtonGroup in gas-properties. HoldConstantControl observers other Properties to decide which of the radio buttons should be enabled. So I need references to the radio buttons, which AquaRadioButtonGroup does not provide.

If any devs are watching this issue, please chime in with suggestions.

EDIT: Moved to #569.

[pixelzoom]

@ariel-phet I've looked at my sims in the checklist. Only a couple were straightforward to convert. Others have significant problems to work out, and I've created issues, as noted in the checklist.

[pixelzoom]

In https://github.com/phetsims/sun/issues/555#issuecomment-570730096, it was noted that AquaRadioButtonGroup has no support for interleaved UI components, a requirement for several sims. Since work/discussion on this has stalled, I've moved that to its own issue. See https://github.com/phetsims/sun/issues/583.

[jbphet]

This was discussed in the 3/26/2020 developer meeting, and it seems like there is significant tension between the current phet-io API design and the ability to use radio button groups in different layouts, such as the ones shown in https://github.com/phetsims/sun/issues/555#issuecomment-570730096. @pixelzoom was making the point that the layout and the "groupness" are separate functionalities that should not be combined into one class, which is where it stands now. We decided that this situation needs to be discussed with the phet-io designers so that we can work out a good way to separate the "groupness" from the layout, yet have it still be usable and not overly complicated in the phet-io API.

I've marked the issue for the next phet-io meeting where @zepumph can bring it up.

[zepumph]

From a phet-io spec perspective, the nesting in the studio tree could be pretty flexible.

Option 1 - model structure may be independent of visual display. We would be ok with this, but it isn't quite as clear as Option 3 in the general case.
graphRadioButtonGroup
    barGraphRadioButton
    energyPlotRadioButton
    forcePlotRadioButton
energyCheckbox

Option 2 - nested checkbox is a sibling to its parent radio button (weird). Let's not do this
graphRadioButtonGroup
    barGraphRadioButton
    energyPlotRadioButton
    forcePlotRadioButton
    energyCheckbox

Option 3 - visual nested component would be also nested in studio under that radio button. We prefer this option
graphRadioButtonGroup
    barGraphRadioButton
    energyPlotRadioButton
    forcePlotRadioButton
        energyCheckbox (or any Node)

An important piece that should be implemented for PhET-iO is that toggling the visibility of the parent radio button should also toggle the visibility of the nested content.

Implementation

One way to support this would be in AquaRadioButton solely: Add support in the items list for a new property, like childContentNode, and then AquaRadioButtonGroup would control the layout of the sub-components

We also discussed solving this more generally, noting that AquaRadioButtons and checkboxes are often stacked, interleaved, and nested (CAF, HL, etc). Perhaps when we work on this next, a layout manager like "VerticalFormTreeLayoutManager" that does this work for any Node tree-like structure.

We decided that we would not work on this until the next time a simulation that uses these weird AquaRadioButton cases would need it. Unassigning until then.

@pixelzoom edit: I modified the above options to correspond to this example in Hooke's Law, since that was the context of our phet-io meeting discussion: