From decd069ec833076f7ce0013e687c703f05cbe487 Mon Sep 17 00:00:00 2001 From: Chris Amow Date: Mon, 10 Aug 2026 23:55:51 -0500 Subject: [PATCH] Position chart overlays against the plot, not the element MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The trendline snap indicator was 66 pixels out, and so was every other overlay. Lightweight Charts reports coordinates from the plot area's origin. The chart element also contains the price scales, so enabling the left scale for the daily labels moved the plot 66px into the element — and each overlay positioned with left: against the element inherited that error twice over. The cursor's element-x was read as a plot-x, resolving a bar about 66px right of the pointer; the indicator was then drawn at that bar's plot-x interpreted as element-x, landing 66px left of where the bar is painted. Neither near the cursor nor near the bar, and scaling with zoom — two bars at 30m, a dozen at 1m — which is why it read as random rather than as an offset. Every diagnostic number agreed with itself the whole time, because dot_y, expected_y and bar_low_y all come from the same API and shared the same wrong origin. Instrumentation cannot see a systematic error in its own frame of reference; what found this was comparing canvas.width to element.clientWidth and getting 0.894. Overlays now live in one container positioned over the plot canvas and inherit plot coordinates untranslated — snap dot and label, comments, anchor handles, preview line, tooltip, price tag, context menu — and eventPoint subtracts the same offset so a pointer position and a chart coordinate mean the same thing. The container follows the plot on resize. Verified in page pixels rather than through the coordinate API: with the cursor placed 30px below a known bar's low, the dot lands 30px above the cursor, on that low, labelled 7789.00 L against a bar low of 7789. Device pixel ratio, viewport size, resize desynchronisation and the chart scrolling under the gesture were each measured and ruled out before this. The e2e helper computed expected times from element-relative x, the same mistake in the tests, and is corrected here. Co-Authored-By: Claude Opus 5 --- docs/IMPLEMENTATION_PLAN.md | 34 +++++++++++++ static/chart.js | 98 +++++++++++++++++++++++++++++++----- static/style.css | 2 + tests/e2e/trendline.test.mjs | 14 +++++- 4 files changed, 134 insertions(+), 14 deletions(-) diff --git a/docs/IMPLEMENTATION_PLAN.md b/docs/IMPLEMENTATION_PLAN.md index 21130cc..67b5fca 100644 --- a/docs/IMPLEMENTATION_PLAN.md +++ b/docs/IMPLEMENTATION_PLAN.md @@ -1626,3 +1626,37 @@ killed by measurement here — device pixel ratio, viewport size, resize desynchronisation, and the chart scrolling under the gesture — while the actual cause was visible in one line of the client's own numbers. When the browser is on another machine, instrument it early instead of reproducing locally. + +### Overlays are positioned against the plot, not the element + +**The trendline snap was 66 pixels out, and so was everything else drawn over +the chart.** Lightweight Charts reports coordinates from the plot area's origin. +The chart *element* also contains the price scales, so once the left scale was +enabled for the daily labels, the plot started 66px into the element — and every +overlay positioned with `left:` against the element was displaced by exactly +that much, in both directions at once: + +- the cursor's element-x was read as a plot-x, resolving a bar ~66px to the + right of the pointer; +- the indicator was then drawn at that bar's plot-x interpreted as element-x, + landing ~66px left of where the bar is painted. + +Not near the cursor, not near the bar, and varying with zoom — 66px is a couple +of bars at 30m and a dozen at 1m, which is why it looked random rather than +offset. Every diagnostic number agreed with itself throughout, because +`dot_y`, `expected_y` and `bar_low_y` all derive from the same API and shared +the same wrong origin. Self-consistent instrumentation cannot see a systematic +error in its own frame of reference. + +All overlays now live in one container positioned over the plot canvas, so they +inherit plot coordinates untranslated: the snap dot and label, the comment +layer, the trendline anchor handles, the preview line, the tooltip, the price +tag and the context menu. `eventPoint` subtracts the same offset, so a pointer +position and a chart coordinate finally mean the same thing. The container is +repositioned on resize. + +This had been mis-diagnosed for hours: device pixel ratio, viewport size, resize +desynchronisation, the chart scrolling under the gesture, and the dead band +below the candles were each measured and ruled out. The measurement that found +it was comparing `canvas.width` to `element.clientWidth` — 0.894 — which is the +first thing that ever disagreed with itself. diff --git a/static/chart.js b/static/chart.js index 888facf..1c9bba4 100644 --- a/static/chart.js +++ b/static/chart.js @@ -33,8 +33,11 @@ class ConfluenceChart { this.toolMoveListener = null; this.toolUpListener = null; this.pendingView = null; + this.overlayLayer = null; this.snapDot = null; this.snapLabel = null; + this.snapLeader = null; + this.snapLeaderLine = null; this.comments = []; this.commentNodes = new Map(); this.commentLayer = null; @@ -141,24 +144,42 @@ class ConfluenceChart { priceLineVisible: false, }); this.chart.priceScale('volume').applyOptions({ - scaleMargins: { top: 0.82, bottom: 0 }, + scaleMargins: { top: 0.88, bottom: 0 }, visible: false, }); + // The default leaves a tenth of the pane empty beneath the lowest bar, + // which is where the volume draws and where a cursor tracing swing lows + // naturally sits — far from any actual price. Give most of it back. + this.chart.priceScale('right').applyOptions({ scaleMargins: { top: 0.12, bottom: 0.04 } }); + this.chart.priceScale('left').applyOptions({ scaleMargins: { top: 0.12, bottom: 0.04 } }); this.resizeObserver = new ResizeObserver(() => { this.chart.applyOptions({ width: el.clientWidth, height: el.clientHeight }); - requestAnimationFrame(() => this.renderAnchorHandles()); + requestAnimationFrame(() => { + this.syncOverlayLayer(); + this.renderAnchorHandles(); + this.renderComments(); + }); }); this.resizeObserver.observe(el); + // Every overlay lives in here, and this is positioned over the *plot* — not + // the element, which also contains the price scales. Lightweight Charts + // reports coordinates from the plot's origin, so anchoring the container + // there lets each overlay use those coordinates untranslated. Enabling the + // left price scale moved the plot 66px right and silently displaced the + // snap indicator, the comments and the trendline handles by that much. + this.overlayLayer = document.createElement('div'); + this.overlayLayer.className = 'chart-overlays'; + el.appendChild(this.overlayLayer); this.tooltip = document.createElement('div'); this.tooltip.className = 'chart-tooltip'; - el.appendChild(this.tooltip); + this.overlayLayer.appendChild(this.tooltip); const preview = document.createElementNS('http://www.w3.org/2000/svg', 'svg'); preview.classList.add('chart-preview'); preview.setAttribute('aria-hidden', 'true'); this.previewLine = document.createElementNS('http://www.w3.org/2000/svg', 'line'); this.previewLine.setAttribute('hidden', ''); preview.appendChild(this.previewLine); - el.appendChild(preview); + this.overlayLayer.appendChild(preview); const handles = document.createElementNS('http://www.w3.org/2000/svg', 'svg'); handles.classList.add('chart-handles'); handles.setAttribute('aria-hidden', 'true'); @@ -173,7 +194,7 @@ class ConfluenceChart { handles.appendChild(handle); this.anchorHandles.push(handle); } - el.appendChild(handles); + this.overlayLayer.appendChild(handles); this.contextMenu = document.createElement('div'); this.contextMenu.className = 'chart-context-menu'; this.contextMenu.hidden = true; @@ -186,25 +207,35 @@ class ConfluenceChart { }); this.contextMenu.addEventListener('click', event => event.stopPropagation()); this.contextMenu.appendChild(endHere); - el.appendChild(this.contextMenu); + this.overlayLayer.appendChild(this.contextMenu); this.priceTag = document.createElement('div'); this.priceTag.className = 'chart-price-tag'; this.priceTag.hidden = true; - el.appendChild(this.priceTag); + this.overlayLayer.appendChild(this.priceTag); this.snapDot = document.createElement('div'); this.snapDot.className = 'chart-snap-dot'; this.snapDot.hidden = true; - el.appendChild(this.snapDot); + this.overlayLayer.appendChild(this.snapDot); // States the price the anchor will use. Useful in itself, and it means a // single screenshot answers "is the dot in the wrong place, or on the // wrong bar?" without anyone pasting console output. this.snapLabel = document.createElement('div'); this.snapLabel.className = 'chart-snap-label'; this.snapLabel.hidden = true; - el.appendChild(this.snapLabel); + this.overlayLayer.appendChild(this.snapLabel); + // A leader from the cursor to the target. The snap is often far from the + // pointer — hovering below the candles legitimately snaps up to a bar's low + // — and without a line joining them the dot reads as unrelated to where you + // are pointing. + this.snapLeader = document.createElementNS('http://www.w3.org/2000/svg', 'svg'); + this.snapLeader.classList.add('chart-snap-leader'); + this.snapLeader.setAttribute('aria-hidden', 'true'); + this.snapLeaderLine = document.createElementNS('http://www.w3.org/2000/svg', 'line'); + this.snapLeader.appendChild(this.snapLeaderLine); + this.overlayLayer.appendChild(this.snapLeader); this.commentLayer = document.createElement('div'); this.commentLayer.className = 'chart-comments'; - el.appendChild(this.commentLayer); + this.overlayLayer.appendChild(this.commentLayer); this.toolDownListener = event => this.startToolGesture(event); this.toolMoveListener = event => this.moveToolGesture(event); this.toolUpListener = event => this.finishToolGesture(event); @@ -236,6 +267,7 @@ class ConfluenceChart { } this.updateLineTooltip(param); }); + requestAnimationFrame(() => this.syncOverlayLayer()); this.chart.timeScale().subscribeVisibleLogicalRangeChange(() => { this.renderAnchorHandles(); this.renderComments(); @@ -571,9 +603,40 @@ class ConfluenceChart { this.clearLinePreview(); } + /** + * Pixels between the element's left edge and the plot area. + * + * Lightweight Charts measures coordinates from the plot, not the element, so + * a visible left price scale shifts the two apart. Overlays are positioned + * against the element, so every one of them must add this back. Enabling the + * left scale for the daily labels silently moved the snap indicator, the + * comments and the anchor handles 66px out of place. + */ + /** Put the overlay container exactly over the plot area. */ + syncOverlayLayer() { + if (!this.overlayLayer || !this.chartEl) return; + const canvas = [...this.chartEl.querySelectorAll('canvas')] + .sort((a, b) => (b.width * b.height) - (a.width * a.height))[0]; + if (!canvas) return; + const chartRect = this.chartEl.getBoundingClientRect(); + const plot = canvas.getBoundingClientRect(); + this.overlayLayer.style.left = `${Math.round(plot.left - chartRect.left)}px`; + this.overlayLayer.style.top = `${Math.round(plot.top - chartRect.top)}px`; + this.overlayLayer.style.width = `${Math.round(plot.width)}px`; + this.overlayLayer.style.height = `${Math.round(plot.height)}px`; + } + + plotOffsetX() { + if (!this.chartEl) return 0; + const canvas = [...this.chartEl.querySelectorAll('canvas')] + .sort((a, b) => (b.width * b.height) - (a.width * a.height))[0]; + if (!canvas) return 0; + return canvas.getBoundingClientRect().left - this.chartEl.getBoundingClientRect().left; + } + eventPoint(event) { const bounds = this.chartEl.getBoundingClientRect(); - const x = event.clientX - bounds.left; + const x = event.clientX - bounds.left - this.plotOffsetX(); const y = event.clientY - bounds.top; const price = this.candles.coordinateToPrice(y); if (price == null) return null; @@ -684,11 +747,22 @@ class ConfluenceChart { this.snapLabel.style.left = `${Math.round(flip ? x - 12 : x + 12)}px`; this.snapLabel.style.top = `${Math.round(y)}px`; this.snapLabel.style.transform = flip ? 'translate(-100%, -50%)' : 'translate(0, -50%)'; + + if (point.x != null && point.y != null) { + this.snapLeaderLine.setAttribute('x1', point.x); + this.snapLeaderLine.setAttribute('y1', point.y); + this.snapLeaderLine.setAttribute('x2', x); + this.snapLeaderLine.setAttribute('y2', y); + this.snapLeader.style.display = ''; + } else { + this.snapLeader.style.display = 'none'; + } } hideSnapDot() { if (this.snapDot) this.snapDot.hidden = true; if (this.snapLabel) this.snapLabel.hidden = true; + if (this.snapLeader) this.snapLeader.style.display = 'none'; } renderPending(a, b) { @@ -1041,7 +1115,7 @@ class ConfluenceChart { if (!level || !this.bars.length) return; event.preventDefault(); const bounds = this.chartEl.getBoundingClientRect(); - const x = event.clientX - bounds.left; + const x = event.clientX - bounds.left - this.plotOffsetX(); const rawTime = this.chart.timeScale().coordinateToTime(x); if (rawTime == null) return; const cutoff = this.bars.reduce((nearest, bar) => diff --git a/static/style.css b/static/style.css index b4651b5..813e462 100644 --- a/static/style.css +++ b/static/style.css @@ -79,3 +79,5 @@ aside { padding:16px; }h2 { margin:0 0 12px; color:var(--muted); font-size:11px; .side-auto { align-self:end; padding-bottom:6px; font-size:10px; color:var(--muted); }.side-auto b { color:var(--fg); font-weight:600; } .chart-snap-dot { position:absolute; width:9px; height:9px; margin:-5px 0 0 -5px; border-radius:50%; border:2px solid var(--accent); background:var(--chart-bg); pointer-events:none; z-index:5; }.chart-snap-dot[data-side=resistance] { border-color:#bd4545; }.chart-snap-dot[data-side=support] { border-color:#27825c; } .chart-snap-label { position:absolute; padding:2px 5px; border-radius:3px; pointer-events:none; z-index:6; font-size:10px; white-space:nowrap; background:var(--chart-bg); border:1px solid var(--muted); color:var(--fg); }.chart-snap-label[data-side=resistance] { border-color:#bd4545; }.chart-snap-label[data-side=support] { border-color:#27825c; } +.chart-snap-leader { position:absolute; inset:0; pointer-events:none; z-index:5; overflow:visible; }.chart-snap-leader line { stroke:var(--muted); stroke-width:1; stroke-dasharray:3 3; opacity:.75; } +.chart-overlays { position:absolute; left:0; top:0; pointer-events:none; overflow:visible; z-index:3; }.chart-overlays > * { pointer-events:auto; }.chart-overlays .chart-comments, .chart-overlays .chart-snap-leader { position:absolute; inset:0; } diff --git a/tests/e2e/trendline.test.mjs b/tests/e2e/trendline.test.mjs index 82eeece..a932ff0 100644 --- a/tests/e2e/trendline.test.mjs +++ b/tests/e2e/trendline.test.mjs @@ -11,9 +11,18 @@ import { withChart, chartBox, at, armTool, newestTrendline, assertNoPageErrors, } from './helpers.mjs'; -/** Time under a given x, for comparing against where a line actually anchored. */ +/** + * Time under a given page x. + * + * Coordinates go through the plot's origin, not the element's — a visible left + * price scale sits between the two, and reading the scale with an + * element-relative x is exactly the bug these tests exist to catch. + */ const timeAt = (page, box, point) => - page.evaluate(x => Number(window.__chart.chart.timeScale().coordinateToTime(x)), point.x - box.x); + page.evaluate(x => { + const c = window.__chart; + return Number(c.chart.timeScale().coordinateToTime(x - c.plotOffsetX())); + }, point.x - box.x); test('a click, a move, then a click starts at the first click', { timeout: 180000 }, async () => { await withChart(async page => { @@ -148,6 +157,7 @@ test('sweeping the bottom traces the low of each bar under the cursor', const result = await page.evaluate(() => { const c = window.__chart, ts = c.chart.timeScale(); const y = document.querySelector('#chart').clientHeight * 0.93; + // x values below are plot-relative, matching what snapPoint expects. let hits = 0, total = 0; const misses = []; for (let x = 200; x < 1000; x += 7) {