-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unfeature userControlledProperty #370
Comments
Since this is |
History
My opinions:
Perhaps best to call upon designers to decide how to proceed. |
@zepumph and I were wondering if we should leave it featured and change the documentation to: |
From design meeting:
|
Surprisingly, this patch was not enough to fix the behavior in the state wrapper when dragging a downstream block. What other parts may need to be guarded? Subject: [PATCH] Use blended values for gravity and buoyancy force in the force diagram, and increase threshold for showing values, see https://github.com/phetsims/density-buoyancy-common/issues/380
---
Index: js/common/model/Mass.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/Mass.ts b/js/common/model/Mass.ts
--- a/js/common/model/Mass.ts (revision 19bc7ee4eba57150a61f0918e9fa311aa79770e0)
+++ b/js/common/model/Mass.ts (date 1725149806328)
@@ -226,12 +226,18 @@
const tandem = options.tandem;
this.userControlledProperty = new BooleanProperty( false, {
tandem: tandem.createTandem( 'userControlledProperty' ),
- phetioDocumentation: 'For internal use only',
+ phetioDocumentation: 'Indicates whether the user is currently controlling the item',
phetioReadOnly: true,
phetioState: false,
phetioFeatured: true
} );
+ // If a user was dragging a Mass at the moment the state is set, it should no longer be user controlled.
+ phet.phetio.phetioEngine.phetioStateEngine.stateSetEmitter.addListener( () => {
+ this.userControlledProperty.reset();
+ this.interruptedEmitter.emit();
+ } );
+
this.inputEnabledProperty = new BooleanProperty( options.canMove, combineOptions<BooleanPropertyOptions>( {
tandem: options.canMove ? tandem.createTandem( 'inputEnabledProperty' ) : Tandem.OPT_OUT,
phetioDocumentation: 'Sets whether the element will have input enabled, and hence be interactive', |
Working on this. @samreid could you specify what is the desired downstream behavior is for this? Also, I pushed the phetioDocumentation part of the patch so we can easily focus on the other feature. |
Back to QA for double checking, this should be good now :) |
@AgustinVallejo Can I get some help on what I need to verify with the state wrapper? It isn't clear to me what I should do. |
Drag a block in the downstream simulation, when the state is automatically set it should release the downstream block and set it back to the upstream position. |
The objects all go back to match the upstream position, except for the boat, which continuously sinks if I move it to the bottom of the pool in the downstream sim. I think that will be addressed in #384, but would like dev confirmation on that.
EDIT: This has been updated in sparky (sorry I was looking at the wrong link) |
Thanks, I added the comment above to #384 and this issue seems good to close. Anyone please reopen for any reason. |
For phetsims/qa#1136, the
userControlledProperty
is described as "For Internal Use Only" but is currently featured.It makes sense to unfeature this property for all draggable items--blocks, shapes, scales, boat and bottle.
The text was updated successfully, but these errors were encountered: