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) {