search

esnet / react-timeseries-charts

Declarative and modular timeseries charting components for React
http://software.es.net/react-timeseries-charts
Other
861 stars 284 forks source link

YAxis Component now rerenders if min/max changes #348

Closed llaver closed 5 years ago

llaver commented 5 years ago

Currently the YAxis components shouldComponentUpdate function returns false at all times.

This will change that function to return true if either min or max has been updated, causing the YAxis component to rerender.

Fixes issue #346

d3 does not lose control of the component when it is rerendered as previously thought, so this works just fine. Unless I'm missing something obscure that is breaking.

Sorry for the slowness of the gif, that's my fault.

screen recording 2018-12-13 at 9 48 47 am

llaver commented 5 years ago

It's been about a month since I made this update in my project. Have had no side affects or issues caused by this change.

pjm17971 commented 5 years ago

It seems that adding max and min to this conditional would solve this problem, in the same way as other props changes update: https://github.com/esnet/react-timeseries-charts/pull/348/files#diff-b58d4445dea66d6c044380b8a657c118R133

What I'm mostly wondering though is why the existing conditional didn't catch that (i.e. why the scale check didn't cause a rescale of the axis. It might be helpful to understand that.