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

Factor out airResistanceCheckbox and dragCoefficient readout to separate Node #225

Closed
zepumph opened this issue Feb 27, 2020 · 5 comments
Closed

Comments

@zepumph
Copy link
Member

zepumph commented Feb 27, 2020

From #219 this grouping will help factor out code from the intro and vectors screen while achieving phet-io instrumentation goals.

@zepumph
Copy link
Member Author

zepumph commented Feb 27, 2020

Review comment:

Group airResistanceCheckbox, airResistanceText, and dragCoefficientReadout into airResistanceControl or simply airResistance. (@kathy-phet do you have a preference for the name?)

@zepumph
Copy link
Member Author

zepumph commented Feb 27, 2020

I factored out these components in the above commit. I ended up naming the new control AirResistanceControl so that is what I named the phet-io element as well. Let me know if you would like to see something else.

Furthermore I simplified some of the children tandem names. I think it is a bit clearer, but please let me know if you would like them to be something else.

@arouinfar
Copy link
Contributor

@zepumph AirResistanceControl is great! One thing that is a little unclear is airResistanceControl.text. Could it be renamed title or titleNode (if it is indeed a Node)? Otherwise, everything looks good.

@zepumph
Copy link
Member Author

zepumph commented Sep 2, 2021

I went with titleText, since it is a Text (a subtype of Node). This seemed better to me because in addition to getting the visibleProperty from Node, it also has a textProperty which comes from Text, In general, I have found that this suffix can help describe what component parts the PhET-iO element has. What do you think?

@arouinfar
Copy link
Contributor

airResistanceControl.titleText.textProperty looks great, closing.

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

2 participants