Apply the chart cursor correction exactly once
uPlot writes what `cursor.move` returns back into the value it hands in next time and calls it again on every redraw — and a chart sets its data on every render. The correction was therefore applied repeatedly: the cursor walked left while the pointer stood still, and the position taken at mousedown no longer matched the one held at mouseup, which uPlot reads as a drag and answers by swallowing the click. That is what stopped a chart tile being selectable in the dashboard editor. The refiner now returns its previous answer unchanged when handed it back, so a redraw is a no-op. Lifted out of the config as `cursorRefiner` so the branch can be checked without a browser; `cursor.check.ts` replays uPlot's own sequences, including the press-is-not-a-drag one that regressed. The Playwright hover test now also asserts the cursor holds its place across a redraw, which is what the first version missed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ZeGnqVsf5VHQqvz4HdUhN
This commit is contained in:
@@ -59,6 +59,35 @@ export function slotFor(index: number, palette: string[] = DEFAULT_PALETTE) {
|
|||||||
return slots[index % slots.length]
|
return slots[index % slots.length]
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The pointer, put back into layout pixels on a CSS-scaled panel.
|
||||||
|
*
|
||||||
|
* A dashboard canvas is drawn at its panel's own pixel size and CSS-scaled to
|
||||||
|
* fit the screen, while uPlot maps the pointer with `clientX - rect.left` —
|
||||||
|
* visual pixels — against its own unscaled plot width. On a scaled panel the
|
||||||
|
* cursor then drifts further right the further into the chart it goes.
|
||||||
|
* `drawn` is the ratio the element is actually painted at; unscaled it is 1
|
||||||
|
* and every call here is a no-op.
|
||||||
|
*
|
||||||
|
* Applied exactly once per position, which is the whole trick. uPlot writes
|
||||||
|
* what this returns back into the value it hands in next time and calls it
|
||||||
|
* again on every redraw — and a chart redraws on every render. Dividing twice
|
||||||
|
* would walk the cursor left while the pointer stood still, and would leave
|
||||||
|
* the position taken at mousedown disagreeing with the one at mouseup, which
|
||||||
|
* uPlot reads as a drag: it then swallows the click, and a chart in the
|
||||||
|
* dashboard editor cannot be selected at all.
|
||||||
|
*/
|
||||||
|
export function cursorRefiner() {
|
||||||
|
let placed: [number, number] = [-10, -10]
|
||||||
|
return (drawn: number, left: number, top: number): [number, number] => {
|
||||||
|
if (!(drawn > 0) || drawn === 1) return [left, top]
|
||||||
|
// Handed back what it was last given: already in layout pixels.
|
||||||
|
if (left === placed[0] && top === placed[1]) return placed
|
||||||
|
placed = [left / drawn, top / drawn]
|
||||||
|
return placed
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/** Room for the axis ticks; uPlot measures the rest of the box itself. */
|
/** Room for the axis ticks; uPlot measures the rest of the box itself. */
|
||||||
const PADDING: uPlot.Padding = [10, 12, 0, 0]
|
const PADDING: uPlot.Padding = [10, 12, 0, 0]
|
||||||
|
|
||||||
@@ -193,6 +222,7 @@ export function UplotChart({
|
|||||||
/** The x value the page was last told about, so a move within one bucket
|
/** The x value the page was last told about, so a move within one bucket
|
||||||
* does not re-render it. */
|
* does not re-render it. */
|
||||||
let told: number | null = null
|
let told: number | null = null
|
||||||
|
const refine = cursorRefiner()
|
||||||
// One builder for the whole chart: it is a factory, and the series only
|
// One builder for the whole chart: it is a factory, and the series only
|
||||||
// need the function it returns.
|
// need the function it returns.
|
||||||
const spline = smooth ? { paths: uPlot.paths.spline?.() } : {}
|
const spline = smooth ? { paths: uPlot.paths.spline?.() } : {}
|
||||||
@@ -206,18 +236,10 @@ export function UplotChart({
|
|||||||
padding: PADDING,
|
padding: PADDING,
|
||||||
cursor: {
|
cursor: {
|
||||||
y: false,
|
y: false,
|
||||||
// A dashboard canvas is CSS-scaled to fit its panel, and uPlot maps
|
// See `cursorRefiner`: the panel is scaled, uPlot's pointer maths
|
||||||
// the pointer with `clientX - rect.left` — visual pixels — against
|
// is not, and this must run exactly once per position.
|
||||||
// its own unscaled plot width. On a scaled panel the cursor then
|
move: (self, left, top) =>
|
||||||
// drifts further right the further in it goes. Dividing by the ratio
|
refine(self.rect.width / self.over.clientWidth, left, top),
|
||||||
// the element is actually drawn at puts it back in layout pixels;
|
|
||||||
// unscaled, the ratio is 1 and this is a no-op.
|
|
||||||
move: (self, left, top) => {
|
|
||||||
const drawn = self.rect.width / self.over.clientWidth
|
|
||||||
return drawn > 0 && drawn !== 1
|
|
||||||
? [left / drawn, top / drawn]
|
|
||||||
: [left, top]
|
|
||||||
},
|
|
||||||
},
|
},
|
||||||
legend: {
|
legend: {
|
||||||
live: true,
|
live: true,
|
||||||
|
|||||||
@@ -0,0 +1,81 @@
|
|||||||
|
/**
|
||||||
|
* The cursor refiner, checked.
|
||||||
|
*
|
||||||
|
* ponytail: a script rather than a suite, like the two beside `ColorWidget` —
|
||||||
|
* the frontend's only runner is Playwright and this is arithmetic:
|
||||||
|
*
|
||||||
|
* cd frontend && bun run src/components/Common/cursor.check.ts
|
||||||
|
*
|
||||||
|
* Written because the first version of this shipped a regression Playwright
|
||||||
|
* only caught two tests later. uPlot hands `cursor.move` its own output back
|
||||||
|
* and calls it again on every redraw, so refining a second time walked the
|
||||||
|
* cursor left and — worse — made a still pointer look like a drag, which uPlot
|
||||||
|
* answers by swallowing the click. The sequences below are uPlot's own:
|
||||||
|
* `mouseLeft1` is set raw from the event, refined in place by `updateCursor`,
|
||||||
|
* and then fed back in unchanged by every later redraw.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import assert from "node:assert/strict"
|
||||||
|
|
||||||
|
import { cursorRefiner } from "./UplotChart"
|
||||||
|
|
||||||
|
/** What a panel scaled to fit a 1280x720 window is drawn at. */
|
||||||
|
const DRAWN = 0.64
|
||||||
|
|
||||||
|
// Unscaled — Health and Home — is untouched, whatever the sequence.
|
||||||
|
{
|
||||||
|
const refine = cursorRefiner()
|
||||||
|
for (const at of [0, 12, 300, -10]) {
|
||||||
|
assert.deepEqual(refine(1, at, at), [at, at], `${at} is left alone at 1:1`)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// One pointer move, then the redraws a live dashboard does on every render.
|
||||||
|
// The position must not move while the pointer does not.
|
||||||
|
{
|
||||||
|
const refine = cursorRefiner()
|
||||||
|
const [left, top] = refine(DRAWN, 128, 64)
|
||||||
|
assert.equal(left, 200, "128 visual pixels is 200 layout pixels at 0.64")
|
||||||
|
assert.equal(top, 100)
|
||||||
|
for (let redraw = 0; redraw < 25; redraw++) {
|
||||||
|
const again = refine(DRAWN, left, top)
|
||||||
|
assert.deepEqual(again, [left, top], `redraw ${redraw} moved the cursor`)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// What a click is, in uPlot's own terms: `mouseLeft1` follows the move and
|
||||||
|
// every redraw after it, `mouseLeft0` is refined from the raw press on its
|
||||||
|
// own, and uPlot calls it a drag when the two disagree. They must not, or the
|
||||||
|
// click is swallowed and the dashboard never hears it.
|
||||||
|
{
|
||||||
|
const refine = cursorRefiner()
|
||||||
|
let [left1, top1] = refine(DRAWN, 128, 64)
|
||||||
|
for (let redraw = 0; redraw < 3; redraw++) {
|
||||||
|
;[left1, top1] = refine(DRAWN, left1, top1)
|
||||||
|
}
|
||||||
|
const [left0, top0] = refine(DRAWN, 128, 64)
|
||||||
|
assert.deepEqual(
|
||||||
|
[left1, top1],
|
||||||
|
[left0, top0],
|
||||||
|
"a press where the pointer already was is not a drag",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// A real drag still reads as one.
|
||||||
|
{
|
||||||
|
const refine = cursorRefiner()
|
||||||
|
const from = refine(DRAWN, 128, 64)
|
||||||
|
const to = refine(DRAWN, 192, 64)
|
||||||
|
assert.notDeepEqual(to, from, "a pointer that moved has moved")
|
||||||
|
}
|
||||||
|
|
||||||
|
// Leaving the plot: uPlot parks the cursor off-plot at -10, and it has to stay
|
||||||
|
// off-plot however it is scaled, or the line would stick where it was.
|
||||||
|
{
|
||||||
|
const refine = cursorRefiner()
|
||||||
|
refine(DRAWN, 128, 64)
|
||||||
|
const [left, top] = refine(DRAWN, -10, -10)
|
||||||
|
assert.ok(left < 0 && top < 0, "the parked cursor stays off the plot")
|
||||||
|
}
|
||||||
|
|
||||||
|
console.log("cursor: ok")
|
||||||
@@ -445,10 +445,22 @@ test("a chart's cursor follows the pointer", async ({ page }) => {
|
|||||||
|
|
||||||
const cursor = chart.locator(".u-cursor-x")
|
const cursor = chart.locator(".u-cursor-x")
|
||||||
await expect(cursor).toBeVisible()
|
await expect(cursor).toBeVisible()
|
||||||
const line = (await cursor.boundingBox()) as { x: number }
|
const at = async () => ((await cursor.boundingBox()) as { x: number }).x
|
||||||
|
const line = await at()
|
||||||
expect(
|
expect(
|
||||||
Math.abs(line.x - x),
|
Math.abs(line - x),
|
||||||
`the cursor is drawn at ${line.x.toFixed(1)}, the pointer is at ${x.toFixed(1)}`,
|
`the cursor is drawn at ${line.toFixed(1)}, the pointer is at ${x.toFixed(1)}`,
|
||||||
|
).toBeLessThan(3)
|
||||||
|
|
||||||
|
// And it stays there. The chart sets its data on every render, which makes
|
||||||
|
// uPlot recompute the cursor from the position it already holds — so a
|
||||||
|
// correction applied twice would walk the line left while nothing moved.
|
||||||
|
await publish(page, w("level"), 55)
|
||||||
|
await page.waitForTimeout(1500)
|
||||||
|
const settled = await at()
|
||||||
|
expect(
|
||||||
|
Math.abs(settled - x),
|
||||||
|
`after a redraw the cursor is at ${settled.toFixed(1)}, the pointer at ${x.toFixed(1)}`,
|
||||||
).toBeLessThan(3)
|
).toBeLessThan(3)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user