amrij / zonnepanelen

Website which shows telemetry data from the TCP traffic of SolarEdge PV inverters
GNU General Public License v3.0
12 stars 6 forks source link

Remove unused variables #46

Closed mirakels closed 5 years ago

jvanderzande commented 5 years ago

This PR can't be merged yet as one of the commit contains the removal of the graph reflows, which are required as far as I remember for proper sizing of the graphs in case the screen was resized since the start

mirakels commented 5 years ago

The reflow is also moved to the paneelFillSeries() function. Do you mean the redraw? Is that needed whe a reflow is already done?

jvanderzande commented 5 years ago

The reflow is also moved to the paneelFillSeries() function. Do you mean the redraw? Is that needed whe a reflow is already done?

mmm ... strange you remove it when not knowing for 100% whether it is needed or not. ;-) To my knowledge we are doing AddPoint() with the redraw parameter set to false, so would think one needs to fire the redraw operation when all points are added. The reflow only does an extra sizing operation as the redraw doesn't always make it properly fit to the defined DIV box. Either way: My vote goes to : "If it aint broke don't fix it" when not being 100% sure when it comes to these kind of things.

ps: You are of course right about the reflow just being moved. :)

mirakels commented 5 years ago

well, it was actually a mistake that a fogot to move the redraw ;) And based on your comment I wondered if it would be needed as it does not give problems on my side...

I;ll add a fix to include the redraw again.

mirakels commented 5 years ago

done

mirakels commented 5 years ago

Ah I see now thre are different animations. (chart and series). reverted that patch.

will look at redraw/reflow later.

mirakels commented 5 years ago

put reflow back to old location...