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

Updates to phetioFeatured #345

Closed
11 tasks done
arouinfar opened this issue Jul 28, 2023 · 2 comments
Closed
11 tasks done

Updates to phetioFeatured #345

arouinfar opened this issue Jul 28, 2023 · 2 comments

Comments

@arouinfar
Copy link

arouinfar commented Jul 28, 2023

For #323

Due to changes made in https://github.com/phetsims/studio/issues/303 and https://github.com/phetsims/studio/issues/309, there are some additional elements we should make phetioFeatured: true so linked elements properly show up in the Featured tree.

  • view.graphs.pedigreeNode.pedigreeGraphNode
  • model.generationClock
  • view.graphs.populationNode.populationModel <= This is a linked element, so model.graphs.populationModel is actually featured.
  • view.graphs.populationNode.populationGraphNode.dataProbeNode
  • model.graphs.populationModel.dataProbe
  • model.graphs.proportionsModel

For consistency, I would also expect to see these things featured, and was surprised to find they weren't. @pixelzoom feel free to veto if you disagree.

  • view.addMutationsPanel.visibleProperty
  • view.graphs.populationNode.populationGraphNode.yZoomButtonGroup.visibleProperty
  • model.graphs.populationModel.yZoomLevelProperty

I've requested that these elements be made phetioReadOnly: true in #344, so I recommend that we also make them phetioFeatured: false

@pixelzoom
Copy link
Contributor

pixelzoom commented Jul 28, 2023

Done in the commits below. There were API changes, no migration rules.

A couple of deviations from what was requested:

  • view.graphs.populationNode.populationModel is a linked element. So the element that is featured is actually model.graphs.populationModel.
  • view.graphs.pedigreeNode.allelesPanel.*Row.checkbox.visibleProperty are no longer instrumented, so there's nothing to do for this.

@arouinfar please review. Close if OK.

@arouinfar
Copy link
Author

Looks good @pixelzoom, thanks!

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