Chart drawing tools: trend, horizontal, rectangle + per-slab persistence

[0x-SquidSol]

Summary

Adds user-drawing primitives to the price chart — trend lines, horizontal lines, and rectangles — with a left-edge toolbar, pointer-mode select / delete, and a per-slab clear-all. Drawings persist in localStorage scoped per slab address.

Architecture

• Data layer (chart-drawings.ts): discriminated union Drawing = trend | horizontal | rectangle, versioned storage envelope, MAX_DRAWINGS_PER_SLAB = 100 cap with FIFO drop-oldest, tolerant deserializer that drops malformed entries, Record<Kind, true> validation tables that fail compile-closed when a new variant is added without a matching key.
• Persistence hook (useChartDrawings.ts): per-slab localStorage bucket keyed by Solana base58 pubkey (validated against /^[1-9A-HJ-NP-Za-km-z]{32,44}$/). Writes are debounced 250 ms trailing; flushes on slab change, unmount, and pagehide so no in-flight write is lost.
• Coordinate transformer (chart-coords.ts): structural PriceConverter / TimeConverter interfaces over the lightweight-charts series + timescale APIs. Math.trunc(ms/1000) for ms→s conversion to keep pre-epoch semantics consistent.
• Hit testing (chart-hit-test.ts): per-kind distance-to-segment with degenerate-segment fallback, edge-only rectangle hit-test, reverse-iteration so the top-most-drawn drawing wins.
• Canvas helper (chart-canvas.ts): DPR-aware sizing with min-1 clamps to avoid 0×0 crashes.
• Tool state (useChartDrawingTool.ts): session-local active tool. Intentionally NOT persisted across sessions — leaving a creation tool active across page loads silently dropped a creation anchor on the next visit's first click.
• Overlay (ChartDrawingOverlay.tsx): transparent canvas with `pointer-events: none`. Click dispatch routes through `chart.subscribeClick` so native pan / zoom keep working. Subscription deps key only on chart refs + chartReady (subscribe-once); state changes flow via refs and an imperative redrawRef. Crosshair-move and rectangle-drag mousemove rAF-coalesce to one redraw per vsync. Canvas clips to the price-pane's candle-area bounds (top scaleMargin → 1-bottom scaleMargin) so drawings can't paint into volume / oscillator panes; clicks outside that band are rejected so phantom drawings can't anchor at extrapolated prices.
• Toolbar (ChartDrawingToolbar.tsx): vertical bar at the chart's top-left. Hidden `md:flex` (drawing tools are desktop-only for v1) and gated on `!showEmptyOverlay`. Labelled `
  ` with `` toggles — deliberately not `role="toolbar"` because we don't implement APG roving tabindex. Clear-all uses `window.confirm` with singular / plural wording and resets the active tool to pointer afterwards.

Mobile

Touch viewports short-circuit every tool at click dispatch (creation tools have no toolbar to back out of, pointer mode has no Backspace / Delete to clean up a selection). Drawings created on desktop still render on mobile as static art.

Theme awareness

Stroke is the brand accent in both themes. Fill alpha resolves per-frame via `document.documentElement[data-theme]` — 15 % on dark, 22 % on light (the lower alpha washes out against the near-white light bg). Stroke contrast meets WCAG AA 3:1 for graphics in both themes.

Test plan

• Unit tests for the data layer, hit-test math, coordinate transformer, canvas sizing, persistence hook, and tool hook
• Component tests for the toolbar (ARIA contract, active-tool feedback, clear-all confirm flow) and overlay (subscription lifecycle, click dispatch per tool, keyboard priority chain, rectangle drag including stuck-state recovery via contextmenu + window blur, slab-change reset, pane-bounds clipping)
• `tsc --noEmit` clean for the feature files
• Full app suite passes locally (chart-drawing tests: all green)
• Manual smoke: draw each kind on devnet BTC market, switch slab, refresh, verify per-slab persistence
• Manual smoke: hover-preview on a slow laptop to confirm rAF coalescing keeps frames smooth
• Manual smoke: light theme drawings legible against `#FAFAFD` bg
• Manual smoke: rectangle drag → contextmenu / alt-tab cancels cleanly without latching pan-suppression

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

