Skip to content

Commit 366184d

Browse files
committed
docs: fix hardcoded note heights in geometry calculations - getNoteRect now accepts heights parameter from useNoteHeights, useArrowDrag uses actual note heights for source origin, and fitToScreen prioritizes selected notes over all root notes
1 parent 39035f4 commit 366184d

13 files changed

Lines changed: 135 additions & 19 deletions

docs/SPATIAL_PARITY_CHECKLIST.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ Every field from legacy `INoteCollab` and `IArrowCollab` is present in the new Y
276276

277277
- [x] Checklist contains ≥ 60 rows. (Current count: **82+ rows**)
278278
- [x] Schema diff table covers every legacy `INoteCollab` and `IArrowCollab` field.
279-
- [x] Every "Done" item has a passing automated test. **205 tests passing across 28 test files in `features/spatial/` as of 2026-06-01.** Test gaps filled: `SpatialPageView.test.ts` now covers delete note (1.2), teleport overlay (1.15), z-index ordering (1.21), zoom reset (4.6), zoom indicator (4.8), selection count (5.7), undo/redo buttons (8.3). `CollabAvatars.test.ts` (4 tests) added for user avatars on canvas (12.27). `PagePropertiesCard.test.ts` (5 tests) added for page properties (12.13). Partial items promoted to Done: note drop zones (1.16), arrow handles (1.17), note frame styling (1.19), scrollbar handling (1.23), arrow color matching (3.12), active element tracking (5.5), search across note head/body (9.2), awareness / remote user avatars (10.4), note properties (12.11), arrow properties (12.12), page properties (12.13), canvas context menu (12.17). Remaining minor gaps: active region tracking (5.6), loading overlay (12.20), some camera pan interactions (4.2/4.3). **Known quality issues discovered in independent evaluation:** `note-geometry.ts:getNoteRect` hardcodes note height as `80px` (should use `useNoteHeights`); `useArrowDrag.ts` hardcodes `sourceNote.pos.y + 40` for arrow drag origin; interregional arrows are schema-only (3.16); selection formatting integration is missing; color system lacks legacy `light`/`highlight` variants.
279+
- [x] Every "Done" item has a passing automated test. **208 tests passing across 28 test files in `features/spatial/` as of 2026-06-01.** Test gaps filled: `SpatialPageView.test.ts` now covers delete note (1.2), teleport overlay (1.15), z-index ordering (1.21), zoom reset (4.6), zoom indicator (4.8), selection count (5.7), undo/redo buttons (8.3). `CollabAvatars.test.ts` (4 tests) added for user avatars on canvas (12.27). `PagePropertiesCard.test.ts` (5 tests) added for page properties (12.13). Partial items promoted to Done: note drop zones (1.16), arrow handles (1.17), note frame styling (1.19), scrollbar handling (1.23), arrow color matching (3.12), active element tracking (5.5), search across note head/body (9.2), awareness / remote user avatars (10.4), note properties (12.11), arrow properties (12.12), page properties (12.13), canvas context menu (12.17). Remaining minor gaps: active region tracking (5.6), loading overlay (12.20), some camera pan interactions (4.2/4.3). **Quality issues fixed (2026-06-01):** `note-geometry.ts:getNoteRect` now accepts `heights` parameter from `useNoteHeights` instead of hardcoding `80px`; `useArrowDrag.ts` uses actual note heights for source origin; `useCanvasActions.fitToScreen` now considers selection bounds first. **Remaining quality issues:** interregional arrows are schema-only (3.16); selection formatting integration is missing; color system lacks legacy `light`/`highlight` variants.
280280
- [x] Phase 6 is not declared done until ≥ 80% of rows are **Done**. **MET.** 72 of 82 rows (88%) are now Done.
281281

282282
---

docs/restart-plan/README.md

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,13 @@ A criterion is **not met** until the verification command or check passes in CI.
7777

7878
### Phase 6 — Spatial canvas polish (**Complete**)
7979

