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

instrumentation of microScreen.model.pHMeter #137

Closed
pixelzoom opened this issue Mar 7, 2020 · 2 comments
Closed

instrumentation of microScreen.model.pHMeter #137

pixelzoom opened this issue Mar 7, 2020 · 2 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Mar 7, 2020

Something I noticed while working on another issue...

Instrumentation of the pH meter mode in Macro screen looks like this:

-phScale
  -microScreen
    -model
      -pHMeter
        valueProperty
        -probe
          positionProperty

There are a few problems here:

  • valueProperty is never updated, always null
  • this meter doesn't have a separate movable probe, so both probe and positionProperty are irrelevant
  • it's created in the model, but not actually used in the view

Like the My Solutions screen, there's no need for microScreen.model.pHMeter -- it has no moving parts, and its pH is always the same as the solutions. So it should be removed.

@pixelzoom
Copy link
Contributor Author

microScreen.model.pHMeter has been removed. @arouinfar please review in master, let me know if you have any questions about this.

@pixelzoom pixelzoom assigned arouinfar and unassigned pixelzoom Mar 9, 2020
@pixelzoom pixelzoom reopened this Mar 9, 2020
pixelzoom added a commit that referenced this issue Mar 9, 2020
@arouinfar
Copy link
Contributor

That all sounds reasonable to me @pixelzoom, and it looks good in master.

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