Closed zepumph closed 4 years ago
I think the second bullet is addressed in #154.
The first bullet I believe has been fixed when I refactored the toIndex functions in #173 (see 22ae634bb7c09c9bf950846fc40b8e0c68a46d43). I think all of the additional region business was rendered unnecessary with the most recent set of design changes, so I removed it. @zepumph assigning over to you to review.
Looks great, see you in that other issue. Closing
In ConcentrationDescriber.solidsToIndex there is this comment:
This seems very confusing to me. Why is this unnamed region defined here instead of in the regions list? Why does it take up the same amount of range as all others? ". . .right after saturation point. . . "-- how is this defined, as it seems like the unnamed region isn't tied to any particular saturation value?
MolarityModel has a
isSaturated
function that is based solely on how much precipitate there is. I think I recall hearing something recently that the a11y team has discovered this may not be as closely tied, as it is possible for it to be saturated without yet creating any precipitate. Let's talk to see if the model needs to be reworked, as it seems like the model'sisSaturated
function should be the ground truth in this case.