terezka / elm-charts

Create SVG charts in Elm.

https://www.elm-charts.org

BSD 3-Clause "New" or "Revised" License

711 stars 67 forks source link

Adjust fill for the appropriate minimum #72

Closed joshuaclayton closed 3 years ago

joshuaclayton commented 7 years ago

What?

This updates Plot.addNiceReachForArea to recalculate the minimum y using closestToZero instead of mining against 0. This ensures that the minimum isn't artificially low relative to the provided data.

Before:

analysis__t01_-_2017-01-01

After:

analysis__t01_-_2017-01-01

joshuaclayton commented 7 years ago

One thing I ran into (and it seems Travis is reporting the same) is that I wasn't able to get the tests to run. I'm also happy to add tests if you feel it's valuable for this, but it seems like a fairly straightforward fix!

joshuaclayton commented 6 years ago

Good afternoon @terezka! Just wanted to check in on this; is there anything else I can do to help make this easier to merge for you? I'm happy to help!

joshuaclayton commented 6 years ago

As it turns out, this runs into issues when the value runs below zero (e.g. a chart that travels from -5 to 5). This isn't quite ready to be merged, although it does address the issue I displayed above!

terezka commented 6 years ago

Hi! I see what you mean. Let me know when you think it's done and I'll merge it :)