phetsims / sun

User-interface components for PhET simulations, built on top of Scenery.
MIT License
4 stars 12 forks source link

public Emitters from input needed #701

Open zepumph opened 3 years ago

zepumph commented 3 years ago

While discussing https://github.com/phetsims/ratio-and-proportion/issues/380 with @jessegreenberg, @samreid, and @jonathanolson, we realized the need for being able to subscribe generally to when input occurs in common code, and not just when/how the underlying Property changes.

We looked at Checkbox in particular and came up with this patch.

Checkbox updates with a usage in Ratio and Proportion ```diff Index: sun/js/Checkbox.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/sun/js/Checkbox.js b/sun/js/Checkbox.js --- a/sun/js/Checkbox.js (revision 01b4965cda5f1cd1af68ec59297144e9b7e81f10) +++ b/sun/js/Checkbox.js (date 1621958535183) @@ -7,11 +7,13 @@ */ import Action from '../../axon/js/Action.js'; +import Emitter from '../../axon/js/Emitter.js'; import Property from '../../axon/js/Property.js'; import TinyProperty from '../../axon/js/TinyProperty.js'; import validate from '../../axon/js/validate.js'; import InstanceRegistry from '../../phet-core/js/documentation/InstanceRegistry.js'; import merge from '../../phet-core/js/merge.js'; +import SceneryEvent from '../../scenery/js/input/SceneryEvent.js'; import FireListener from '../../scenery/js/listeners/FireListener.js'; import Node from '../../scenery/js/nodes/Node.js'; import Rectangle from '../../scenery/js/nodes/Rectangle.js'; @@ -70,16 +72,18 @@ super(); + // @public - emits after the checkbox toggles + this.checkboxToggledEmitter = new Emitter( { + parameters: [ + { valueType: [ SceneryEvent, null ] }, + { valueType: 'boolean' }, + { valueType: 'boolean' } + ] + } ); + // @private - sends out notifications when the checkbox is toggled. - const toggleAction = new Action( () => { + const toggleAction = new Action( event => { property.value = !property.value; - validate( property.value, BOOLEAN_VALIDATOR ); - if ( property.value ) { - options.checkedSoundPlayer.play(); - } - else { - options.uncheckedSoundPlayer.play(); - } }, { parameters: [], tandem: options.tandem.createTandem( 'toggleAction' ), @@ -89,6 +93,16 @@ phetioEventType: EventType.USER } ); + this.checkboxToggledEmitter.addListener( () => { + validate( property.value, BOOLEAN_VALIDATOR ); + if ( property.value ) { + options.checkedSoundPlayer.play(); + } + else { + options.uncheckedSoundPlayer.play(); + } + } ); + // @private - Create the background. Until we are creating our own shapes, just put a rectangle behind the font // awesome checkbox icons. this.backgroundNode = new Rectangle( 0, -options.boxWidth, options.boxWidth * 0.95, options.boxWidth * 0.95, @@ -125,7 +139,7 @@ // interactivity const fireListener = new FireListener( { - fire: () => toggleAction.execute(), + fire: event => toggleAction.execute( event ), tandem: options.tandem.createTandem( 'fireListener' ) } ); this.addInputListener( fireListener ); Index: ratio-and-proportion/js/create/view/CreateScreenView.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/ratio-and-proportion/js/create/view/CreateScreenView.js b/ratio-and-proportion/js/create/view/CreateScreenView.js --- a/ratio-and-proportion/js/create/view/CreateScreenView.js (revision 20b3f370f28da93fce90479345606afceefbe76d) +++ b/ratio-and-proportion/js/create/view/CreateScreenView.js (date 1621957373258) @@ -5,11 +5,9 @@ */ import Property from '../../../../axon/js/Property.js'; -import FireListener from '../../../../scenery/js/listeners/FireListener.js'; import Node from '../../../../scenery/js/nodes/Node.js'; import RichText from '../../../../scenery/js/nodes/RichText.js'; import Checkbox from '../../../../sun/js/Checkbox.js'; -import Tandem from '../../../../tandem/js/Tandem.js'; import ActivationUtterance from '../../../../utterance-queue/js/ActivationUtterance.js'; import RAPColorProfile from '../../common/view/RAPColorProfile.js'; import RAPScreenView from '../../common/view/RAPScreenView.js'; @@ -71,18 +69,11 @@ lockRatioCheckbox.touchArea = lockRatioCheckbox.localBounds.dilatedXY( 8, 0.5 * lockRatioCheckbox.height ); lockRatioCheckbox.mouseArea = lockRatioCheckbox.localBounds.dilatedXY( 8, 0.5 * lockRatioCheckbox.height ); - // TODO: this should not be a separate FireListener. Instead we should be able to use the checkbox somehow. https://github.com/phetsims/ratio-and-proportion/issues/227 - lockRatioCheckbox.addInputListener( new FireListener( { - attach: false, // Since this is the second PressListener to be added to the checkbox (so annoying) - fire: () => { - ratioLockedUtterance.alert = model.ratio.lockedProperty.value ? ratioAndProportionStrings.a11y.lockRatioCheckboxContextResponse : - ratioAndProportionStrings.a11y.ratioNoLongerLocked; - phet.joist.sim.utteranceQueue.addToBack( ratioLockedUtterance ); - }, - - // phet-io - tandem: Tandem.OPT_OUT - } ) ); + lockRatioCheckbox.checkboxToggledEmitter.addListener( () => { + ratioLockedUtterance.alert = model.ratio.lockedProperty.value ? ratioAndProportionStrings.a11y.lockRatioCheckboxContextResponse : + ratioAndProportionStrings.a11y.ratioNoLongerLocked; + phet.joist.sim.utteranceQueue.addToBack( ratioLockedUtterance ); + } ); // The "lock ratio" checkbox should not be enabled when the ratio is not in proportion. Property.multilink( [ ```

Some points of discussion:

zepumph commented 3 years ago

This is just high priority for me, not for the project.

zepumph commented 2 years ago

Over in https://github.com/phetsims/sun/commit/6afc6a986c4988c858dd3fc75360a33137103085 it was easiest to just add the specific feature to Checkbox instead of having to add a custom listener in each usage. I can see that being the case. Perhaps this strategy can be used for less foundational items (not for Checkbox/Button/RadioButton/ComboBox etc).

zepumph commented 1 year ago

Great progress today on this in the above issues. I'm very excited about this!

chrisklus commented 1 year ago

We had great success over in https://github.com/phetsims/number-suite-common/issues/60 from changes in the tagged issues above, yay!