80-
All major deliverables implemented and tested. 88% of `docs/SPATIAL_PARITY_CHECKLIST.md` rows are Done. Independent evaluation (see `phase-6-spatial-polish.md`) confirmed genuine parity with a cleaner architecture, but identified these gaps ordered by severity:
80+
All major deliverables implemented and tested. 88% of `docs/SPATIAL_PARITY_CHECKLIST.md` rows are Done. Independent evaluation (see `phase-6-spatial-polish.md`) confirmed genuine parity with a cleaner architecture. Two critical gaps were **fixed on 2026-06-01**; remaining gaps are non-blocking:
8181

82-
1. **`note-geometry.ts` hardcodes note height as `80px`** (`getNoteRect`). Affects box selection accuracy and container overlap detection. `DisplayNote.vue` publishes real heights via `useNoteHeights`, but `getNoteRect` ignores them. `useArrowDrag.ts` also hardcodes `sourceNote.pos.y + 40` for arrow drag origin. **Functional bug — should be fixed before cutover.**
83-
2. **`fitToScreen` only uses `rootNoteList` bounds.** Legacy `PageCamera.fitToScreen()` considers selection first, then falls back to all page elements.
82+
**Fixed (2026-06-01):**
83+
1. ~~**`note-geometry.ts` hardcodes note height as `80px`**~~ `getNoteRect` now accepts `heights` from `useNoteHeights`. All callers (`useBoxSelection`, `useNoteDrag`, `useArrowReconnect`) pass actual rendered heights. `useArrowDrag.ts` no longer hardcodes `sourceNote.pos.y + 40`.
84+
2. ~~**`fitToScreen` only uses `rootNoteList` bounds**~~ `useCanvasActions` now accepts `selectedNoteIds`; `fitToScreen` prioritizes selected notes, falling back to all root notes.
85+
86+
**Remaining (non-blocking):**
8487
3. **Interregional arrows are schema-only.** `interregional`, `fakePos`, `looseEndpoint` fields exist in Yjs but `DisplayArrow.vue` does not render cross-region arrows with fake endpoints.
8588
4. **Selection formatting integration is missing.** Legacy `PageSelection.format()` allowed applying bold/italic/etc across all selected note editors. No equivalent in new code.
8689
5. **Color system is simplified.** Legacy had `light`/`highlight`/`base`/`final` color variants via `lightenByRatio`. New code uses flat 10-color map with `/18` opacity tint only.
@@ -104,7 +107,7 @@ Pending infrastructure/deployment:
104107

105108
- **Realtime notification toast** — only `/notifications` page exists, no badge/toast.
106109
- **Auth: `rememberDevice` UI missing in login**`LoginView.vue` has no "Remember this device" checkbox for 2FA login; users are re-prompted every time. API schema already supports it.
107-
- **Spatial: hardcoded note heights in geometry**`note-geometry.ts:getNoteRect` hardcodes `const h = 80;` and `useArrowDrag.ts` hardcodes `sourceNote.pos.y + 40`. These should read actual rendered heights from `useNoteHeights` before cutover. See `phase-6-spatial-polish.md` Evaluation findings.
110+
- ~~**Spatial: hardcoded note heights in geometry**~~ **FIXED (2026-06-01).** `note-geometry.ts:getNoteRect` now reads actual rendered heights via optional `heights` parameter. `useArrowDrag.ts` uses `noteHeights` for source origin. See `phase-6-spatial-polish.md` Evaluation findings.
108111
- **Auth: no distributed locking** — Legacy used Redlock (`user-lock:${userId}`) around password change, email change, and 2FA mutations. New code relies on DB transactions only.
109112

110113
---

docs/restart-plan/phase-6-spatial-polish.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,8 @@ An independent codebase audit compared legacy (`apps/client/src/code/pages/page/
125125
4. **Test coverage is strong.** 205 tests across 28 files using real Yjs documents (not mocks).
126126

127127
### Gaps identified (ordered by severity)
128-
1. **`note-geometry.ts` hardcodes note height as `80px`** (`getNoteRect`). This affects box selection accuracy and container overlap detection. `DisplayNote.vue` publishes real heights via `useNoteHeights`, but `getNoteRect` ignores them. Similarly, `useArrowDrag.ts` hardcodes `sourceNote.pos.y + 40` for arrow drag origin. **Functional bug — should be fixed before cutover.**
129-
2. **`fitToScreen` only uses `rootNoteList` bounds.** Legacy `PageCamera.fitToScreen()` considers selection first, then falls back to all page elements.
128+
1. ~~**`note-geometry.ts` hardcodes note height as `80px`**~~ **FIXED (2026-06-01).** `getNoteRect` now accepts an optional `heights` parameter and reads actual rendered heights from `useNoteHeights`. All callers (`useBoxSelection`, `useNoteDrag`, `useArrowReconnect`) updated to pass heights. `useArrowDrag.ts` now uses `noteHeights.value.get(sourceNote.id) ?? 80` instead of hardcoded `+ 40`. New tests added in `note-geometry.test.ts` and `useCanvasActions.test.ts`.
129+
2. ~~**`fitToScreen` only uses `rootNoteList` bounds**~~ **FIXED (2026-06-01).** `useCanvasActions` now accepts `selectedNoteIds` and `fitToScreen` prioritizes selected note bounds, falling back to all root notes when nothing is selected. Legacy behavior matched.
130130
3. **Interregional arrows are schema-only.** `arrow-model.ts` exposes `interregional`, `fakePos`, `looseEndpoint`, but `DisplayArrow.vue` does not implement legacy's sophisticated interregional rendering (cross-region arrows with fake endpoints).
131131
4. **Selection formatting integration is missing.** Legacy `PageSelection.format()`, `toggleMark()`, `toggleNode()` allow applying bold/italic/etc across all selected note editors. New selection has no equivalent.
132132
5. **Color system is simplified.** Legacy uses `colorNameToColorHex` with `light`/`highlight`/`base`/`final` variants (`lightenByRatio`). New code uses a flat hardcoded 10-color map with a single `/18` opacity tint.
@@ -137,7 +137,7 @@ An independent codebase audit compared legacy (`apps/client/src/code/pages/page/
137137

138138
## Verification
139139

140-
- [x] Each deliverable has a test (unit, component, or integration). **Met.** 205 tests passing across 28 test files in `features/spatial/`.
140+
- [x] Each deliverable has a test (unit, component, or integration). **Met.** 208 tests passing across 28 test files in `features/spatial/`.
141141
- [x] Phase 1 checklist is >80% marked done. **MET.** 72 of 82 rows (88%) are Done.
142142

143143
---
@@ -153,7 +153,7 @@ An independent codebase audit compared legacy (`apps/client/src/code/pages/page/
153153
- [x] `DisplayArrow.vue` supports full legacy arrow behavior. **Done.** Curve/line bodies and heads work; line body has rectangle-edge intersection; arrow color matching is implemented and tested. Interregional coordinate transforms and `fakePos`/`looseEndpoint` rendering remain minor gaps.
154154
- [x] `MainToolbar`, `LeftSidebar`, `RightSidebar`, and `TableContextMenu` are implemented as standalone shadcn components and visible on `/pages/:pageId`. `PageToolbarActions.vue` (insert note/arrow, zoom in/out, fit-to-screen) is wired into the toolbar actions slot.
155155
- [x] Sidebar panels (`RecentPages`, `FavoritePages`) display real data from API.
156-
- [ ] Arrow geometry and `fitToScreen` read actual note heights instead of hardcoding `80px`. **NOT MET.** `note-geometry.ts:getNoteRect` still hardcodes `const h = 80;`. `useArrowDrag.ts` hardcodes `sourceNote.pos.y + 40` for arrow drag origin. These should read actual rendered heights via `useNoteHeights` before cutover.
156+
- [x] Arrow geometry and `fitToScreen` read actual note heights instead of hardcoding `80px`. **MET (2026-06-01).** `note-geometry.ts:getNoteRect` now accepts optional `heights` parameter and falls back to `80` only when height is unavailable. `useArrowDrag.ts` uses `noteHeights.value.get(sourceNote.id) ?? 80` for source origin. `useCanvasActions.fitToScreen` already used heights correctly; now also considers selection bounds first.
157157
- [x] Per-note context menu (`NoteContextMenu.vue`) implemented with bring-to-front, send-to-back, and delete actions. Tested via `useNoteContextMenu.test.ts` (5 tests).
158158
- [x] `SpatialPageView.vue` is refactored to avoid god-component anti-pattern. Keyboard shortcuts extracted to `useSpatialKeyboard.ts`; box selection extracted to `useBoxSelection.ts`; arrow drag extracted to `useArrowDrag.ts`; arrow reconnect extracted to `useArrowReconnect.ts`; note drag extracted to `useNoteDrag.ts`; note geometry extracted to `note-geometry.ts`; canvas actions extracted to `useCanvasActions.ts`; canvas context menu handlers extracted to `useCanvasContextMenu.ts`; per-note context menu handlers extracted to `useNoteContextMenu.ts`. Component reduced from ~740 to ~260 lines.
159159
- [x] Selection implements `bringToTop`. Formatting integration and active element/region navigation remain missing.

new-deepnotes/apps/web/src/features/spatial/SpatialPageView.vue

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ const {
182182
const { onCanvasDoubleClick, fitToScreen } = useCanvasActions({
183183
canvasRef,
184184
rootNoteList,
185+
selectedNoteIds: selection.selectedIds,
185186
createNoteAt,
186187
defaultNoteTemplate: props.defaultNoteTemplate,
187188
});

new-deepnotes/apps/web/src/features/spatial/note-geometry.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,43 @@ describe("note-geometry", () => {
9999
});
100100
});
101101

102+
it("reads actual height from heights map when provided", () => {
103+
const noteList = [
104+
{
105+
id: "n1",
106+
model: {
107+
pos: { value: { x: 0, y: 0 } },
108+
width: { value: { expanded: "Auto" } },
109+
} as unknown as NoteModel,
110+
},
111+
];
112+
const heights = new Map([["n1", 120]]);
113+
expect(getNoteRect("n1", noteList, new Map(), heights)).toEqual({
114+
x: 0,
115+
y: 0,
116+
width: 160,
117+
height: 120,
118+
});
119+
});
120+
121+
it("falls back to 80 when heights map is missing the note", () => {
122+
const noteList = [
123+
{
124+
id: "n1",
125+
model: {
126+
pos: { value: { x: 0, y: 0 } },
127+
width: { value: { expanded: "Auto" } },
128+
} as unknown as NoteModel,
129+
},
130+
];
131+
expect(getNoteRect("n1", noteList, new Map(), new Map())).toEqual({
132+
x: 0,
133+
y: 0,
134+
width: 160,
135+
height: 80,
136+
});
137+
});
138+
102139
it("returns null for missing note", () => {
103140
expect(getNoteRect("missing", [], new Map())).toBeNull();
104141
});

new-deepnotes/apps/web/src/features/spatial/note-geometry.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,14 @@ export function getNoteRect(
4646
noteId: string,
4747
noteList: { id: string; model: NoteModel }[],
4848
parentOf: Map<string, string>,
49+
heights?: Map<string, number>,
4950
): NoteRect | null {
5051
const entry = noteList.find((n) => n.id === noteId);
5152
if (!entry) return null;
5253
const pos = getNoteEffectiveWorldPos(noteId, noteList, parentOf);
5354
if (!pos) return null;
5455
const wStr = entry.model.width.value.expanded;
5556
const w = wStr === "Auto" ? 160 : parseFloat(wStr);
56-
const h = 80;
57+
const h = heights?.get(noteId) ?? 80;
5758
return { x: pos.x, y: pos.y, width: w, height: h };
5859
}

new-deepnotes/apps/web/src/features/spatial/useArrowDrag.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { computed, ref } from "vue";
22
import type { Ref } from "vue";
33
import type { NoteModel } from "./note-model";
44
import { worldToScreen } from "./spatial-viewport-math";
5+
import { useNoteHeights } from "./useNoteHeights";
56
import type { ClipboardArrow } from "./clipboard";
67

78
export interface UseArrowDragInput {
@@ -17,6 +18,7 @@ export interface UseArrowDragInput {
1718
}
1819

1920
export function useArrowDrag(input: UseArrowDragInput) {
21+
const { heights: noteHeights } = useNoteHeights();
2022
const arrowDrag = ref<{
2123
sourceId: string;
2224
endX: number;
@@ -39,9 +41,10 @@ export function useArrowDrag(input: UseArrowDragInput) {
3941

4042
const wStr = sourceNote.model.width.value.expanded;
4143
const w = wStr === "Auto" ? 160 : parseFloat(wStr);
44+
const h = noteHeights.value.get(sourceNote.id) ?? 80;
4245
const sourceScreen = worldToScreen(
4346
sourceNote.model.pos.value.x + w / 2,
44-
sourceNote.model.pos.value.y + 40,
47+
sourceNote.model.pos.value.y + h / 2,
4548
cx,
4649
cy,
4750
camX,

new-deepnotes/apps/web/src/features/spatial/useArrowReconnect.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { NoteModel } from "./note-model";
44
import type { ArrowModel } from "./arrow-model";
55
import { screenToWorld } from "./spatial-viewport-math";
66
import { getNoteRect } from "./note-geometry";
7+
import { useNoteHeights } from "./useNoteHeights";
78

89
export interface UseArrowReconnectInput {
910
canvasRef: Ref<{
@@ -18,6 +19,7 @@ export interface UseArrowReconnectInput {
1819
}
1920

2021
export function useArrowReconnect(input: UseArrowReconnectInput) {
22+
const { heights: noteHeights } = useNoteHeights();
2123
const reconnectingArrowId = ref<string | null>(null);
2224
const reconnectingFrom = ref<'source' | 'target' | null>(null);
2325
const hoveredNoteId = ref<string | null>(null);
@@ -55,7 +57,7 @@ export function useArrowReconnect(input: UseArrowReconnectInput) {
5557
let bestNoteId: string | null = null;
5658

5759
for (const note of input.noteList.value) {
58-
const noteRect = getNoteRect(note.id, input.noteList.value, input.parentOf.value);
60+
const noteRect = getNoteRect(note.id, input.noteList.value, input.parentOf.value, noteHeights.value);
5961
if (!noteRect) continue;
6062

6163
if (

new-deepnotes/apps/web/src/features/spatial/useBoxSelection.test.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,22 @@
1-
import { describe, expect, it } from "vitest";
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
22
import { ref } from "vue";
33
import { useBoxSelection } from "./useBoxSelection";
44
import { useSpatialSelection } from "./selection";
55
import { useSpatialEditing } from "./useSpatialEditing";
66
import type { NoteModel } from "./note-model";
7+
import { useNoteHeights } from "./useNoteHeights";
8+
9+
vi.mock("./useNoteHeights", () => ({
10+
useNoteHeights: vi.fn(),
11+
}));
712

813
describe("useBoxSelection", () => {
14+
beforeEach(() => {
15+
(useNoteHeights as ReturnType<typeof vi.fn>).mockReturnValue({
16+
heights: ref(new Map<string, number>()),
17+
});
18+
});
19+
920
function setup() {
1021
const selection = useSpatialSelection();
1122
const editing = useSpatialEditing();

new-deepnotes/apps/web/src/features/spatial/useBoxSelection.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { SpatialEditing } from "./useSpatialEditing";
44
import type { NoteModel } from "./note-model";
55
import { screenToWorld } from "./spatial-viewport-math";
66
import { getNoteRect, rectsIntersect } from "./note-geometry";
7+
import { useNoteHeights } from "./useNoteHeights";
78

89
const DRAG_THRESHOLD = 4;
910

@@ -29,6 +30,7 @@ export interface UseBoxSelectionInput {
2930
}
3031

3132
export function useBoxSelection(input: UseBoxSelectionInput) {
33+
const { heights: noteHeights } = useNoteHeights();
3234
let boxState: BoxSelectionState | null = null;
3335

3436
function onCanvasPointerDown(e: PointerEvent) {
@@ -96,7 +98,7 @@ export function useBoxSelection(input: UseBoxSelectionInput) {
9698
const boxH = Math.max(w1.y, w2.y) - boxY;
9799

98100
for (const note of input.rootNoteList.value) {
99-
const noteRect = getNoteRect(note.id, input.noteList.value, input.parentOf.value);
101+
const noteRect = getNoteRect(note.id, input.noteList.value, input.parentOf.value, noteHeights.value);
100102
if (!noteRect) continue;
101103

102104
if (

0 commit comments

Comments
 (0)