AccordionBox is over-instrumented

[samreid]

@pixelzoom identified that AccordionBox is over-instrumented in phetsims/graphing-quadratics#14. It instruments expandedBoxOutline, collapsedBoxOutline, plusSymbol and minusSymbol which should be uninstrumented. Maybe more.

[samreid]

I uninstrumented the specified parts of AccordionBox. @pixelzoom requested more specifically:

Chris Malley [9:34 PM]
Perhaps I’m at a place where I should push. I can use the sim in Studio. But I see continuous “Uncaught Error: Cannot read property ‘instance’ of null”.
I have several AccordionBoxes in this sim. It’s PhET-iO API is a little over the top (change visibility of + and - on the expand/collapse button?) My team wants to only provide the ability to change visibility of the entire AccordionBox. How do I make that happen?

Sam Reid [9:37 PM]
If your goal is to prevent CT breakages, you can test http://localhost/phet-io-wrappers/phet-io-wrappers-tests.html?sim=graphing-quadratics&ea before commits. On my working copy I’m seeing some failed tests for that.

Chris Malley [9:37 PM]
Failed tests for graphing-quadratics?

Sam Reid [9:38 PM]
Incomplete phet-io coverage, failing phet-io wrapper tests. Possibly related to the “required tandem” switch I flipped earlier today.
In my opinion, it would be better to push changes so I can help move things forward, even if Bayes CT is red for a while.
It sounds like AccordionBox was instrumented with the “instrument everything” approach and now we need to prune that back.

Chris Malley [9:39 PM]
Even it was pruned back… How does one omit things that are instrumented in common components?

Sam Reid [9:41 PM]
For example, if a common code component was instrumented by default and you didn’t want it to show up in the instrumentation? Ideally you would just not pass in a tandem to it. We don’t have logic for specifying which sub-components should not be instrumented--it’s all or none at the moment.

Chris Malley [9:42 PM]
I want to be able to change visibility (and only visibility) of the AccordionBoxes.
pushed graphing-quadratics.

Sam Reid [9:45 PM]
Got it, thanks

Chris Malley [9:45 PM]
QuadraticIO is a stub. I don’t know if it’s a sufficient stub. I was looking at IO types in masses-and-springs as examples.

Sam Reid [9:45 PM]
The remaining unit test fails look related to QuadraticIO, I’ll take a look.

I want to be able to change visibility (and only visibility) of the AccordionBoxes.

You mean of the +/- button only, or the corresponding title bar too?

Chris Malley [9:46 PM]
The entire AccordionBox. No access to its subcomponents.
Either the AccordionBox is visible or it’s not.

Sam Reid [9:48 PM]
I’m not too familiar with AccordionBox. Does that entail the expanded/collapsed panel as well? (edited)

Chris Malley [9:48 PM]
var accordionBox = new AccordionBox(…);
accordionBox.visible = false;
accordionBox.visible = true;
That is what we want to expose to the PhET-iO client, nothing more.
Same for NumberPickers.

Sam Reid [9:49 PM]
ok thanks, I’m not sure of the answer off the top of my head but will take a look

Chris Malley [9:49 PM]
… but enabled instead of visible.

[samreid]

When a NodeIO is instrumented, it gets all of the NodeIO features, which includes visibility, pickability and opacity. I don't think we should complicate things by sometimes saying certain nodes only have a subset of these options. When @zepumph and I addressed this for the PhET-iO button on the navigation bar, we created a custom IO type (so the PhET-iO logo can be made unpickable but not hidden), but this should be done rarely. So this issue should be for deciding what PhET-iO features Accordion Boxes should generally have, and hence will have for Graphing Quadratics in particular.

[samreid]

I removed a few more seemingly extraneous sub-components in the preceding commit. Now AccordionBox has:
titleNode (if not provided in options)
expandedProperty (if not provided in options)
expandCollapseButton

To me, that seems like a good level of granularity for now. Reassigned to @pixelzoom to see if this will be acceptable for Graphing Quadratics.

[pixelzoom]

This looks 1000% better. Thanks for handing. Closing.