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

Create CorrespondingViewIO? Or Maybe GroupMemeberIO? #106

Closed
zepumph opened this issue Sep 27, 2019 · 2 comments
Closed

Create CorrespondingViewIO? Or Maybe GroupMemeberIO? #106

zepumph opened this issue Sep 27, 2019 · 2 comments

Comments

@zepumph
Copy link
Member

zepumph commented Sep 27, 2019

from phetsims/charges-and-fields#170, we had for a time ModelElementNodeIO (https://github.com/phetsims/charges-and-fields/blob/b7f5214eb6091e7da3af26fe0748939e1229ec52/js/charges-and-fields/view/ModelElementNodeIO.js), This held a reference to the model element, though it wasn't used by anything, and there was no reason to have it in state for supporting state.

In that file, there was a modelPhetioID. If we feel like this is valuable, then perhaps we can change the ReferenceIOs in CAF to use this new type. If we don't think that this level of granularity for the view-specific IO type is needed, then maybe just a GroupMemberIO. Something more specific than ReferenceIO, which really doesn't belong here, it just happened to work. Note that ObjectIO doesn't work because these types can't be structured cloned with PostMessage.

@samreid what do you think we should do. Create CorrespondingViewIO? Create GroupMemberIO? Keep ReferenceIO? Wild Card?

@samreid
Copy link
Member

samreid commented Jan 17, 2020

I don't see the advantage of developing something like ReferenceIO but with a modelPhetioID, but I don't really understand the role of ReferenceIO in charges and fields. For instance, ElectricFieldSensorNode has phetioType: ReferenceIO. Should we instead have put phetioState: false and used phetioType: NodeIO which is the default for that type?

Also, I'm unclear how the introduction PhetioGroup may help or clarify things for this case.

@zepumph can you please re-evaluate this issue now that we're further along?

@samreid samreid assigned zepumph and unassigned samreid Jan 17, 2020
@zepumph
Copy link
Member Author

zepumph commented Jan 24, 2020

Yes I agree. I don't think that this common IO type makes sense to create. I don't think that it adequately fits into how we are using the upside-down "U" strategy for implement dynamic state. I'm going to close.

@zepumph zepumph closed this as completed Jan 24, 2020
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