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.