phetsims / circuit-construction-kit-common

"Circuit Construction Kit: Basics" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
10 stars 10 forks source link

advancedAccordionBox - sourceResistanceControl and wireRestivityControl title labels #948

Closed matthew-blackman closed 1 year ago

matthew-blackman commented 1 year ago

Found in tree review - https://github.com/phetsims/circuit-construction-kit-common/issues/917

@arouinfar @samreid and I agreed that the advancedAccordionBox slider controls do not need to have their slider visibleProperties independently instrumented, as the text should always share the same visible state as its associated slider.

Additionally, advancedAccordionBox sourceResistanceControl and wireResistivityControl should have titleText instrumented.

samreid commented 1 year ago

I reviewed the code and tested studio. Everything seems good. I observed that instrumenting circuitConstructionKitDc.labScreen.view.advancedAccordionBox.sourceResistanceControl.maxLabelText.titleText doesn't really do anything. It is not customizable like "tiny". So perhaps I'll try to uninstrument that.

samreid commented 1 year ago

On the contrary! There are some strings that need to use PatternStringProperty so their components can be discovered. And we need to eliminate an extraneous layer of 'titleNode` in the tree.

samreid commented 1 year ago

Fixed in the commits. Some "extreme" tandems sneaked into the api changes. @matthew-blackman can you please review? If it seems pretty safe, feel free to close. Also feel free to check the tree changes with @arouinfar as needed.