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

Feature rootsNode.*CoordinatesProperty #194

Closed
arouinfar opened this issue Jun 20, 2023 · 3 comments
Closed

Feature rootsNode.*CoordinatesProperty #194

arouinfar opened this issue Jun 20, 2023 · 3 comments

Comments

@arouinfar
Copy link

For #180

The quadraticProperty contains everything one might want to know about the quadratic except for the roots. Knowing the coordinates of roots seems like useful data, so I think it would be nice to feature rootsNode.*CoordinatesProperty.

I looked through the repo and notes docs to see if this had been considered before but didn't see anything. @pixelzoom if you know of a reason why these were omitted, please let me know. Otherwise, let's feature rootsNode.*CoordinatesProperty.

@pixelzoom
Copy link
Contributor

I don't recall this being considered before. I went ahead and featured rootsNode.leftCoordinatesProperty and rootsNode.rightCoordinatesProperty in the above commits.

But there's another possibility, which may be a better UX... We could add the roots to QuadraticIO, so that they appear in the state object for the value of quadraticProperty. @arouinfar thoughts?

@pixelzoom
Copy link
Contributor

pixelzoom commented Jun 21, 2023

... We could add the roots to QuadraticIO, so that they appear in the state object for the value of quadraticProperty.

@arouinfar and I discussed this. We're not going to do this because (1) no one has asked for it, (2) the roots are already in the data stream, (3) roots are kind of odd, see #146, and (4) adding roots to state will take a non-trivial amount of time, and likely involved other PhET-iO developers. If someone asked for this in the future, we can add it then.

So... @arouinfar please review rootsNode.leftCoordinatesProperty and rootsNode.rightCoordinatesProperty. Close if OK.

@pixelzoom pixelzoom removed their assignment Jun 21, 2023
@arouinfar
Copy link
Author

Thanks for summarizing @pixelzoom.

So... @arouinfar please review rootsNode.leftCoordinatesProperty and rootsNode.rightCoordinatesProperty. Close if OK.

Looks good, closing.

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