From 8f71beb59f7df0f8a3323e8dae52cbf16c274d51 Mon Sep 17 00:00:00 2001 From: Chris Amow Date: Tue, 11 Aug 2026 02:07:17 -0500 Subject: [PATCH] Hand off the residual 10px overlay offset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The snap indicator still draws about one bar left of the cursor, with the right height and the right bar chosen. Measured: bar spacing 6.96px, dot centre 10.5px left of the cursor, and syncOverlayLayer reading containerLeft 56 against a true plot offset of 66. It runs once in a requestAnimationFrame during create(), before the left price scale has sized itself to its label text. The offset settles at 66 once labels render and nothing re-measures, so the container — and every overlay in it — stays 10px left for the life of the page. Only x is affected, which is why the height has always looked correct. Written up at the bottom of the plan with the measurements, the three candidate fixes, the trap that the scale's width tracks its label text, and a note that the e2e suite cannot catch this because its assertions go through the same coordinate API that carries the error — a page-pixel assertion is needed. Co-Authored-By: Claude Opus 5 --- docs/IMPLEMENTATION_PLAN.md | 99 +++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/docs/IMPLEMENTATION_PLAN.md b/docs/IMPLEMENTATION_PLAN.md index 67b5fca..a4d3d7e 100644 --- a/docs/IMPLEMENTATION_PLAN.md +++ b/docs/IMPLEMENTATION_PLAN.md @@ -1660,3 +1660,102 @@ 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. + +--- + +# Handoff: the snap indicator is ~10px left of where it belongs + +**Status: diagnosed, not fixed.** Everything below is measured, not inferred. + +## The symptom + +With the Trendline tool armed, the snap dot sits about one bar to the left of +the cursor. The *height* is correct and the *bar it chooses* is correct — only +the horizontal drawing position is wrong. Reported from a real browser and +reproduced headlessly. + +## The measurement + +``` +bar spacing 6.96 px +dot centre - cursor -10.5 px (= 1.5 bars at that zoom) +chosen bar correct (label names the bar under the cursor) +``` + +And the cause, from `ConfluenceChart.syncOverlayLayer()`: + +``` +at load: containerLeft 56 true plot offset 66 <- 10px stale +after a re-sync: containerLeft 66 true plot offset 66 <- correct +``` + +## Why + +Lightweight Charts reports coordinates from the **plot area's** origin. The +chart *element* also contains the price scales, so with the left scale enabled +the plot begins 66px in. All overlays therefore live in a container +(`.chart-overlays`) positioned over the plot, so they can use chart coordinates +untranslated — see `create()` and `syncOverlayLayer()` in `static/chart.js`. + +`syncOverlayLayer()` runs once in a `requestAnimationFrame` during `create()`. +At that moment the left price scale has not finished sizing itself to its label +text, so the measured offset is 56. It settles at 66 once labels render, and +nothing re-measures. The container stays 10px left of the plot for the life of +the page, which drags every overlay with it: the snap dot and label, comments, +trendline anchor handles, the preview line, the tooltip and the price tag. + +Only `x` is affected. `y` never passes through this offset, which is why the +height has always looked right. + +## The fix to write + +Re-measure instead of measuring once. Options, cheapest first: + +1. `ResizeObserver` on the plot canvas — fires when the scale settles and on + every later change. Probably the right answer. +2. Call `syncOverlayLayer()` at the top of `renderComments()` and + `showSnapDot()`. Correct but does DOM reads on every mouse move. +3. Re-sync on `subscribeVisibleLogicalRangeChange` as well as on resize. Cheap, + but misses a scale that widens without the range changing. + +Beware: the left scale's width depends on its **label text**, so it changes when +the price range gains a digit or a longer level label appears. Whatever you +choose must survive that, not just the initial load. + +## How to verify + +```bash +./bin/e2e trendline # 7 cases, all currently pass — they do not catch this +``` + +The suite misses it because its assertions go through the same coordinate API +that carries the error. Add a test that measures in **page pixels**: place the +cursor exactly at a bar's centre and assert the dot's centre is within ~2px +horizontally. The reproduction is: + +```js +const r = el.getBoundingClientRect(); +const bx = chart.timeScale().timeToCoordinate(bar.t); +await page.mouse.move(r.x + c.plotOffsetX() + bx, r.y + c.candles.priceToCoordinate(bar.l) - 6); +// dot centre x should equal the cursor x; today it is ~10px left +``` + +Live numbers from the client are available without a console: open the chart +with `?diag=1`, then `docker compose logs api | grep SNAPDBG`. Note that +`SNAPDBG` will **not** show this bug — `dot_y`, `expected_y` and `bar_low_y` all +derive from the same API and share the same origin, so they agree with each +other while being wrong together. The error is only visible by comparing against +something outside that frame of reference: `canvas.getBoundingClientRect()` +against `element.getBoundingClientRect()`, or painted pixels. + +## Context worth having + +- `window.__chart` is a deliberate debug handle exposing the wrapper. +- The dev stack is at `http://localhost:8010`, and `http://api:8000` from inside + the playwright container. It runs on a remote machine; the user's browser does + not. Headless passes prove little about their screen — see "Where things run" + in AGENTS.md. +- Related history is above under "Overlays are positioned against the plot, not + the element", which fixed the 66px case this 10px residue survived. +- One e2e test, "clicking a comment collapses it", is flaky (roughly one run in + three) and unrelated. Worth fixing before trusting the suite.