• New Features

  • Added chart drawing tools: pointer, trend line, horizontal line, and rectangle for annotating charts
  • Drawing toolbar with clear all functionality to manage annotations
  • Persistent drawing storage across sessions
  • Keyboard support: Escape to cancel, Delete/Backspace to delete drawings

• Tests

  • Added comprehensive test coverage for drawing components and utilities

[vercel]

@0x-SquidSol is attempting to deploy a commit to the Khubair Nasir's projects Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

📝 Walkthrough

Walkthrough

A new chart drawing system is introduced comprising data types, utilities, React components, and hooks to enable users to annotate trading charts with trend lines, horizontal levels, and rectangles. The system includes persistent per-slab storage, hit-test detection, and comprehensive test coverage across all layers.

Changes

Chart Drawing System

Layer / File(s)	Summary
Data & Type Layer app/lib/chart-drawings.ts	Defines Drawing, DrawingKind ("trend", "horizontal", "rectangle"), and PricePoint types; exports validators isDrawing and mergeDrawings to handle versioned envelope storage and defend against prototype pollution.
Coordinate & Geometry Layer app/lib/chart-coords.ts, app/lib/chart-canvas.ts, app/lib/chart-hit-test.ts	chart-coords.ts converts between internal ms/price space and lightweight-charts pixel coordinates via PriceConverter and TimeConverter interfaces; chart-canvas.ts provides sizeCanvasForDpr for DPR-aware canvas setup; chart-hit-test.ts implements distanceToSegment and shape-specific hit-test functions (hitTestTrend, hitTestHorizontal, hitTestRectangle) plus findHitDrawingId for overlap resolution.
UI Components app/components/trade/ChartDrawingOverlay.tsx, app/components/trade/ChartDrawingToolbar.tsx	ChartDrawingOverlay renders a canvas overlay for interactive drawing, manages tool state (pointer/trend/horizontal/rectangle), handles click/drag/keyboard interactions, and subscribes to chart events (range, size, click, crosshair). ChartDrawingToolbar provides a vertical toolbar to switch tools and a clear-all action with user confirmation.
State Management Hooks app/hooks/useChartDrawingTool.ts, app/hooks/useChartDrawings.ts	useChartDrawingTool maintains ephemeral in-memory tool selection. useChartDrawings manages per-slab drawing CRUD with debounced localStorage persistence, hydration on valid slab addresses, and pagehide flush to prevent data loss.
Integration app/components/trade/TradingChart.tsx	Wires ChartDrawingOverlay, ChartDrawingToolbar, and both hooks into the main trading chart; renders overlays when chart has data and adjusts tooltip positioning for toolbar space.
Tests app/__tests__/components/trade/ChartDrawing*.test.tsx, app/__tests__/hooks/useChart*.test.ts, app/__tests__/lib/chart-*.test.ts	Comprehensive test suites covering component rendering, interaction lifecycle, hook state management, persistence semantics, coordinate transformations, geometry/hit-test logic, and edge cases (off-scale, degenerate segments, prototype pollution, localStorage errors).

Sequence Diagram

sequenceDiagram
    participant User
    participant Toolbar as ChartDrawingToolbar
    participant Overlay as ChartDrawingOverlay
    participant Chart as Lightweight Charts
    participant Storage as localStorage

    User->>Toolbar: Select "trend" tool
    Toolbar->>Overlay: setTool("trend")
    activate Overlay
    User->>Chart: Click point 1
    Chart->>Overlay: subscribeClick triggered
    Overlay->>Overlay: pendingP1 = pixel coordinate
    Overlay->>Chart: Subscribe to crosshairMove
    User->>Chart: Move cursor (crosshair preview)
    Chart->>Overlay: subscribeCrosshairMove
    Overlay->>Overlay: previewP2 = live coordinate
    Overlay->>Overlay: redraw trend preview
    User->>Chart: Click point 2
    Chart->>Overlay: subscribeClick triggered
    Overlay->>Overlay: Create Drawing from p1, p2
    Overlay->>Overlay: addDrawing(input)
    Overlay->>Storage: Schedule persist (debounced)
    deactivate Overlay
    Overlay->>Toolbar: Update drawingCount
    User->>Toolbar: Click "Clear all drawings"
    Toolbar->>User: Confirm dialog
    User->>Toolbar: Confirm
    Toolbar->>Overlay: clearAll()
    Overlay->>Storage: Persist empty array
    Toolbar->>Toolbar: setTool("pointer")

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested reviewers

• dcccrypto

---

A rabbit hops through canvas bright,
Drawing trends and rects with delight,
Click and drag to mark the chart,
Each design a work of art! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.76% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main additions: chart drawing tools (trend, horizontal, rectangle) and per-slab persistence. It is concise and specific enough to convey the primary change.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (4)

app/__tests__/components/trade/ChartDrawingToolbar.test.tsx (1)

128-131: 💤 Low value

Dead fallback in getByRole chain.

getByRole throws when the element is missing — it never returns null/undefined, so the ?? fallback is unreachable and only makes the assertion harder to read. A single getByRole("separator") is sufficient (the separator has an explicit role="separator").

🧹 Proposed simplification

-      expect(
-        screen.getByRole("separator", { hidden: true }) ??
-          screen.getByRole("separator"),
-      ).toBeInTheDocument();
+      expect(screen.getByRole("separator")).toBeInTheDocument();

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/__tests__/components/trade/ChartDrawingToolbar.test.tsx` around lines 128
- 131, The test uses a redundant nullish fallback with
screen.getByRole("separator") which never returns null (it throws), so remove
the fallback and simplify the assertion to a single call to
screen.getByRole("separator") in the ChartDrawingToolbar.test.tsx test; update
the assertion that currently references screen.getByRole("separator", { hidden:
true }) ?? screen.getByRole("separator") to just use
screen.getByRole("separator") (keeping the toBeInTheDocument() expectation).

app/hooks/useChartDrawings.ts (2)

257-262: 💤 Low value

clearAll updater doesn't need to read prev state.

Since the updater ignores current and always returns [], the indirection adds no value over a direct setDrawings([]) call. Persisting outside the updater also keeps the side effect out of the state-reducer (consistent with React's "updaters should be pure" guidance, even though schedulePersist is idempotent).

🧹 Proposed simplification

   const clearAll = useCallback(() => {
-    setDrawings(() => {
-      schedulePersist(slabRef.current, []);
-      return [];
-    });
+    setDrawings([]);
+    schedulePersist(slabRef.current, []);
   }, [schedulePersist]);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/hooks/useChartDrawings.ts` around lines 257 - 262, The clearAll updater
uses a functional setDrawings but ignores the previous value; change clearAll to
call setDrawings([]) directly and move the side-effect
schedulePersist(slabRef.current, []) outside the state updater (call it
immediately after setDrawings), keeping the schedulePersist and slabRef
references and preserving the useCallback dependencies so the behavior remains
the same without using an updater function in clearAll.

---

90-98: 💤 Low value

The ref-mutation-during-render pattern is discouraged but appears intentionally guarded by targeted tests.

React's docs explicitly warn against "writing or reading a ref during rendering" for good reason — under concurrent rendering, re-runs and discards can make the timing unpredictable. The safer canonical form is useEffect(() => { slabRef.current = slabAddress; }) to sync after commit.

However, the tests include a case specifically for this: addDrawing writes to the NEW slab when called immediately after a slab change, which validates that a setter fired between the render-time ref assignment and the effect commit lands under the correct slab. The test comment shows the team understands the constraint.

That said, the tests don't appear to run under React's StrictMode or explicit concurrent rendering, which would stress-test the render-discarding behavior that makes this pattern risky. Either confirm the pattern is intentional with its trade-offs, or migrate to the safer useEffect form to eliminate this edge case entirely.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/hooks/useChartDrawings.ts` around lines 90 - 98, The current
useChartDrawings hook mutates slabRef.current during render (slabRef.current =
slabAddress) which is unsafe under concurrent rendering; change this to
synchronize after commit by removing the render-time assignment and adding a
useEffect(() => { slabRef.current = slabAddress; }, [slabAddress]) inside
useChartDrawings so the ref is updated post-commit. Update any tests that assume
immediate render-time mutation (notably the test expecting addDrawing to write
to the NEW slab immediately after changing slabAddress) to wait for the effect
to flush (e.g., using act/flush effects or invoking addDrawing after the effect)
so they reflect the new post-commit semantics. Ensure references to slabRef,
slabAddress and addDrawing in useChartDrawings are adjusted accordingly.

app/__tests__/components/trade/ChartDrawingOverlay.test.tsx (1)

156-186: ⚡ Quick win

Use vi.spyOn / vi.stubGlobal so cleanup actually restores the originals.

Lines 156–165 directly mutate HTMLCanvasElement.prototype.getContext via property assignment. vi.restoreAllMocks() only restores spies created via vi.spyOn; it cannot revert raw property assignments. Lines 170–171 similarly assign globalThis.ResizeObserver = FakeResizeObserver directly with @ts-expect-error, so vi.unstubAllGlobals() (line 185) is a no-op for it. This is inconsistent with the file's idiomatic use of vi.stubGlobal for rAF/cAF (lines 176–180).

Switch to vi.spyOn(HTMLCanvasElement.prototype, "getContext") and vi.stubGlobal("ResizeObserver", ...) to ensure proper cleanup, drop the @ts-expect-error, and align with Vitest patterns.

♻️ Proposed cleanup

 function stubCanvasContext() {
   /* ...ctxStub setup... */
-  HTMLCanvasElement.prototype.getContext = vi
-    .fn()
-    .mockReturnValue(ctxStub) as unknown as typeof HTMLCanvasElement.prototype.getContext;
+  vi.spyOn(HTMLCanvasElement.prototype, "getContext").mockReturnValue(
+    ctxStub as unknown as RenderingContext,
+  );
   return { setTransform, clearRect, beginPath, moveTo, lineTo, stroke, fillRect, strokeRect, arc, save, restore };
 }

 function stubNullCanvasContext() {
-  HTMLCanvasElement.prototype.getContext = vi
-    .fn()
-    .mockReturnValue(null) as unknown as typeof HTMLCanvasElement.prototype.getContext;
+  vi.spyOn(HTMLCanvasElement.prototype, "getContext").mockReturnValue(null);
 }

 beforeEach(() => {
   lastResizeObserver = null;
-  // `@ts-expect-error`: stubbing the global for tests.
-  globalThis.ResizeObserver = FakeResizeObserver;
+  vi.stubGlobal("ResizeObserver", FakeResizeObserver);
   vi.stubGlobal("requestAnimationFrame", (cb: FrameRequestCallback) => {

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/__tests__/components/trade/ChartDrawingOverlay.test.tsx` around lines 156
- 186, The test directly assigns HTMLCanvasElement.prototype.getContext and
globalThis.ResizeObserver which won't be reverted by
vi.restoreAllMocks/vi.unstubAllGlobals; change the assignments to use
vi.spyOn(HTMLCanvasElement.prototype, "getContext") for the canvas context stub
and vi.stubGlobal("ResizeObserver", FakeResizeObserver) for the resize observer
(remove the `@ts-expect-error`), and keep the existing vi.stubGlobal usage for
requestAnimationFrame/cancelAnimationFrame so all globals and spies are properly
cleaned up by vi.restoreAllMocks() and vi.unstubAllGlobals().

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/__tests__/components/trade/ChartDrawingToolbar.test.tsx`:
- Around line 50-80: The tests for ChartDrawingToolbar render it without
required props drawingCount and clearAll; update each render call in
ChartDrawingToolbar.test.tsx (the four it blocks that call
render(<ChartDrawingToolbar ... />)) to include the missing props by spreading
the existing noopProps (i.e., render(<ChartDrawingToolbar {...noopProps}
tool="..." setTool={setTool} />)) so the ChartDrawingToolbar component receives
drawingCount and clearAll and TypeScript compilation succeeds.

---

Nitpick comments:
In `@app/__tests__/components/trade/ChartDrawingOverlay.test.tsx`:
- Around line 156-186: The test directly assigns
HTMLCanvasElement.prototype.getContext and globalThis.ResizeObserver which won't
be reverted by vi.restoreAllMocks/vi.unstubAllGlobals; change the assignments to
use vi.spyOn(HTMLCanvasElement.prototype, "getContext") for the canvas context
stub and vi.stubGlobal("ResizeObserver", FakeResizeObserver) for the resize
observer (remove the `@ts-expect-error`), and keep the existing vi.stubGlobal
usage for requestAnimationFrame/cancelAnimationFrame so all globals and spies
are properly cleaned up by vi.restoreAllMocks() and vi.unstubAllGlobals().

In `@app/__tests__/components/trade/ChartDrawingToolbar.test.tsx`:
- Around line 128-131: The test uses a redundant nullish fallback with
screen.getByRole("separator") which never returns null (it throws), so remove
the fallback and simplify the assertion to a single call to
screen.getByRole("separator") in the ChartDrawingToolbar.test.tsx test; update
the assertion that currently references screen.getByRole("separator", { hidden:
true }) ?? screen.getByRole("separator") to just use
screen.getByRole("separator") (keeping the toBeInTheDocument() expectation).

In `@app/hooks/useChartDrawings.ts`:
- Around line 257-262: The clearAll updater uses a functional setDrawings but
ignores the previous value; change clearAll to call setDrawings([]) directly and
move the side-effect schedulePersist(slabRef.current, []) outside the state
updater (call it immediately after setDrawings), keeping the schedulePersist and
slabRef references and preserving the useCallback dependencies so the behavior
remains the same without using an updater function in clearAll.
- Around line 90-98: The current useChartDrawings hook mutates slabRef.current
during render (slabRef.current = slabAddress) which is unsafe under concurrent
rendering; change this to synchronize after commit by removing the render-time
assignment and adding a useEffect(() => { slabRef.current = slabAddress; },
[slabAddress]) inside useChartDrawings so the ref is updated post-commit. Update
any tests that assume immediate render-time mutation (notably the test expecting
addDrawing to write to the NEW slab immediately after changing slabAddress) to
wait for the effect to flush (e.g., using act/flush effects or invoking
addDrawing after the effect) so they reflect the new post-commit semantics.
Ensure references to slabRef, slabAddress and addDrawing in useChartDrawings are
adjusted accordingly.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a9fb4e0b-fdd0-43d6-8ec7-50d9de5ac647

📥 Commits

Reviewing files that changed from the base of the PR and between 7df38d3 and 61f1eff.

📒 Files selected for processing (17)

• app/__tests__/components/trade/ChartDrawingOverlay.test.tsx
• app/__tests__/components/trade/ChartDrawingToolbar.test.tsx
• app/__tests__/hooks/useChartDrawingTool.test.ts
• app/__tests__/hooks/useChartDrawings.test.ts
• app/__tests__/lib/chart-canvas.test.ts
• app/__tests__/lib/chart-coords.test.ts
• app/__tests__/lib/chart-drawings.test.ts
• app/__tests__/lib/chart-hit-test.test.ts
• app/components/trade/ChartDrawingOverlay.tsx
• app/components/trade/ChartDrawingToolbar.tsx
• app/components/trade/TradingChart.tsx
• app/hooks/useChartDrawingTool.ts
• app/hooks/useChartDrawings.ts
• app/lib/chart-canvas.ts
• app/lib/chart-coords.ts
• app/lib/chart-drawings.ts
• app/lib/chart-hit-test.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Tests omit required props (drawingCount, clearAll).

The ChartDrawingToolbarProps interface marks drawingCount and clearAll as required (no ?). The renders at lines 52, 59, 66, and 77 only pass tool and setTool, which will fail TypeScript compilation. Spread noopProps like the other tests do.

🔧 Proposed fix

   it("clicking an inactive tool calls setTool with that kind", () => {
     const setTool = vi.fn();
-    render(<ChartDrawingToolbar tool="pointer" setTool={setTool} />);
+    render(<ChartDrawingToolbar {...noopProps} tool="pointer" setTool={setTool} />);
     fireEvent.click(screen.getByRole("button", { name: /Trend line/i }));
     expect(setTool).toHaveBeenCalledWith("trend");
   });

   it("clicking the active non-pointer tool returns to pointer", () => {
     const setTool = vi.fn();
-    render(<ChartDrawingToolbar tool="rectangle" setTool={setTool} />);
+    render(<ChartDrawingToolbar {...noopProps} tool="rectangle" setTool={setTool} />);
     fireEvent.click(screen.getByRole("button", { name: /Rectangle/i }));
     expect(setTool).toHaveBeenCalledWith("pointer");
   });

   it("clicking the active pointer tool sets pointer (no-op equivalent)", () => {
     const setTool = vi.fn();
-    render(<ChartDrawingToolbar tool="pointer" setTool={setTool} />);
+    render(<ChartDrawingToolbar {...noopProps} tool="pointer" setTool={setTool} />);
     fireEvent.click(screen.getByRole("button", { name: /Pointer/i }));
     expect(setTool).toHaveBeenCalledWith("pointer");
   });
   …
-    render(<ChartDrawingToolbar tool="trend" setTool={setTool} />);
+    render(<ChartDrawingToolbar {...noopProps} tool="trend" setTool={setTool} />);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	it("clicking an inactive tool calls setTool with that kind", () => {
	const setTool = vi.fn();
	render(<ChartDrawingToolbar tool="pointer" setTool={setTool} />);
	fireEvent.click(screen.getByRole("button", { name: /Trend line/i }));
	expect(setTool).toHaveBeenCalledWith("trend");
	});
	it("clicking the active non-pointer tool returns to pointer", () => {
	const setTool = vi.fn();
	render(<ChartDrawingToolbar tool="rectangle" setTool={setTool} />);
	fireEvent.click(screen.getByRole("button", { name: /Rectangle/i }));
	expect(setTool).toHaveBeenCalledWith("pointer");
	});
	it("clicking the active pointer tool sets pointer (no-op equivalent)", () => {
	const setTool = vi.fn();
	render(<ChartDrawingToolbar tool="pointer" setTool={setTool} />);
	fireEvent.click(screen.getByRole("button", { name: /Pointer/i }));
	expect(setTool).toHaveBeenCalledWith("pointer");
	});
	it("does NOT register a keydown listener (Escape is owned by the overlay)", () => {
	// The Escape priority chain (cancel-pending → deselect → reset-tool)
	// lives in ChartDrawingOverlay so a single handler can sequence
	// those states. The toolbar must not race a second handler for
	// the same key.
	const setTool = vi.fn();
	render(<ChartDrawingToolbar tool="trend" setTool={setTool} />);
	fireEvent.keyDown(document, { key: "Escape" });
	expect(setTool).not.toHaveBeenCalled();
	});
	it("clicking an inactive tool calls setTool with that kind", () => {
	const setTool = vi.fn();
	render(<ChartDrawingToolbar {...noopProps} tool="pointer" setTool={setTool} />);
	fireEvent.click(screen.getByRole("button", { name: /Trend line/i }));
	expect(setTool).toHaveBeenCalledWith("trend");
	});
	it("clicking the active non-pointer tool returns to pointer", () => {
	const setTool = vi.fn();
	render(<ChartDrawingToolbar {...noopProps} tool="rectangle" setTool={setTool} />);
	fireEvent.click(screen.getByRole("button", { name: /Rectangle/i }));
	expect(setTool).toHaveBeenCalledWith("pointer");
	});
	it("clicking the active pointer tool sets pointer (no-op equivalent)", () => {
	const setTool = vi.fn();
	render(<ChartDrawingToolbar {...noopProps} tool="pointer" setTool={setTool} />);
	fireEvent.click(screen.getByRole("button", { name: /Pointer/i }));
	expect(setTool).toHaveBeenCalledWith("pointer");
	});
	it("does NOT register a keydown listener (Escape is owned by the overlay)", () => {
	// The Escape priority chain (cancel-pending → deselect → reset-tool)
	// lives in ChartDrawingOverlay so a single handler can sequence
	// those states. The toolbar must not race a second handler for
	// the same key.
	const setTool = vi.fn();
	render(<ChartDrawingToolbar {...noopProps} tool="trend" setTool={setTool} />);
	fireEvent.keyDown(document, { key: "Escape" });
	expect(setTool).not.toHaveBeenCalled();
	});

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/__tests__/components/trade/ChartDrawingToolbar.test.tsx` around lines 50
- 80, The tests for ChartDrawingToolbar render it without required props
drawingCount and clearAll; update each render call in
ChartDrawingToolbar.test.tsx (the four it blocks that call
render(<ChartDrawingToolbar ... />)) to include the missing props by spreading
the existing noopProps (i.e., render(<ChartDrawingToolbar {...noopProps}
tool="..." setTool={setTool} />)) so the ChartDrawingToolbar component receives
drawingCount and clearAll and TypeScript compilation succeeds.