search

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

Visibility of fuseRepairButton and batteryReverseButton gets overridden #956

Closed arouinfar closed 1 year ago

arouinfar commented 1 year ago

In https://github.com/phetsims/circuit-construction-kit-common/issues/917#issuecomment-1419514579 we discovered that the fuseRepairButton.visibleProperty was read-only and we wanted to make it writable. @matthew-blackman made the change, but it revealed the reason why it was probably read-only in the first place.

The model sets fuseRepairButton.visibleProperty every time a fuse is selected/deselected. Setting the visibleProperty to false will only hide button until the next time a fuse is selected (assuming isRepairableProperty: true).

The same is also true of batteryReverseButton.visibleProperty. Each time a battery with isReversibleProperty: true is selected, the model will set visibleProperty: true.

Interestingly, trashButton.visibleProperty does not exhibit this same behavior. If a client hides the trash button, it stays hidden. For consistency and convenience, it would be nice if we could do the same with fuseRepairButton.visibleProperty and batteryReverseButton.visibleProperty.

Clients could technically use isRepairableProperty and isReversibleProperty to hide these buttons, but doing it for every single fuse and battery that students create is a real drag. It would be simpler to have global control over the button's visibility.

samreid commented 1 year ago

We added a 2nd gate for the fuse, still need to do the batteries.

samreid commented 1 year ago

@matthew-blackman and I addressed both cases, thanks for discovering this. Ready for review, and please close if all is well.

arouinfar commented 1 year ago

Thanks, looks good!