Add phet-io support to ComboBox

[samreid]

Needed for phetsims/concentration#34

[samreid]

This one is somewhat tricky since things are created dynamically. I implemented one idea above, but it should be discussed.

[samreid]

Perhaps @pixelzoom & I can also discuss this on Monday as it pertains to phetsims/concentration#34

[pixelzoom]

Discussed with @samreid. Sounds like the together approach is going to change, so implementation here will be revised. One general thing that should change is how the parts of a combo box are identified. Items in the list are currently identified as buttons. A combo box has one button, and a list of items (which may be of any type).

[samreid]

@bereaphysics I have forthcoming changes for this issue, but wasn't sure about Linear Regression sim status. Are you close to release candidate? Have you already recorded safe shas? Just wondering how I should proceed here without running any risk to your sim.

[veillette]

We have already branched out and issued a RC for LSR.

[samreid]

Perfect, thanks for letting me know. I'll work in master, testing 1-2 sims for each commit and requesting review for nontrivial changes, unless @pixelzoom @jbphet @bereaphysics @aaronsamuel137 or @jonathanolson prefers I work in a branch.

[samreid]

In our meeting today, we decided it will be a lot of work for sim components to make it possible to generate sim playbacks for every feature (whether buttons are pressed, highlighted, etc.). We need to ask if this level of highlighting is necessary, or if it is sufficient to show coarse model features.

For now, I checked in the changes in a branch above. Can @pixelzoom @jbphet or @jonathanolson review those changes before my metacog meeting on Friday 10am?

[samreid]

Also, note that ComboBox has one other together feature already in master, but it is very non-invasive.

[pixelzoom]

@samreid wrote:

For now, I checked in the changes in a branch above.

Assuming that you're referring to commit c5118ea in the 'together' branch... The only bit that I dislike is listNodeVisibleProperty. In general, capturing the behavior of UI components via axon Properties would be a costly approach. So I think we should (a) find out if capturing such behavior is necessary, and (b) if so, investigate other approaches.

[samreid]

So I think we should (a) find out if capturing such behavior is necessary, and (b) if so, investigate other approaches.

Let us assume for the sake of discussion that capturing such behavior is necessary.

@pixelzoom in the case of ComboBox, we must call moveList() immediately before showing it, so it will appear in the right position. Can you elaborate on acceptable alternatives to AXON/Property (or at a minimum elaborate more on why Property is a problem so I can propose alternatives)? Some possibilities that come to my mind:

1. Create an ES5-setter set listVisible in ComboBox that calls moveList()
2. Create a method in ComboBox setListVisible that calls moveList()

In general, capturing the behavior of UI components via axon Properties would be a costly approach.

Our mileage may vary on this. Some components, such as Sun/buttons are already driven by axon Properties. But in general I agree it would be better if we not require every UI component to be Property-driven.

[pixelzoom]

@samreid Is this issue still relevant, now that the approach has changed?

[samreid]

Well, we still have the following issues:
(1) we may wish to get a message whenever the user mouses over an item and it highlights
(2) we may wish to make individual buttons in the combo box be separate UI components, see https://github.com/phetsims/together/issues/76
(3) we may need a way to playback recorded sessions using combo box (though not for immediate deadlines)

[samreid]

Unassigned because I have no plans to work on this at the moment.

[pixelzoom]

Since we're about to close #405 (instrument ComboBox better), I'm guessing that this issue can also be closed. @zepumph @samreid if you agree, please close.

[samreid]

I think this issue can be closed--leaving assigned to @zepumph for confirmation.