Skip to content

fix(scale): make time getValueForPixel the inverse of getPixelForValue - #12289

Open
contactjawad wants to merge 1 commit into
chartjs:masterfrom
contactjawad:fix-time-getvalueforpixel-offset
Open

fix(scale): make time getValueForPixel the inverse of getPixelForValue#12289
contactjawad wants to merge 1 commit into
chartjs:masterfrom
contactjawad:fix-time-getvalueforpixel-offset

Conversation

@contactjawad

Copy link
Copy Markdown

What

TimeScale.getValueForPixel (and TimeSeriesScale.getValueForPixel) is meant to be the exact inverse of getPixelForValue, but it subtracts the wrong margin offset. On a time axis with offset: true and irregularly spaced ticks — where offsets.start !== offsets.end — converting a pixel back to a value is wrong by a constant, so hover/tooltip/nearest-element interaction maps to the wrong data.

Why

getPixelForValue maps a value to a decimal using offsets.start:

return this.getPixelForDecimal((offsets.start + pos) * offsets.factor);

so the inverse must be pos = decimal / factor - offsets.start. Instead the code used offsets.end:

const pos = this.getDecimalForPixel(pixel) / offsets.factor - offsets.end;

When the two margins differ, the result is off by (start - end) * (max - min).

How

Subtract offsets.start in both src/scales/scale.time.js and src/scales/scale.timeseries.js, making getValueForPixel the exact inverse of getPixelForValue (which uses offsets.start).

Test

Added a spec in test/specs/scale.time.tests.js that builds a time axis with offset: true and irregularly spaced labels so offsets.start !== offsets.end (the test asserts this premise), then verifies the round trip getValueForPixel(getPixelForValue(v)) returns v. Before the fix the round trip is off by ~21 hours for the chosen data; after the fix it matches.

getPixelForValue maps a value using offsets.start, so the pixel->value
inverse must subtract offsets.start, not offsets.end. On an offset axis
with asymmetric margins (offsets.start !== offsets.end) the conversion was
wrong by a constant (start - end) * (max - min), so hover/tooltip lookups
mapped to the wrong value. Apply the same correction to the timeseries
scale.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant