Skip to content

Commit 5c7364b

Browse files
gustavotoyotagustavotoyota
committed
fix(web): fix note property mutations in PageEditorView
Vue 3's automatic ref unwrapping in reactive proxies broke mutations on nested refs inside noteModel objects passed to the properties panel. Changes: - Use shallowRef for selectedNoteModel/selectedArrowModel to prevent Vue from auto-unwrapping nested Yjs-backed refs (colorValue, etc.) - Expose writeable refs (colorValue, widthExpanded, posX, anchorX, etc.) from useNoteModel so consumers can mutate Yjs data directly - Make yjs-reactivity.ts two-way (watch + flush: sync) so Vue ref assignments write back to Yjs maps - Fix NotePropertiesCard color computed to use colorValue (string) instead of color (computed object) for ColorPalette :model-value - Extract all inline @update:* handlers in PageEditorView.vue into explicit handler functions to avoid Vue compiler quirks Fixes color, width, height, pos, anchor, collapsible, and container property changes in the right sidebar. Includes regression tests.
1 parent daa8e71 commit 5c7364b

5 files changed

Lines changed: 218 additions & 46 deletions

File tree

‎new-deepnotes/apps/web/src/features/pages/PageEditorView.vue‎

Lines changed: 89 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<script setup lang="ts">
2-
import { computed, onMounted, ref, watch } from "vue";
2+
import { computed, onMounted, ref, shallowRef, watch } from "vue";
33
import { RouterLink, useRoute, useRouter } from "vue-router";
44
55
import { Button } from "@/components/ui/button";
@@ -79,11 +79,60 @@ const spatialViewRef = ref<any>(null);
7979
8080
// Track selected note for properties panel
8181
const selectedNoteId = ref<string | null>(null);
82-
const selectedNoteModel = ref<any>(null);
82+
const selectedNoteModel = shallowRef<any>(null);
8383
8484
// Track selected arrow for properties panel
8585
const selectedArrowId = ref<string | null>(null);
86-
const selectedArrowModel = ref<any>(null);
86+
const selectedArrowModel = shallowRef<any>(null);
87+
88+
function handleSelectNote(id: string | null, model: any) {
89+
selectedNoteId.value = id ?? null;
90+
selectedNoteModel.value = model ?? null;
91+
}
92+
93+
function handleSelectArrow(id: string | null, model: any) {
94+
selectedArrowId.value = id ?? null;
95+
selectedArrowModel.value = model ?? null;
96+
}
97+
98+
// Note property update handlers
99+
function onUpdateNoteLink(v: string) { if (selectedNoteModel.value) selectedNoteModel.value.link.value = v; }
100+
function onUpdateNoteHeadEnabled(v: boolean) { if (selectedNoteModel.value) selectedNoteModel.value.head.enabled.value = v; }
101+
function onUpdateNoteBodyEnabled(v: boolean) { if (selectedNoteModel.value) selectedNoteModel.value.body.enabled.value = v; }
102+
function onUpdateNoteHeadWrap(v: boolean) { if (selectedNoteModel.value) selectedNoteModel.value.head.wrap.value = v; }
103+
function onUpdateNoteBodyWrap(v: boolean) { if (selectedNoteModel.value) selectedNoteModel.value.body.wrap.value = v; }
104+
function onUpdateNotePosX(v: number) { if (selectedNoteModel.value) selectedNoteModel.value.posX.value = v; }
105+
function onUpdateNotePosY(v: number) { if (selectedNoteModel.value) selectedNoteModel.value.posY.value = v; }
106+
function onUpdateNoteAnchorX(v: number) { if (selectedNoteModel.value) selectedNoteModel.value.anchorX.value = v; }
107+
function onUpdateNoteAnchorY(v: number) { if (selectedNoteModel.value) selectedNoteModel.value.anchorY.value = v; }
108+
function onUpdateNoteWidth(v: string) { if (selectedNoteModel.value) selectedNoteModel.value.widthExpanded.value = v; }
109+
function onUpdateNoteHeight(v: string) { if (selectedNoteModel.value) selectedNoteModel.value.heightExpanded.value = v; }
110+
function onUpdateNoteColor(v: string) { if (selectedNoteModel.value) selectedNoteModel.value.colorValue.value = v; }
111+
function onUpdateNoteColorInherit(v: boolean) { if (selectedNoteModel.value) selectedNoteModel.value.colorInherit.value = v; }
112+
function onUpdateNoteCollapsible(v: boolean) { if (selectedNoteModel.value) selectedNoteModel.value.collapsing.enabled.value = v; }
113+
function onUpdateNoteCollapsed(v: boolean) { if (selectedNoteModel.value) selectedNoteModel.value.collapsing.collapsed.value = v; }
114+
function onUpdateNoteMovable(v: boolean) { if (selectedNoteModel.value) selectedNoteModel.value.movable.value = v; }
115+
function onUpdateNoteResizable(v: boolean) { if (selectedNoteModel.value) selectedNoteModel.value.resizable.value = v; }
116+
function onUpdateNoteReadOnly(v: boolean) { if (selectedNoteModel.value) selectedNoteModel.value.readOnly.value = v; }
117+
function onUpdateNoteContainerEnabled(v: boolean) { if (selectedNoteModel.value) selectedNoteModel.value.container.enabled.value = v; }
118+
function onUpdateNoteContainerHorizontal(v: boolean) { if (selectedNoteModel.value) selectedNoteModel.value.container.horizontal.value = v; }
119+
function onUpdateNoteContainerSpatial(v: boolean) { if (selectedNoteModel.value) selectedNoteModel.value.container.spatial.value = v; }
120+
function onUpdateNoteContainerWrapChildren(v: boolean) { if (selectedNoteModel.value) selectedNoteModel.value.container.wrapChildren.value = v; }
121+
function onUpdateNoteContainerStretchChildren(v: boolean) { if (selectedNoteModel.value) selectedNoteModel.value.container.stretchChildren.value = v; }
122+
function onUpdateNoteContainerForceColorInheritance(v: boolean) { if (selectedNoteModel.value) selectedNoteModel.value.container.forceColorInheritance.value = v; }
123+
function onUpdateNoteLocalCollapsing(v: boolean) { if (selectedNoteModel.value) selectedNoteModel.value.collapsing.localCollapsing.value = v; }
124+
function onUpdateNoteLocallyCollapsed(v: boolean) { if (selectedNoteModel.value) selectedNoteModel.value.collapsing.locallyCollapsed.value = v; }
125+
126+
// Arrow property update handlers
127+
function onUpdateArrowBodyType(v: string) { if (selectedArrowModel.value) selectedArrowModel.value.bodyType.value = v; }
128+
function onUpdateArrowBodyStyle(v: string) { if (selectedArrowModel.value) selectedArrowModel.value.bodyStyle.value = v; }
129+
function onUpdateArrowSourceHead(v: string) { if (selectedArrowModel.value) selectedArrowModel.value.sourceHead.value = v; }
130+
function onUpdateArrowTargetHead(v: string) { if (selectedArrowModel.value) selectedArrowModel.value.targetHead.value = v; }
131+
function onUpdateArrowColor(v: string) { if (selectedArrowModel.value) selectedArrowModel.value.color.value = v; }
132+
function onUpdateArrowColorInherit(_v: boolean) { /* arrow model has no color.inherit */ }
133+
function onUpdateArrowReadOnly(v: boolean) { if (selectedArrowModel.value) selectedArrowModel.value.readOnly.value = v; }
134+
function onUpdateArrowSourceAnchor(v: string) { if (selectedArrowModel.value) selectedArrowModel.value.sourceAnchor.value = v === 'null' ? null : JSON.parse(v); }
135+
function onUpdateArrowTargetAnchor(v: string) { if (selectedArrowModel.value) selectedArrowModel.value.targetAnchor.value = v === 'null' ? null : JSON.parse(v); }
87136
88137
// Track recent/favorite/selected pages for left sidebar
89138
const {
@@ -644,8 +693,8 @@ onMounted(() => {
644693
:awareness="collabAwareness"
645694
:default-note-template="noteTemplate"
646695
:default-arrow-template="arrowTemplate"
647-
@select-note="selectedNoteId = $event?.[0] ?? null; selectedNoteModel = $event?.[1] ?? null"
648-
@select-arrow="selectedArrowId = $event?.[0] ?? null; selectedArrowModel = $event?.[1] ?? null"
696+
@select-note="(id, model) => handleSelectNote(id, model)"
697+
@select-arrow="(id, model) => handleSelectArrow(id, model)"
649698
/>
650699

651700
<!-- === Toolbar center: breadcrumb path === -->
@@ -782,32 +831,32 @@ onMounted(() => {
782831
:note-id="selectedNoteId"
783832
:note-model="selectedNoteModel"
784833
:read-only="cryptoError !== null"
785-
@update:link="selectedNoteModel.link.value = $event"
786-
@update:head-enabled="selectedNoteModel.head.enabled.value = $event"
787-
@update:body-enabled="selectedNoteModel.body.enabled.value = $event"
788-
@update:head-wrap="selectedNoteModel.head.wrap.value = $event"
789-
@update:body-wrap="selectedNoteModel.body.wrap.value = $event"
790-
@update:pos-x="selectedNoteModel.pos.value.x = $event"
791-
@update:pos-y="selectedNoteModel.pos.value.y = $event"
792-
@update:anchor-x="selectedNoteModel.anchor.value.x = $event"
793-
@update:anchor-y="selectedNoteModel.anchor.value.y = $event"
794-
@update:width="selectedNoteModel.width.value.expanded = $event"
795-
@update:height="selectedNoteModel.height.value.expanded = $event"
796-
@update:color="selectedNoteModel.color.value = $event"
797-
@update:color-inherit="selectedNoteModel.color.inherit.value = $event"
798-
@update:collapsible="selectedNoteModel.collapsing.enabled.value = $event"
799-
@update:collapsed="selectedNoteModel.collapsing.collapsed.value = $event"
800-
@update:movable="selectedNoteModel.movable.value = $event"
801-
@update:resizable="selectedNoteModel.resizable.value = $event"
802-
@update:read-only="selectedNoteModel.readOnly.value = $event"
803-
@update:container-enabled="selectedNoteModel.container.enabled.value = $event"
804-
@update:container-horizontal="selectedNoteModel.container.horizontal.value = $event"
805-
@update:container-spatial="selectedNoteModel.container.spatial.value = $event"
806-
@update:container-wrap-children="selectedNoteModel.container.wrapChildren.value = $event"
807-
@update:container-stretch-children="selectedNoteModel.container.stretchChildren.value = $event"
808-
@update:container-force-color-inheritance="selectedNoteModel.container.forceColorInheritance.value = $event"
809-
@update:local-collapsing="selectedNoteModel.collapsing.localCollapsing.value = $event"
810-
@update:locally-collapsed="selectedNoteModel.collapsing.locallyCollapsed.value = $event"
834+
@update:link="onUpdateNoteLink($event)"
835+
@update:head-enabled="onUpdateNoteHeadEnabled($event)"
836+
@update:body-enabled="onUpdateNoteBodyEnabled($event)"
837+
@update:head-wrap="onUpdateNoteHeadWrap($event)"
838+
@update:body-wrap="onUpdateNoteBodyWrap($event)"
839+
@update:pos-x="onUpdateNotePosX($event)"
840+
@update:pos-y="onUpdateNotePosY($event)"
841+
@update:anchor-x="onUpdateNoteAnchorX($event)"
842+
@update:anchor-y="onUpdateNoteAnchorY($event)"
843+
@update:width="onUpdateNoteWidth($event)"
844+
@update:height="onUpdateNoteHeight($event)"
845+
@update:color="onUpdateNoteColor($event)"
846+
@update:color-inherit="onUpdateNoteColorInherit($event)"
847+
@update:collapsible="onUpdateNoteCollapsible($event)"
848+
@update:collapsed="onUpdateNoteCollapsed($event)"
849+
@update:movable="onUpdateNoteMovable($event)"
850+
@update:resizable="onUpdateNoteResizable($event)"
851+
@update:read-only="onUpdateNoteReadOnly($event)"
852+
@update:container-enabled="onUpdateNoteContainerEnabled($event)"
853+
@update:container-horizontal="onUpdateNoteContainerHorizontal($event)"
854+
@update:container-spatial="onUpdateNoteContainerSpatial($event)"
855+
@update:container-wrap-children="onUpdateNoteContainerWrapChildren($event)"
856+
@update:container-stretch-children="onUpdateNoteContainerStretchChildren($event)"
857+
@update:container-force-color-inheritance="onUpdateNoteContainerForceColorInheritance($event)"
858+
@update:local-collapsing="onUpdateNoteLocalCollapsing($event)"
859+
@update:locally-collapsed="onUpdateNoteLocallyCollapsed($event)"
811860
@create-new-page="handleCreateNewPage"
812861
@swap-head-body="handleSwapHeadBody"
813862
@copy-link="handleCopyNoteLink"
@@ -821,15 +870,15 @@ onMounted(() => {
821870
:arrow-id="selectedArrowId"
822871
:arrow-model="selectedArrowModel"
823872
:read-only="cryptoError !== null"
824-
@update:body-type="selectedArrowModel.bodyType.value = $event"
825-
@update:body-style="selectedArrowModel.bodyStyle.value = $event"
826-
@update:source-head="selectedArrowModel.sourceHead.value = $event"
827-
@update:target-head="selectedArrowModel.targetHead.value = $event"
828-
@update:color="selectedArrowModel.color.value = $event"
829-
@update:color-inherit="selectedArrowModel.color.inherit.value = $event"
830-
@update:read-only="selectedArrowModel.readOnly.value = $event"
831-
@update:source-anchor="selectedArrowModel.sourceAnchor.value = $event === 'null' ? null : JSON.parse($event)"
832-
@update:target-anchor="selectedArrowModel.targetAnchor.value = $event === 'null' ? null : JSON.parse($event)"
873+
@update:body-type="onUpdateArrowBodyType($event)"
874+
@update:body-style="onUpdateArrowBodyStyle($event)"
875+
@update:source-head="onUpdateArrowSourceHead($event)"
876+
@update:target-head="onUpdateArrowTargetHead($event)"
877+
@update:color="onUpdateArrowColor($event)"
878+
@update:color-inherit="onUpdateArrowColorInherit($event)"
879+
@update:read-only="onUpdateArrowReadOnly($event)"
880+
@update:source-anchor="onUpdateArrowSourceAnchor($event)"
881+
@update:target-anchor="onUpdateArrowTargetAnchor($event)"
833882
@swap-arrowheads="handleSwapArrowheads"
834883
@copy-link="handleCopyArrowLink"
835884
@set-as-default="handleSetArrowAsDefault"

‎new-deepnotes/apps/web/src/features/spatial/NotePropertiesCard.vue‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ const anchorX = computed(() => props.noteModel?.anchor?.value?.x ?? 0.5)
6868
const anchorY = computed(() => props.noteModel?.anchor?.value?.y ?? 0.5)
6969
const width = computed(() => props.noteModel?.width?.value?.expanded ?? 'Auto')
7070
const height = computed(() => props.noteModel?.height?.value?.expanded ?? 'Auto')
71-
const color = computed(() => (props.noteModel?.color?.value as string) ?? 'grey')
72-
const colorInherit = computed(() => props.noteModel?.color?.inherit?.value ?? false)
71+
const color = computed(() => props.noteModel?.colorValue?.value ?? 'grey')
72+
const colorInherit = computed(() => props.noteModel?.colorInherit?.value ?? false)
7373
const collapsible = computed(() => props.noteModel?.collapsing?.enabled?.value ?? false)
7474
const collapsed = computed(() => props.noteModel?.collapsing?.collapsed?.value ?? false)
7575
const movable = computed(() => props.noteModel?.movable?.value ?? true)

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

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,4 +243,81 @@ describe("note-model reactivity", () => {
243243

244244
expect(model.width.value.collapsed).toBe("Minimum");
245245
});
246+
247+
it("allows direct write to colorValue and colorInherit refs", () => {
248+
const ydoc = createPageYDoc();
249+
const note = addNoteToPage(ydoc, "n1");
250+
const model = useNoteModel(note);
251+
252+
expect(model.color.value).toEqual({ inherit: false, value: "grey" });
253+
254+
model.colorValue.value = "blue";
255+
model.colorInherit.value = true;
256+
257+
expect(model.color.value).toEqual({ inherit: true, value: "blue" });
258+
259+
const colorMap = note.get(YPAGE_NOTE_KEY.color) as Y.Map<unknown>;
260+
expect(colorMap.get("value")).toBe("blue");
261+
expect(colorMap.get("inherit")).toBe(true);
262+
});
263+
264+
it("allows direct write to widthExpanded and widthCollapsed refs", () => {
265+
const ydoc = createPageYDoc();
266+
const note = addNoteToPage(ydoc, "n1");
267+
const model = useNoteModel(note);
268+
269+
model.widthExpanded.value = "200px";
270+
model.widthCollapsed.value = "Minimum";
271+
272+
expect(model.width.value).toEqual({ expanded: "200px", collapsed: "Minimum" });
273+
274+
const widthMap = note.get(YPAGE_NOTE_KEY.width) as Y.Map<string>;
275+
expect(widthMap.get("expanded")).toBe("200px");
276+
expect(widthMap.get("collapsed")).toBe("Minimum");
277+
});
278+
279+
it("allows direct write to heightExpanded and heightCollapsed refs", () => {
280+
const ydoc = createPageYDoc();
281+
const note = addNoteToPage(ydoc, "n1");
282+
const model = useNoteModel(note);
283+
284+
model.heightExpanded.value = "120px";
285+
model.heightCollapsed.value = "Auto";
286+
287+
expect(model.height.value).toEqual({ expanded: "120px", collapsed: "Auto" });
288+
289+
const heightMap = note.get(YPAGE_NOTE_KEY.height) as Y.Map<string>;
290+
expect(heightMap.get("expanded")).toBe("120px");
291+
expect(heightMap.get("collapsed")).toBe("Auto");
292+
});
293+
294+
it("allows direct write to posX and posY refs", () => {
295+
const ydoc = createPageYDoc();
296+
const note = addNoteToPage(ydoc, "n1");
297+
const model = useNoteModel(note);
298+
299+
model.posX.value = 150;
300+
model.posY.value = 250;
301+
302+
expect(model.pos.value).toEqual({ x: 150, y: 250 });
303+
304+
const posMap = note.get(YPAGE_NOTE_KEY.pos) as Y.Map<number>;
305+
expect(posMap.get("x")).toBe(150);
306+
expect(posMap.get("y")).toBe(250);
307+
});
308+
309+
it("allows direct write to anchorX and anchorY refs", () => {
310+
const ydoc = createPageYDoc();
311+
const note = addNoteToPage(ydoc, "n1");
312+
const model = useNoteModel(note);
313+
314+
model.anchorX.value = 0.25;
315+
model.anchorY.value = 0.75;
316+
317+
expect(model.anchor.value).toEqual({ x: 0.25, y: 0.75 });
318+
319+
const anchorMap = note.get(YPAGE_NOTE_KEY.anchor) as Y.Map<number>;
320+
expect(anchorMap.get("x")).toBe(0.25);
321+
expect(anchorMap.get("y")).toBe(0.75);
322+
});
246323
});

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,14 @@ export function useNoteModel(noteMap: Y.Map<unknown>) {
145145

146146
return {
147147
pos,
148+
posX,
149+
posY,
148150
width,
151+
widthExpanded,
152+
widthCollapsed,
149153
height,
154+
heightExpanded,
155+
heightCollapsed,
150156
head: {
151157
enabled: headEnabled,
152158
wrap: headWrap,
@@ -175,12 +181,16 @@ export function useNoteModel(noteMap: Y.Map<unknown>) {
175181
localCollapsing: collapsingLocalCollapsing,
176182
},
177183
color,
184+
colorValue,
185+
colorInherit,
178186
zIndex,
179187
link,
180188
movable,
181189
resizable,
182190
readOnly,
183191
anchor,
192+
anchorX,
193+
anchorY,
184194
regionId,
185195
createdAt,
186196
editedAt,

‎new-deepnotes/apps/web/src/features/spatial/yjs-reactivity.ts‎

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { ref, type Ref } from "vue";
1+
import { ref, watch, type Ref } from "vue";
22
import type * as Y from "yjs";
33

44
/**
@@ -24,8 +24,20 @@ export function useYMapNumber(
2424
): Ref<number> {
2525
const value = ref<number>((ymap.get(key) as number) ?? defaultValue);
2626
ymap.observe(() => {
27-
value.value = (ymap.get(key) as number) ?? defaultValue;
27+
const next = (ymap.get(key) as number) ?? defaultValue;
28+
if (value.value !== next) {
29+
value.value = next;
30+
}
2831
});
32+
watch(
33+
value,
34+
(next) => {
35+
if (ymap.get(key) !== next) {
36+
ymap.set(key, next);
37+
}
38+
},
39+
{ flush: "sync" },
40+
);
2941
return value;
3042
}
3143

@@ -36,8 +48,20 @@ export function useYMapBoolean(
3648
): Ref<boolean> {
3749
const value = ref<boolean>((ymap.get(key) as boolean) ?? defaultValue);
3850
ymap.observe(() => {
39-
value.value = (ymap.get(key) as boolean) ?? defaultValue;
51+
const next = (ymap.get(key) as boolean) ?? defaultValue;
52+
if (value.value !== next) {
53+
value.value = next;
54+
}
4055
});
56+
watch(
57+
value,
58+
(next) => {
59+
if (ymap.get(key) !== next) {
60+
ymap.set(key, next);
61+
}
62+
},
63+
{ flush: "sync" },
64+
);
4165
return value;
4266
}
4367

@@ -48,8 +72,20 @@ export function useYMapString(
4872
): Ref<string> {
4973
const value = ref<string>((ymap.get(key) as string) ?? defaultValue);
5074
ymap.observe(() => {
51-
value.value = (ymap.get(key) as string) ?? defaultValue;
75+
const next = (ymap.get(key) as string) ?? defaultValue;
76+
if (value.value !== next) {
77+
value.value = next;
78+
}
5279
});
80+
watch(
81+
value,
82+
(next) => {
83+
if (ymap.get(key) !== next) {
84+
ymap.set(key, next);
85+
}
86+
},
87+
{ flush: "sync" },
88+
);
5389
return value;
5490
}
5591

0 commit comments

Comments
 (0)