tickSize Not Updated On Subsequent Calls To setupGrid #860

Closed
thecountofzero opened this Issue Nov 11, 2012 · 7 comments

Comments

Projects
None yet
4 participants
Contributor

thecountofzero commented Nov 11, 2012

This issue comes from a discussion on the forum

https://groups.google.com/forum/?fromgroups=#!topic/flot-graphs/SxAo58eMQgg

In a graph that is updated using:

setData()
setupGrid()
draw()

The y-axis scale (tickSize) is not recalculated. This can result in a very skewed (ugly) graph if the new data set contains a y-axis value much greater than the original set used to initialize the graph (and tickSize).

After the graph has been initialized, subsequent calls to setupGrid do not recalculate "tickSize"

This is because after initializing the graph, "axis.tickGenerator" exists and line 1239 evaluates to false.

https://github.com/flot/flot/blob/master/jquery.flot.js#L1239

If I change that line to read:

if(opts.mode !== 'time') {

Then it recalculates the "tickSize" and my problem goes away.

Perhaps you can move the code that calculates the tickSize into axis.tickGenerator? This appears to be how it's done in the time plugin.

@thecountofzero thecountofzero added a commit to thecountofzero/flot that referenced this issue Nov 11, 2012

@thecountofzero thecountofzero Fix for issue #860, Recalculate tickSize on updates a0b1552
Contributor

ralphholzmann commented Dec 19, 2012

This pull request fixed my issue. Great job, and hello @thecountofzero :)

This should be merged in asap.

Contributor

thecountofzero commented Dec 22, 2012

Boom. Confirmation. Merge it.

What up @ralphholzmann !

jerzyk commented Jan 12, 2013

any estimates when this patch will be merged into core?

Owner

dnschnur commented Jan 12, 2013

After we release 0.8.

Contributor

ralphholzmann commented Jan 12, 2013

@dnschnur Why after? This seems like a critical bug fix to merge in after a point release.

Contributor

thecountofzero commented Jan 12, 2013

I'd have to agree with @ralphholzmann

Owner

dnschnur commented Jan 12, 2013

Because at some point we need to actually release. That said, you're right that it's higher priority than most, so if I have some time this evening or tomorrow afternoon then I'll do it.

dnschnur was assigned Jan 13, 2013

dnschnur closed this Jan 13, 2013

@dnschnur dnschnur added a commit that referenced this issue Apr 6, 2013

@dnschnur dnschnur Always recalculate tickDecimals and tickSize.
Resolves #1000.  In Flot 0.7 we only calculated tickDecimals and
tickSize once, when creating the tickGenerator for the first time.  This
meant that calls to setupGrid failed to recalculate the values, as
reported in #860.  #861 fixed the problem by moving calculation into
tickGenerator, but this turned out to cause a new problem, since the
function doesn't run when an explicit ticks array is provided.

This commit solves both by always recalculating the values outside of
the tickGenerator function.  As far as I can tell the only reason it
wasn't done this way from the beginning was to avoid unnecessary work in
the case where tickGenerator is already provided in the options.  But
the extra work is negligible, and it's actually more consistent for the
properties to always be set.
209fe53

@dnschnur dnschnur added a commit to dnschnur/flot that referenced this issue Apr 6, 2013

@dnschnur dnschnur Always recalculate tickDecimals and tickSize.
Resolves #1000.  In Flot 0.7 we only calculated tickDecimals and
tickSize once, when creating the tickGenerator for the first time.  This
meant that calls to setupGrid failed to recalculate the values, as
reported in #860.  #861 fixed the problem by moving calculation into
tickGenerator, but this turned out to cause a new problem, since the
function doesn't run when an explicit ticks array is provided.

This commit solves both by always recalculating the values outside of
the tickGenerator function.  As far as I can tell the only reason it
wasn't done this way from the beginning was to avoid unnecessary work in
the case where tickGenerator is already provided in the options.  But
the extra work is negligible, and it's actually more consistent for the
properties to always be set.
c4672fa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment