-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add suffix "RadioButton" to individual radio buttons #334
Comments
Update: This also involves the items in combo boxes, which will impact the thermometer node. |
I handled this in phetsims/sun#615. @arouinfar I fixed up the client guide too, please review. @jbphet said:
The items in the thermometer combo box are not instrumented. |
Relevant cherry-picks: state-of-matter: 1e88872 phet-io-client-guides: https://github.com/phetsims/phet-io-client-guides/commit/d90b0b16c09b99f9cc3c0ecad0d2b3059f2e8850 sun: phetsims/sun@187ac98, phetsims/sun@55c4fc0, phetsims/sun@ce77c42 |
…s (previous one was incorrect), see phetsims/states-of-matter#334
I have cherry-picked the commits for SOM and the client guide to the 1.2 release branch. As for the common code, I only grabbed the one for radio button names, since the combo box changes are not particularly relevant. I updated dependencies in SOM, SOMB, and AI, and tested the release branch locally for each. I think this is ready for deployment. |
@jbphet I added the "RadioButton" suffix to the relevant phetioIDs in SOMB's client-requests.md in the commit above. It will need to be cherry-picked into SOMB. |
Looks good in phetsims/qa/issues/531 and phetsims/qa/issues/532. 🥳 |
In the 8/13/2020 phet-io meeting we decided that radio buttons should have the suffix "RadioButton" in their tandem names. I had previous not done this because I felt that the fact that the elements were in a radio button group provided sufficient context, but since these items can be sometimes used in isolation, have a more complete name is useful.
There is a global issue that is logged in phetsims/sun#615 where an assert will be added to check this, but I don't think it's important to move this to the current 1.2 RC branch. I'll just make the change to the tandems and leave it at that.
For historical reference, the discussion that ultimately motivated this decision originated from a discussion about #331, but that issue is more about a discrepancy between the client guide and the tandems that are available in the sim.
For my own reference, here is a list of what needs to be done to make this a reality:
The text was updated successfully, but these errors were encountered: