Skip to content

Commit f1d7d61

Browse files
committed
Add per-canvas sync mode flag implementation for Roam Canvas
- Introduced a new `canvasSyncMode` flag to explicitly manage sync behavior for each canvas, replacing the previous implicit heuristic. - Added utilities for reading and writing the sync mode, ensuring persistence and defaulting to 'local'. - Updated canvas entrypoint and main menu to reflect the new sync mode, including a toggle for users. - Implemented UI indicators for mixed-version clarity and ensured no heuristic fallback in mode selection. - Established test cases to validate mode persistence and behavior in various scenarios.
1 parent da7012c commit f1d7d61

2 files changed

Lines changed: 165 additions & 0 deletions

File tree

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import getBlockProps, { json } from "~/utils/getBlockProps";
2+
import setBlockProps from "~/utils/setBlockProps";
3+
4+
export type CanvasSyncMode = "local" | "sync";
5+
6+
const QUERY_BUILDER_PROP_KEY = "roamjs-query-builder";
7+
const CANVAS_SYNC_MODE_KEY = "canvasSyncMode";
8+
const DEFAULT_CANVAS_SYNC_MODE: CanvasSyncMode = "local";
9+
10+
const isCanvasSyncMode = (value: unknown): value is CanvasSyncMode =>
11+
value === "local" || value === "sync";
12+
13+
const getRoamJsQueryBuilderProps = (
14+
pageUid: string,
15+
): Record<string, json> => {
16+
const props = getBlockProps(pageUid);
17+
const value = props[QUERY_BUILDER_PROP_KEY];
18+
if (value && typeof value === "object" && !Array.isArray(value)) {
19+
return value;
20+
}
21+
return {};
22+
};
23+
24+
export const getPersistedCanvasSyncMode = ({
25+
pageUid,
26+
}: {
27+
pageUid: string;
28+
}): CanvasSyncMode | null => {
29+
const rjsqb = getRoamJsQueryBuilderProps(pageUid);
30+
const mode = rjsqb[CANVAS_SYNC_MODE_KEY];
31+
return isCanvasSyncMode(mode) ? mode : null;
32+
};
33+
34+
export const getEffectiveCanvasSyncMode = ({
35+
pageUid,
36+
}: {
37+
pageUid: string;
38+
}): CanvasSyncMode => {
39+
return getPersistedCanvasSyncMode({ pageUid }) ?? DEFAULT_CANVAS_SYNC_MODE;
40+
};
41+
42+
export const setCanvasSyncMode = ({
43+
pageUid,
44+
mode,
45+
}: {
46+
pageUid: string;
47+
mode: CanvasSyncMode;
48+
}): void => {
49+
const rjsqb = getRoamJsQueryBuilderProps(pageUid);
50+
setBlockProps(pageUid, {
51+
[QUERY_BUILDER_PROP_KEY]: {
52+
...rjsqb,
53+
[CANVAS_SYNC_MODE_KEY]: mode,
54+
},
55+
});
56+
};
57+
58+
export const ensureCanvasSyncMode = ({
59+
pageUid,
60+
}: {
61+
pageUid: string;
62+
}): CanvasSyncMode => {
63+
const mode = getPersistedCanvasSyncMode({ pageUid });
64+
if (mode) return mode;
65+
setCanvasSyncMode({ pageUid, mode: DEFAULT_CANVAS_SYNC_MODE });
66+
return DEFAULT_CANVAS_SYNC_MODE;
67+
};

