Skip to content
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

Instrument inputEnabledProperty #101

Closed
2 tasks done
pixelzoom opened this issue Mar 13, 2021 · 7 comments
Closed
2 tasks done

Instrument inputEnabledProperty #101

pixelzoom opened this issue Mar 13, 2021 · 7 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Mar 13, 2021

Related to #85, and after phetsims/scenery#1116 has been completed...

Instrument inputEnabledProperty for the following Nodes:

  • DiatomicMoleculeNode

  • TriatomicMoleculeNode
    atomANode
    atomBNode
    atomCNode
    bondABNode
    bondBCNode

Discuss the plan for common-code UI components used by this sim:

  • HSlider
  • Checkbox
  • AquaRadioButtonGroup
  • ABSwitch
@zepumph
Copy link
Member

zepumph commented Mar 17, 2021

phetsims/scenery#1116 is complete and out for review. You can add the following option to instrument inputEnabledProperty for any Node:

inputEnabledPropertyPhetioInstrumented: true

This can be done in your sim for common code components also, but any changes needed inside common code will be done in phetsims/scenery#1158.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Mar 17, 2021

Here's a description of the sim-specific inputEnabledProperty elements that were added in the above commits.

The diatomic molecule has only one DragListener (for rotating), so requires only one element:

moleculePolarity.twoAtomsScreen.view.moleculeNode.inputEnabledProperty

The triatomic molecule has a DragListener for each atom and bond, and they can be individually controlled via these elements:

moleculePolarity.threeAtomsScreen.view.moleculeNode.atomANode.inputEnabledProperty
moleculePolarity.threeAtomsScreen.view.moleculeNode.atomBNode.inputEnabledProperty
moleculePolarity.threeAtomsScreen.view.moleculeNode.atomCNode.inputEnabledProperty

moleculePolarity.threeAtomsScreen.view.moleculeNode.bondABNode.inputEnabledProperty
moleculePolarity.threeAtomsScreen.view.moleculeNode.bondsBCNode.inputEnabledProperty

The triatomic molecule also has this element at the top-level. Setting it to false will disable interaction for all child elements of moleculeNode.

moleculePolarity.threeAtomsScreen.view.moleculeNode.inputEnabledProperty

@arouinfar @kathy-phet please review in master. Anything else for sim-specific inputEnabledProperty?

Next we should discuss what needs to be completed for common-code inputEnabledProperty. See list of UI components in #101 (comment).

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Mar 18, 2021

3/18/21 design meeting: @arouinfar @kathy-phet @pixelzoom

Need to turn off hint arrows for triatomic atoms that are inputEnabledProperty false.

@pixelzoom
Copy link
Contributor Author

Need to turn off hint arrows for triatomic atoms that are inputEnabledProperty false.

Done in the above commit. This was a little tricky because the visiblity of ech hint arrow involves 3 things: inputEnabledProperty for the molecule, inputEnabledProperty for the atom, and whether the user has changed the molecule.

@arouinfar ready for review.

@arouinfar
Copy link
Contributor

arouinfar commented Mar 18, 2021

@pixelzoom looks good on the Three Atoms screen, but I think we'll want to do the same for the Two Atoms screen and have moleculePolarity.twoAtomsScreen.view.moleculeNode.hintArrowsNode.visibleProperty listen to moleculePolarity.twoAtomsScreen.view.moleculeNode.inputEnabledProperty.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Mar 18, 2021

Ah right, the Two Atoms screen... duh.

Done in e76c992 and 43f497d. Please re-test the Three Atoms screen, because I made some improvements in the implementation there, informed by the Two Atoms implementation.

@arouinfar
Copy link
Contributor

Looks good in master @pixelzoom.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants