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

ISLCModel.forceProperty has a todo #38

Closed
jessegreenberg opened this issue Mar 1, 2018 · 6 comments
Closed

ISLCModel.forceProperty has a todo #38

jessegreenberg opened this issue Mar 1, 2018 · 6 comments
Assignees

Comments

@jessegreenberg
Copy link
Contributor

From #30, just marking this as an issue. units option for ForceProperty has this TODO:

units: 'Newtons' // TODO: this appears unused in instance-proxies

@mbarlow12 and/or @samreid does this need to be addressed?

@samreid
Copy link
Member

samreid commented Mar 2, 2018

This issue is caused by https://github.com/phetsims/phet-io/issues/1292 -- inability to specify units or other NumberProperty metadata for DerivedProperties. This doesn't need to be solved before 1.0, and I don't know how we'll proceed with https://github.com/phetsims/phet-io/issues/1292, but when I come up with a plan, I'll use this as a test case.

@jessegreenberg
Copy link
Contributor Author

Thanks @samreid, in that case marking as "deferred" for the 1.0 release.

@jessegreenberg
Copy link
Contributor Author

Linking this issue to the TODO.

@samreid
Copy link
Member

samreid commented Mar 3, 2018

Addressed above. Now the force appears in the PhET-iO Studio wrapper as a numeric read-only property with the appropriate units:

image

Also, this went through a medium-sized merge, @jessegreenberg or @mbarlow12 can you check that nothing is disturbed?

@samreid samreid assigned jessegreenberg and unassigned samreid Mar 3, 2018
samreid added a commit that referenced this issue Mar 3, 2018
@jessegreenberg
Copy link
Contributor Author

Thanks @samreid. Removing deferred label. @mbarlow12 could you review the change?

@mbarlow12
Copy link
Contributor

Reviewed. All sims seem to be running fine as well. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants