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

ABSwitch phetioFeatured defaults #572

Closed
arouinfar opened this issue Feb 28, 2020 · 2 comments
Closed

ABSwitch phetioFeatured defaults #572

arouinfar opened this issue Feb 28, 2020 · 2 comments

Comments

@arouinfar
Copy link
Contributor

In phetsims/ph-scale#117 @kathy-phet and I reviewed the ABSwitch, and decided on the general pattern for what should be featured.

ABSwitch.visibleProperty
ABSwitch.AText.textProperty
ABSwitch.AText.visibleProperty
ABSwitch.BText.textProperty
ABSwitch.BText.visibleProperty
ABSwtich.toggleSwitch.visibleProperty

Assigning to the PhET-iO team.

@zepumph
Copy link
Member

zepumph commented Feb 28, 2020

ABSwtich.toggleSwitch.visibleProperty
ABSwitch.visibleProperty

Implemented above. These were easy because we create these in ABSwitch

ABSwitch.AText.textProperty
ABSwitch.AText.visibleProperty
ABSwitch.BText.textProperty
ABSwitch.BText.visibleProperty

This is going to be much more difficult, because these use "dependency injection" such that they are created, and then passed to ABSwitch. Things to note. ABSwitch allows these to be any Node, it is only ph-scale specifically that is passing texts in. Thus likely you should set the textProperties in the overrides (or put them in ph-scale code).

The best we can do for the visibileProperty features is assert that they are featured when passed into ABSwitch, but it will still be on the client of ABSwitch to implement. Do you think that is preferrable to designers supplying each time in the overrides?

@arouinfar
Copy link
Contributor Author

ABSwtich.toggleSwitch.visibleProperty
ABSwitch.visibleProperty

Implemented above. These were easy because we create these in ABSwitch

👍

ABSwitch.AText.textProperty
ABSwitch.AText.visibleProperty
ABSwitch.BText.textProperty
ABSwitch.BText.visibleProperty

This is going to be much more difficult, because these use "dependency injection" such that they are created, and then passed to ABSwitch. Things to note. ABSwitch allows these to be any Node, it is only ph-scale specifically that is passing texts in. Thus likely you should set the textProperties in the overrides (or put them in ph-scale code).

The best we can do for the visibileProperty features is assert that they are featured when passed into ABSwitch, but it will still be on the client of ABSwitch to implement. Do you think that is preferrable to designers supplying each time in the overrides?

Thanks for the clarification @zepumph! I think it would be best to handle these in the overrides, since it isn't a general pattern for all ABSwitches.

I don't think there's anything else that needs to be done for this issue, so I'll go ahead and close.

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