eng-1324-plan.md

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
# Per-Canvas Sync Mode Flag (Roam Canvas)
2+
3+
## Summary
4+
Replace the current implicit sync heuristic (`new/empty canvas => sync`) with an explicit persisted per-canvas mode flag.
5+
Canvas mode will be determined only by that flag (`local` or `sync`), with a main menu toggle to change it.
6+
Default for canvases without a flag: `local`.
7+
V0 behavior when enabling sync: start from a new blank sync canvas (no auto import/seed).
8+
9+
## Public API / Interface Changes
10+
1. **Canvas page props (`roamjs-query-builder`)**
11+
- Add `canvasSyncMode?: "local" | "sync"` under `:roamjs-query-builder`.
12+
- Effective mode rules:
13+
- If missing: treat as `"local"` and persist `"local"` on first canvas load.
14+
- If `"sync"`: use Cloudflare sync adapter.
15+
- If `"local"`: use Roam local adapter.
16+
- Remove behavioral dependency on `tldraw` presence/emptiness for mode selection.
17+
18+
## Implementation Plan
19+
1. **Introduce canvas sync mode utilities**
20+
- Add a dedicated utility (e.g. `apps/roam/src/components/canvas/canvasSyncMode.ts`) for:
21+
- reading/writing `canvasSyncMode` from page props
22+
- returning effective mode with defaulting
23+
- ensuring initial persistence of default `"local"`
24+
- Keep prop access normalized via existing `getBlockProps`/`setBlockProps` helpers.
25+
26+
2. **Replace mode selection in canvas entrypoint**
27+
- Update `apps/roam/src/components/canvas/Tldraw.tsx`:
28+
- remove mode selection logic based on `hasRoamPersistedCanvasData(pageUid)`
29+
- select adapter solely by `canvasSyncMode` (+ existing global env guard for sync availability)
30+
- Keep existing `isCloudflareSync` rendering path, but source it from mode flag.
31+
32+
3. **Add sync mode toggle to main menu**
33+
- Update `apps/roam/src/components/canvas/uiOverrides.tsx`:
34+
- add a `TldrawUiMenuCheckboxItem` under `MainMenu` (View or a new Collaboration subgroup)
35+
- label: `Sync mode`
36+
- checked reflects current per-canvas mode
37+
- toggle handler persists mode for current canvas page
38+
- when toggled ON, always show a popup/toast notification: user is starting from a new blank sync canvas
39+
- If sync backend is unavailable (`TLDRAW_CLOUDFLARE_SYNC_ENABLED` false or URL missing), show disabled item with explanatory label.
40+
41+
4. **No import/seed in V0**
42+
- Do not attempt to copy local `roamjs-query-builder.tldraw` data into sync room.
43+
- Do not add worker seed endpoint in V0.
44+
- Sync ON simply opens the sync room as-is; for first-time rooms, this is a blank canvas.
45+
- Track auto-import/manual-import as a future enhancement.
46+
47+
5. **UI indicator for mixed-version clarity**
48+
- Keep and strengthen sync-on indicator in canvas UI (existing cloud badge area in `Tldraw.tsx`):
49+
- show when `canvasSyncMode === "sync"` (not based on heuristic)
50+
- tooltip text clarifies collaborators on older/non-realtime builds may still open local canvas behavior.
51+
- This addresses mixed-version behavior without introducing permission/role complexity.
52+
53+
6. **Cleanup**
54+
- Remove or repurpose `hasRoamPersistedCanvasData` so it no longer influences mode choice.
55+
- Ensure no code path flips mode implicitly due to data existence.
56+
57+
## Test Cases and Scenarios
58+
1. **Mode persistence**
59+
- Open canvas with no flag -> mode resolves to local and persists `canvasSyncMode=local`.
60+
- Toggle ON -> `canvasSyncMode=sync` persisted.
61+
- Reload / open in new session / open as another user -> mode remains sync.
62+
63+
2. **No heuristic fallback**
64+
- Canvas with existing local `tldraw` data and `canvasSyncMode=sync` still opens sync adapter.
65+
- Canvas with empty data and `canvasSyncMode=local` still opens local adapter.
66+
67+
3. **V0 blank-sync behavior**
68+
- Toggle sync ON always shows popup/toast: user is starting from a new blank sync canvas.
69+
- No pre-check for existing local shapes is performed before showing this notification.
70+
- Local canvas data is not imported in V0.
71+
72+
4. **Main menu UX**
73+
- Toggle visible in main menu for current canvas.
74+
- Checked state matches persisted mode.
75+
- Disabled state shown when sync backend unavailable.
76+
77+
5. **Mixed-version indicator**
78+
- Sync mode ON always shows indicator.
79+
- Tooltip communicates possible collaborator mismatch (older build/local-only behavior).
80+
81+
6. **Regression checks**
82+
- Legacy canvas upgrade flow still works.
83+
- Local canvas save/remote pull-watch flow remains unchanged in local mode.
84+
85+
## Acceptance Criteria Mapping
86+
- Per-canvas flag exists and is persisted: `canvasSyncMode`.
87+
- Main menu toggle exists and updates current canvas mode.
88+
- Mode selection uses flag only; no empty-canvas heuristic.
89+
- Opening canvases with data no longer forces local mode.
90+
- V0 sync toggle intentionally does not import local data.
91+
92+
## Assumptions and Defaults
93+
- **Chosen default**: canvases without explicit mode default to and persist as `local`.
94+
- **Chosen V0 toggle behavior**: enabling sync switches to sync room directly and shows a blank-canvas notification.
95+
- No permissions model added for who can toggle.
96+
- No broad historical migration job; old canvases are defaulted lazily on open.
97+
- Existing global sync gate (`TLDRAW_CLOUDFLARE_SYNC_ENABLED` + URL) remains in place.
98+
- Auto-import/manual import from local data is explicitly deferred to a later version.

0 commit comments

Comments
 (0)