From 35bbbb9599b60fb8f72bb2d3563a92447ac1049d Mon Sep 17 00:00:00 2001 From: Zimeng Xiong Date: Sat, 7 Feb 2026 17:21:58 -0800 Subject: [PATCH] images in preview --- backend/src/__tests__/drawings.integration.ts | 84 +++++++++++ backend/src/auth/coreRoutes.ts | 30 ++-- backend/src/index.ts | 4 +- backend/src/middleware/auth.ts | 19 +-- backend/src/security.ts | 64 +++++++- frontend/eslint.config.js | 50 ++++++ frontend/src/api/index.ts | 93 ++++++++++-- frontend/src/components/DrawingCard.tsx | 34 ++++- frontend/src/context/AuthContext.test.tsx | 39 +++++ frontend/src/context/AuthContext.tsx | 15 +- frontend/src/pages/Dashboard.tsx | 2 +- frontend/src/pages/Editor.tsx | 142 ++++++++++++++---- .../src/utils/__tests__/previewSvg.test.ts | 45 ++++++ frontend/src/utils/importUtils.ts | 2 +- frontend/src/utils/previewSvg.ts | 108 +++++++++++++ 15 files changed, 654 insertions(+), 77 deletions(-) create mode 100644 frontend/eslint.config.js create mode 100644 frontend/src/utils/__tests__/previewSvg.test.ts create mode 100644 frontend/src/utils/previewSvg.ts diff --git a/backend/src/__tests__/drawings.integration.ts b/backend/src/__tests__/drawings.integration.ts index 0667eaa..c15ffbc 100644 --- a/backend/src/__tests__/drawings.integration.ts +++ b/backend/src/__tests__/drawings.integration.ts @@ -267,6 +267,90 @@ describe("Security Sanitization - Image Data URLs", () => { }); }); + describe("sanitizeDrawingData - preview svg handling", () => { + it("should preserve safe SVG layout attributes needed for thumbnail rendering", () => { + const preview = [ + '', + '', + '', + "", + ].join(""); + + const result = sanitizeDrawingData({ + elements: [], + appState: { viewBackgroundColor: "#ffffff" }, + files: {}, + preview, + }); + + expect(result.preview).toContain('viewBox="0 0 728.39453125 606.908203125"'); + expect(result.preview).toContain('preserveAspectRatio="xMidYMid meet"'); + expect(result.preview).toContain('stroke-linecap="round"'); + expect(result.preview).toContain('xmlns="http://www.w3.org/2000/svg"'); + }); + + it("should preserve safe embedded image previews", () => { + const preview = [ + '', + '', + "", + ].join(""); + + const result = sanitizeDrawingData({ + elements: [], + appState: { viewBackgroundColor: "#ffffff" }, + files: {}, + preview, + }); + + expect(result.preview).toContain(" { + const preview = [ + '', + '', + '', + "", + ].join(""); + + const result = sanitizeDrawingData({ + elements: [], + appState: { viewBackgroundColor: "#ffffff" }, + files: {}, + preview, + }); + + expect(result.preview).not.toContain(" { + const preview = [ + '', + '', + '', + "", + '', + "", + ].join(""); + + const result = sanitizeDrawingData({ + elements: [], + appState: { viewBackgroundColor: "#ffffff" }, + files: {}, + preview, + }); + + expect(result.preview).toContain(""); + expect(result.preview).toContain(" { it("should validate drawing with embedded images", () => { const files = createSampleFilesObject(2, "large"); diff --git a/backend/src/auth/coreRoutes.ts b/backend/src/auth/coreRoutes.ts index 9d1b4a7..1b8eedf 100644 --- a/backend/src/auth/coreRoutes.ts +++ b/backend/src/auth/coreRoutes.ts @@ -267,10 +267,15 @@ export const registerCoreRoutes = (deps: RegisterCoreRoutesDeps) => { expiresAt, }, }); - } catch { - if (process.env.NODE_ENV === "development") { - console.debug("Refresh token storage skipped (feature disabled or table missing)"); + } catch (error) { + if (isMissingRefreshTokenTableError(error)) { + console.error("Refresh token rotation is enabled but refresh token storage is unavailable"); + return res.status(503).json({ + error: "Service unavailable", + message: "Refresh token storage is unavailable. Please run database migrations.", + }); } + throw error; } } @@ -380,10 +385,15 @@ export const registerCoreRoutes = (deps: RegisterCoreRoutesDeps) => { expiresAt, }, }); - } catch { - if (process.env.NODE_ENV === "development") { - console.debug("Refresh token rotation skipped (feature disabled or table missing)"); + } catch (error) { + if (isMissingRefreshTokenTableError(error)) { + console.error("Refresh token rotation is enabled but refresh token storage is unavailable"); + return res.status(503).json({ + error: "Service unavailable", + message: "Refresh token storage is unavailable. Please run database migrations.", + }); } + throw error; } } @@ -514,9 +524,11 @@ export const registerCoreRoutes = (deps: RegisterCoreRoutesDeps) => { } if (isMissingRefreshTokenTableError(error)) { - if (process.env.NODE_ENV === "development") { - console.debug("Refresh token rotation skipped (feature disabled or table missing)"); - } + console.error("Refresh token rotation is enabled but refresh token storage is unavailable"); + return res.status(503).json({ + error: "Service unavailable", + message: "Refresh token storage is unavailable. Please run database migrations.", + }); } else { console.error("Refresh token rotation error:", error); return res.status(500).json({ diff --git a/backend/src/index.ts b/backend/src/index.ts index 50690ae..1279ba9 100644 --- a/backend/src/index.ts +++ b/backend/src/index.ts @@ -707,8 +707,8 @@ export const sanitizeDrawingUpdateData = ( collectionId: data.collectionId, }; const sanitized = sanitizeDrawingData(fullData); - sanitizedData.elements = sanitized.elements; - sanitizedData.appState = sanitized.appState; + if (data.elements !== undefined) sanitizedData.elements = sanitized.elements; + if (data.appState !== undefined) sanitizedData.appState = sanitized.appState; if (data.files !== undefined) sanitizedData.files = sanitized.files; if (data.preview !== undefined) sanitizedData.preview = sanitized.preview; Object.assign(data, sanitizedData); diff --git a/backend/src/middleware/auth.ts b/backend/src/middleware/auth.ts index f0a9227..a51d82b 100644 --- a/backend/src/middleware/auth.ts +++ b/backend/src/middleware/auth.ts @@ -53,23 +53,10 @@ const getAuthEnabled = async (): Promise => { }; const getBootstrapActingUser = async () => { - const user = await prisma.user.findUnique({ + return prisma.user.upsert({ where: { id: BOOTSTRAP_USER_ID }, - select: { - id: true, - username: true, - email: true, - name: true, - role: true, - mustResetPassword: true, - isActive: true, - }, - }); - - if (user) return user; - - return prisma.user.create({ - data: { + update: {}, + create: { id: BOOTSTRAP_USER_ID, email: "bootstrap@excalidash.local", username: null, diff --git a/backend/src/security.ts b/backend/src/security.ts index ec1e9d7..9dbfe3d 100644 --- a/backend/src/security.ts +++ b/backend/src/security.ts @@ -101,11 +101,45 @@ export const sanitizeHtml = (input: string): string => { export const sanitizeSvg = (svgContent: string): string => { if (typeof svgContent !== "string") return ""; - return purify + const safeImageDataUrlPattern = + /^data:image\/(?:png|jpe?g|gif|webp|avif|bmp);base64,[a-z0-9+/=\s]+$/i; + + const sanitizeSvgImageTags = (content: string): string => + content.replace(/]*>/gi, (imageTag) => { + const hrefMatch = + imageTag.match(/\shref\s*=\s*"([^"]*)"/i) ?? + imageTag.match(/\shref\s*=\s*'([^']*)'/i) ?? + imageTag.match(/\sxlink:href\s*=\s*"([^"]*)"/i) ?? + imageTag.match(/\sxlink:href\s*=\s*'([^']*)'/i); + + const hrefValue = hrefMatch?.[1]?.trim(); + if (!hrefValue || !safeImageDataUrlPattern.test(hrefValue)) { + return ""; + } + + const withoutXlinkHref = imageTag.replace( + /\sxlink:href\s*=\s*(?:"[^"]*"|'[^']*')/gi, + "" + ); + + if (/\shref\s*=/i.test(withoutXlinkHref)) { + return withoutXlinkHref.replace( + /\shref\s*=\s*(?:"[^"]*"|'[^']*')/i, + ` href="${hrefValue}"` + ); + } + + return withoutXlinkHref.replace(/ { "tspan", ], ALLOWED_ATTR: [ + "xmlns", + "xmlns:xlink", + "version", + "id", + "viewBox", + "preserveAspectRatio", "x", "y", "width", @@ -133,14 +173,29 @@ export const sanitizeSvg = (svgContent: string): string => { "points", "d", "fill", + "fill-opacity", + "fill-rule", "stroke", "stroke-width", + "stroke-opacity", + "stroke-linecap", + "stroke-linejoin", + "stroke-miterlimit", + "stroke-dasharray", + "stroke-dashoffset", "opacity", "transform", + "vector-effect", + "patternUnits", + "patternContentUnits", "font-size", "font-family", + "font-weight", + "letter-spacing", "text-anchor", "dominant-baseline", + "href", + "xlink:href", ], FORBID_TAGS: [ "script", @@ -149,10 +204,8 @@ export const sanitizeSvg = (svgContent: string): string => { "object", "embed", "use", - "image", "style", "link", - "defs", "symbol", "marker", "clipPath", @@ -166,17 +219,16 @@ export const sanitizeSvg = (svgContent: string): string => { "onmouseover", "onfocus", "onblur", - "href", - "xlink:href", "src", "action", "style", "class", - "id", ], KEEP_CONTENT: true, }) .trim(); + + return sanitizeSvgImageTags(sanitized).trim(); }; export const sanitizeText = ( diff --git a/frontend/eslint.config.js b/frontend/eslint.config.js new file mode 100644 index 0000000..c2c647c --- /dev/null +++ b/frontend/eslint.config.js @@ -0,0 +1,50 @@ +import js from "@eslint/js"; +import globals from "globals"; +import reactHooks from "eslint-plugin-react-hooks"; +import reactRefresh from "eslint-plugin-react-refresh"; +import tseslint from "typescript-eslint"; + +export default tseslint.config( + { + ignores: [ + "dist", + "coverage", + "playwright-report", + "test-results", + "node_modules", + ], + }, + { + extends: [js.configs.recommended, ...tseslint.configs.recommended], + files: ["**/*.{ts,tsx}"], + languageOptions: { + ecmaVersion: 2020, + globals: { + ...globals.browser, + ...globals.node, + }, + }, + plugins: { + "react-hooks": reactHooks, + "react-refresh": reactRefresh, + }, + rules: { + ...reactHooks.configs.recommended.rules, + "react-hooks/set-state-in-effect": "off", + "react-refresh/only-export-components": ["warn", { allowConstantExport: true }], + "@typescript-eslint/no-explicit-any": "off", + "@typescript-eslint/no-unused-vars": [ + "warn", + { argsIgnorePattern: "^_", varsIgnorePattern: "^_" }, + ], + }, + }, + { + files: ["**/*.{test,spec}.{ts,tsx}"], + languageOptions: { + globals: { + ...globals.vitest, + }, + }, + } +); diff --git a/frontend/src/api/index.ts b/frontend/src/api/index.ts index 981275b..647dba8 100644 --- a/frontend/src/api/index.ts +++ b/frontend/src/api/index.ts @@ -1,10 +1,12 @@ import axios from "axios"; import type { Drawing, Collection, DrawingSummary } from "../types"; +import { normalizePreviewSvg } from "../utils/previewSvg"; export const API_URL = import.meta.env.VITE_API_URL || "/api"; export const api = axios.create({ baseURL: API_URL, + withCredentials: true, }); // Re-export axios for type checking @@ -18,14 +20,19 @@ export { api as default }; const TOKEN_KEY = 'excalidash-access-token'; const REFRESH_TOKEN_KEY = 'excalidash-refresh-token'; const USER_KEY = 'excalidash-user'; +const AUTH_ENABLED_CACHE_KEY = "excalidash-auth-enabled"; +const AUTH_STATUS_TTL_MS = 5000; type RetriableRequestConfig = { _retry?: boolean; _csrfRetry?: boolean; + _authModeRetry?: boolean; url?: string; headers?: Record; }; +let authEnabledProbeCache: { value: boolean; fetchedAt: number } | null = null; + const getAuthToken = (): string | null => { if (typeof window === 'undefined') return null; return localStorage.getItem(TOKEN_KEY); @@ -39,7 +46,8 @@ let csrfTokenPromise: Promise | null = null; export const fetchCsrfToken = async (): Promise => { try { const response = await axios.get<{ token: string; header: string }>( - `${API_URL}/csrf-token` + `${API_URL}/csrf-token`, + { withCredentials: true } ); csrfToken = response.data.token; csrfHeaderName = response.data.header || "x-csrf-token"; @@ -71,7 +79,47 @@ const clearStoredAuth = () => { localStorage.removeItem(USER_KEY); }; -const redirectToLogin = () => { +const readCachedAuthEnabled = (): boolean | null => { + if (typeof window === "undefined") return null; + const raw = localStorage.getItem(AUTH_ENABLED_CACHE_KEY); + if (raw === "true") return true; + if (raw === "false") return false; + return null; +}; + +const cacheAuthEnabled = (enabled: boolean) => { + if (typeof window === "undefined") return; + authEnabledProbeCache = { value: enabled, fetchedAt: Date.now() }; + localStorage.setItem(AUTH_ENABLED_CACHE_KEY, String(enabled)); +}; + +const getAuthEnabledStatus = async (): Promise => { + const now = Date.now(); + if (authEnabledProbeCache && now - authEnabledProbeCache.fetchedAt < AUTH_STATUS_TTL_MS) { + return authEnabledProbeCache.value; + } + + try { + const response = await axios.get<{ authEnabled?: boolean; enabled?: boolean }>( + `${API_URL}/auth/status`, + { withCredentials: true } + ); + const enabled = + typeof response.data?.authEnabled === "boolean" + ? response.data.authEnabled + : typeof response.data?.enabled === "boolean" + ? response.data.enabled + : true; + cacheAuthEnabled(enabled); + return enabled; + } catch { + return readCachedAuthEnabled(); + } +}; + +const redirectToLogin = async () => { + const authEnabled = await getAuthEnabledStatus(); + if (authEnabled === false) return; if (window.location.pathname !== '/login') { window.location.href = '/login'; } @@ -87,9 +135,13 @@ const refreshAccessToken = async (): Promise => { throw new Error("Missing refresh token"); } - const refreshResponse = await axios.post(`${API_URL}/auth/refresh`, { - refreshToken, - }); + const refreshResponse = await axios.post( + `${API_URL}/auth/refresh`, + { + refreshToken, + }, + { withCredentials: true } + ); const nextAccessToken = String(refreshResponse.data.accessToken || ""); if (!nextAccessToken) { @@ -173,6 +225,15 @@ api.interceptors.response.use( const url = String(originalRequest.url || ""); const isAuthRoute = url.includes('/auth/'); const hasRefreshToken = Boolean(localStorage.getItem(REFRESH_TOKEN_KEY)); + const authEnabled = !isAuthRoute ? await getAuthEnabledStatus() : true; + + if (!isAuthRoute && authEnabled === false) { + if (!originalRequest._authModeRetry) { + originalRequest._authModeRetry = true; + return api(originalRequest as any); + } + return Promise.reject(error); + } if (!isAuthRoute && hasRefreshToken && !originalRequest._retry) { try { @@ -183,14 +244,14 @@ api.interceptors.response.use( return api(originalRequest as any); } catch { clearStoredAuth(); - redirectToLogin(); + await redirectToLogin(); return Promise.reject(error); } } if (!isAuthRoute) { clearStoredAuth(); - redirectToLogin(); + await redirectToLogin(); } } @@ -243,14 +304,28 @@ const deserializeDrawingSummary = (drawing: unknown): DrawingSummary => { if (typeof drawing !== 'object' || drawing === null) { throw new Error('Invalid drawing data'); } - return deserializeTimestamps(drawing as HasTimestamps & DrawingSummary); + const parsed = drawing as HasTimestamps & DrawingSummary; + return deserializeTimestamps({ + ...parsed, + preview: + typeof parsed.preview === "string" + ? normalizePreviewSvg(parsed.preview) + : parsed.preview, + }); }; const deserializeDrawing = (drawing: unknown): Drawing => { if (typeof drawing !== 'object' || drawing === null) { throw new Error('Invalid drawing data'); } - return deserializeTimestamps(drawing as HasTimestamps & Drawing); + const parsed = drawing as HasTimestamps & Drawing; + return deserializeTimestamps({ + ...parsed, + preview: + typeof parsed.preview === "string" + ? normalizePreviewSvg(parsed.preview) + : parsed.preview, + }); }; export interface PaginatedDrawings { diff --git a/frontend/src/components/DrawingCard.tsx b/frontend/src/components/DrawingCard.tsx index 6ed873e..fe77ad9 100644 --- a/frontend/src/components/DrawingCard.tsx +++ b/frontend/src/components/DrawingCard.tsx @@ -7,6 +7,7 @@ import { formatDistanceToNow } from 'date-fns'; import clsx from 'clsx'; // import { exportToSvg } from "@excalidraw/excalidraw"; // Lazy load this instead import { exportDrawingToFile } from '../utils/exportUtils'; +import { previewHasEmbeddedImages } from '../utils/previewSvg'; import * as api from '../api'; @@ -16,6 +17,31 @@ type HydratedDrawingData = { files: Record; }; +const normalizeImageElementsForPreview = ( + elements: any[] = [], + files: Record = {} +): any[] => + elements.map((element) => { + if (!element || element.type !== "image" || typeof element.fileId !== "string") { + return element; + } + + const file = files[element.fileId]; + const hasImageData = + typeof file?.dataURL === "string" && + file.dataURL.startsWith("data:image/") && + file.dataURL.length > 0; + + if (!hasImageData || element.status === "saved") { + return element; + } + + return { + ...element, + status: "saved", + }; + }); + interface DrawingCardProps { drawing: DrawingSummary; collections: Collection[]; @@ -60,6 +86,7 @@ export const DrawingCard: React.FC = ({ const [isExporting, setIsExporting] = useState(false); const [exportError, setExportError] = useState(null); const [fullData, setFullData] = useState(null); + const hasEmbeddedImages = previewHasEmbeddedImages(previewSvg); const fullDataRef = React.useRef(fullData); fullDataRef.current = fullData; @@ -118,7 +145,7 @@ export const DrawingCard: React.FC = ({ if (cancelled) return; const svg = await exportToSvg({ - elements: data.elements, + elements: normalizeImageElementsForPreview(data.elements, data.files || {}), appState: { ...data.appState, exportBackground: true, @@ -248,7 +275,10 @@ export const DrawingCard: React.FC = ({ {previewSvg ? (
svg]:w-auto [&>svg]:h-auto [&>svg]:max-w-full [&>svg]:max-h-full [&>svg]:drop-shadow-sm transition-transform duration-500", + !hasEmbeddedImages && "dark:[&>svg]:invert dark:[&>svg_rect[fill='white']]:opacity-0 dark:[&>svg_rect[fill='#ffffff']]:opacity-0" + )} dangerouslySetInnerHTML={{ __html: previewSvg }} /> ) : ( diff --git a/frontend/src/context/AuthContext.test.tsx b/frontend/src/context/AuthContext.test.tsx index 6599069..df8082d 100644 --- a/frontend/src/context/AuthContext.test.tsx +++ b/frontend/src/context/AuthContext.test.tsx @@ -87,4 +87,43 @@ describe("AuthProvider", () => { expect(storage.get("excalidash-refresh-token")).toBeUndefined(); expect(storage.get("excalidash-user")).toBeUndefined(); }); + + it("uses cached auth-disabled mode when /auth/status is temporarily unavailable", async () => { + const storage = new Map([ + ["excalidash-auth-enabled", "false"], + ["excalidash-access-token", "token"], + ["excalidash-refresh-token", "refresh"], + ["excalidash-user", JSON.stringify({ id: "u1" })], + ]); + Object.defineProperty(window, "localStorage", { + configurable: true, + value: { + getItem: (key: string) => storage.get(key) ?? null, + setItem: (key: string, value: string) => { + storage.set(key, value); + }, + removeItem: (key: string) => { + storage.delete(key); + }, + }, + }); + + vi.spyOn(axios, "get").mockRejectedValueOnce(new Error("network down")); + + render( + + + + + + ); + + await waitFor(() => { + expect(screen.getByTestId("loading").textContent).toBe("false"); + }); + expect(screen.getByTestId("auth-enabled").textContent).toBe("false"); + expect(storage.get("excalidash-access-token")).toBeUndefined(); + expect(storage.get("excalidash-refresh-token")).toBeUndefined(); + expect(storage.get("excalidash-user")).toBeUndefined(); + }); }); diff --git a/frontend/src/context/AuthContext.tsx b/frontend/src/context/AuthContext.tsx index 8848ee2..9602002 100644 --- a/frontend/src/context/AuthContext.tsx +++ b/frontend/src/context/AuthContext.tsx @@ -30,6 +30,7 @@ const AuthContext = createContext(undefined); const TOKEN_KEY = 'excalidash-access-token'; const REFRESH_TOKEN_KEY = 'excalidash-refresh-token'; const USER_KEY = 'excalidash-user'; +const AUTH_ENABLED_CACHE_KEY = "excalidash-auth-enabled"; export const AuthProvider: React.FC<{ children: ReactNode }> = ({ children }) => { const [user, setUser] = useState(null); @@ -52,6 +53,7 @@ export const AuthProvider: React.FC<{ children: ReactNode }> = ({ children }) => ? statusResponse.data.enabled : true; setAuthEnabled(enabled); + localStorage.setItem(AUTH_ENABLED_CACHE_KEY, String(enabled)); setBootstrapRequired(Boolean(statusResponse.data?.bootstrapRequired)); // In single-user mode, do not require login. @@ -63,8 +65,17 @@ export const AuthProvider: React.FC<{ children: ReactNode }> = ({ children }) => return; } } catch { - // If status fails, default to auth-enabled mode to avoid exposing - // single-user UI paths accidentally. Backend remains the source of truth. + const cachedAuthEnabled = localStorage.getItem(AUTH_ENABLED_CACHE_KEY); + if (cachedAuthEnabled === "false") { + setAuthEnabled(false); + setBootstrapRequired(false); + localStorage.removeItem(TOKEN_KEY); + localStorage.removeItem(REFRESH_TOKEN_KEY); + localStorage.removeItem(USER_KEY); + setUser(null); + return; + } + // If status fails and no cached mode exists, default to auth-enabled mode. setAuthEnabled(true); setBootstrapRequired(false); } diff --git a/frontend/src/pages/Dashboard.tsx b/frontend/src/pages/Dashboard.tsx index 8a35e38..74ad12e 100644 --- a/frontend/src/pages/Dashboard.tsx +++ b/frontend/src/pages/Dashboard.tsx @@ -719,7 +719,7 @@ export const Dashboard: React.FC = () => { {d.preview ? (
) : ( diff --git a/frontend/src/pages/Editor.tsx b/frontend/src/pages/Editor.tsx index 19b51de..4b388ca 100644 --- a/frontend/src/pages/Editor.tsx +++ b/frontend/src/pages/Editor.tsx @@ -30,6 +30,13 @@ interface Peer extends UserIdentity { isActive: boolean; } +class DrawingSaveConflictError extends Error { + constructor(message = "Drawing version conflict") { + super(message); + this.name = "DrawingSaveConflictError"; + } +} + export const Editor: React.FC = () => { const { id } = useParams<{ id: string }>(); const navigate = useNavigate(); @@ -174,6 +181,39 @@ export const Editor: React.FC = () => { [getRenderableBaselineSnapshot] ); + const normalizeImageElementStatus = useCallback( + (elements: readonly any[] = [], files?: Record | null): readonly any[] => { + if (!Array.isArray(elements) || elements.length === 0) return elements; + const fileMap = files || {}; + let changed = false; + + const normalized = elements.map((element: any) => { + if (!element || element.type !== "image" || typeof element.fileId !== "string") { + return element; + } + + const file = fileMap[element.fileId]; + const hasImageData = + typeof file?.dataURL === "string" && + file.dataURL.startsWith("data:image/") && + file.dataURL.length > 0; + + if (!hasImageData || element.status === "saved") { + return element; + } + + changed = true; + return { + ...element, + status: "saved", + }; + }); + + return changed ? normalized : elements; + }, + [] + ); + const emitFilesDeltaIfNeeded = useCallback( (nextFiles: Record) => { if (!socketRef.current || !id) return false; @@ -519,18 +559,23 @@ export const Editor: React.FC = () => { return; } const persistableFiles = files ?? latestFilesRef.current ?? {}; + const normalizedElements = normalizeImageElementStatus( + persistableElements, + persistableFiles + ); + const normalizedElementsForSave = Array.from(normalizedElements); console.log("[Editor] Saving drawing", { drawingId, - elementCount: persistableElements.length, - hasRenderableElements: hasRenderableElements(persistableElements), + elementCount: normalizedElementsForSave.length, + hasRenderableElements: hasRenderableElements(normalizedElementsForSave), appState: persistableAppState, }); const persistScene = async (attempt: number): Promise => { try { const updated = await api.updateDrawing(drawingId, { - elements: persistableElements, + elements: normalizedElementsForSave, appState: persistableAppState, files: persistableFiles, version: currentDrawingVersionRef.current ?? undefined, @@ -538,7 +583,7 @@ export const Editor: React.FC = () => { if (typeof updated.version === "number") { currentDrawingVersionRef.current = updated.version; } - lastPersistedElementsRef.current = persistableElements; + lastPersistedElementsRef.current = normalizedElementsForSave; console.log("[Editor] Save complete", { drawingId }); } catch (err) { if (api.isAxiosError(err) && err.response?.status === 409) { @@ -557,9 +602,7 @@ export const Editor: React.FC = () => { return; } - console.warn("[Editor] Version conflict while saving drawing", { drawingId }); - toast.error("Drawing changed in another tab. Refresh to load latest."); - return; + throw new DrawingSaveConflictError(); } throw err; @@ -568,17 +611,38 @@ export const Editor: React.FC = () => { await persistScene(0); } catch (err) { + if (err instanceof DrawingSaveConflictError) { + console.warn("[Editor] Version conflict while saving drawing", { drawingId }); + toast.error("Drawing changed in another tab. Refresh to load latest."); + throw err; + } console.error('Failed to save drawing', err); toast.error("Failed to save changes"); + throw err; } }; const enqueueSceneSave = useCallback( - (drawingId: string, elements: readonly any[], appState: any, files?: Record) => { + ( + drawingId: string, + elements: readonly any[], + appState: any, + files?: Record, + options?: { suppressErrors?: boolean } + ) => { + const suppressErrors = options?.suppressErrors ?? true; saveQueueRef.current = saveQueueRef.current .catch(() => undefined) .then(async () => { if (!saveDataRef.current) return; + if (suppressErrors) { + try { + await saveDataRef.current(drawingId, elements, appState, files); + } catch { + // Background autosaves already surface their own toast via saveDataRef. + } + return; + } await saveDataRef.current(drawingId, elements, appState, files); }); return saveQueueRef.current; @@ -590,7 +654,12 @@ export const Editor: React.FC = () => { if (!drawingId) return; try { - const candidateSnapshot = latestElementsRef.current ?? elements; + const snapshotFromArgs = Array.isArray(elements) ? elements : []; + const snapshotFromRef = latestElementsRef.current ?? []; + const candidateSnapshot = + hasRenderableElements(snapshotFromArgs) || !hasRenderableElements(snapshotFromRef) + ? snapshotFromArgs + : snapshotFromRef; const { snapshot: currentSnapshot, prevented: preventedPreviewOverwrite, @@ -598,6 +667,7 @@ export const Editor: React.FC = () => { staleNonRenderableSnapshot: staleNonRenderablePreview, } = resolveSafeSnapshot(candidateSnapshot); const currentFiles = latestFilesRef.current ?? files; + const normalizedSnapshot = normalizeImageElementStatus(currentSnapshot, currentFiles); if (suspiciousBlankLoadRef.current && !hasRenderableElements(currentSnapshot)) { console.warn("[Editor] Blocking non-renderable preview due to suspicious blank load", { drawingId, @@ -616,7 +686,7 @@ export const Editor: React.FC = () => { } const svg = await exportToSvg({ - elements: currentSnapshot, + elements: normalizedSnapshot, appState: { ...appState, exportBackground: true, @@ -628,7 +698,7 @@ export const Editor: React.FC = () => { console.log("[Editor] Saving preview", { drawingId, - elementCount: currentSnapshot.length, + elementCount: normalizedSnapshot.length, }); await api.updateDrawing(drawingId, { preview }); @@ -1013,6 +1083,14 @@ export const Editor: React.FC = () => { if (didEmit && latestAppStateRef.current && debouncedSaveRef.current) { hasSceneChangesSinceLoadRef.current = true; debouncedSaveRef.current(id, latestElementsRef.current, latestAppStateRef.current, nextFiles); + if (savePreviewRef.current) { + void savePreviewRef.current( + id, + latestElementsRef.current, + latestAppStateRef.current, + nextFiles + ); + } } }, 1000); @@ -1044,18 +1122,21 @@ export const Editor: React.FC = () => { if (isSavingOnLeave) return; // Prevent double clicks setIsSavingOnLeave(true); + let shouldNavigate = false; // Save drawing and generate preview before navigating try { - if (excalidrawAPI.current && saveDataRef.current && savePreviewRef.current) { - if (!hasSceneChangesSinceLoadRef.current) { - console.log("[Editor] Skipping back-navigation save: no scene changes since load", { - drawingId: id, - }); - navigate('/'); - return; - } - if (!id) return; + if (!(excalidrawAPI.current && saveDataRef.current && savePreviewRef.current)) { + // If editor API is not ready, allow navigation instead of trapping the user. + shouldNavigate = true; + } else if (!hasSceneChangesSinceLoadRef.current) { + console.log("[Editor] Skipping back-navigation save: no scene changes since load", { + drawingId: id, + }); + shouldNavigate = true; + } else if (!id) { + shouldNavigate = true; + } else { const elements = excalidrawAPI.current.getSceneElementsIncludingDeleted(); const { snapshot: safeElements, @@ -1081,22 +1162,25 @@ export const Editor: React.FC = () => { elementCount: safeElements.length, }); toast.warning("Blank scene detected on load. Skipping save to protect existing data."); - navigate('/'); - return; + shouldNavigate = true; + } else { + await Promise.all([ + enqueueSceneSave(id, safeElements, appState, files, { suppressErrors: false }), + savePreviewRef.current(id, safeElements, appState, files) + ]); + console.log("[Editor] Saved on back navigation", { drawingId: id }); + shouldNavigate = true; } - - await Promise.all([ - enqueueSceneSave(id, safeElements, appState, files), - savePreviewRef.current(id, safeElements, appState, files) - ]); - console.log("[Editor] Saved on back navigation", { drawingId: id }); } } catch (err) { console.error('Failed to save on back navigation', err); + toast.error("Failed to save changes. Please retry before leaving."); } finally { setIsSavingOnLeave(false); } - navigate('/'); + if (shouldNavigate) { + navigate('/'); + } }; return ( diff --git a/frontend/src/utils/__tests__/previewSvg.test.ts b/frontend/src/utils/__tests__/previewSvg.test.ts new file mode 100644 index 0000000..a18168c --- /dev/null +++ b/frontend/src/utils/__tests__/previewSvg.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from "vitest"; +import { normalizePreviewSvg, previewHasEmbeddedImages } from "../previewSvg"; + +describe("normalizePreviewSvg", () => { + it("adds viewBox from background rect when missing", () => { + const raw = [ + '', + '', + '', + "", + ].join(""); + + const normalized = normalizePreviewSvg(raw); + + expect(normalized).toContain('viewBox="0 0 728.39453125 606.908203125"'); + expect(normalized).toContain('preserveAspectRatio="xMidYMid meet"'); + }); + + it("leaves existing viewBox unchanged", () => { + const raw = ''; + const normalized = normalizePreviewSvg(raw); + + expect(normalized).toContain('viewBox="0 0 100 50"'); + }); + + it("detects embedded image tags", () => { + const raw = ''; + expect(previewHasEmbeddedImages(raw)).toBe(true); + expect(previewHasEmbeddedImages("")).toBe(false); + }); + + it("repairs flattened image previews that are hidden by white canvas rect", () => { + const raw = [ + '', + '', + '', + '', + "", + ].join(""); + + const normalized = normalizePreviewSvg(raw); + + expect(normalized).toContain('fill="transparent"'); + }); +}); diff --git a/frontend/src/utils/importUtils.ts b/frontend/src/utils/importUtils.ts index a274f44..1cc195d 100644 --- a/frontend/src/utils/importUtils.ts +++ b/frontend/src/utils/importUtils.ts @@ -231,7 +231,7 @@ export const importLegacyFiles = async ( // Import each drawing entry for (let i = 0; i < drawings.length; i += 1) { const d = drawings[i] as LegacyExportDrawing; - const elements = Array.isArray(d.elements) ? d.elements : null; + const elements = Array.isArray(d.elements) ? (d.elements as any[]) : null; const appState = typeof d.appState === "object" && d.appState !== null ? (d.appState as Record) diff --git a/frontend/src/utils/previewSvg.ts b/frontend/src/utils/previewSvg.ts new file mode 100644 index 0000000..1a76c67 --- /dev/null +++ b/frontend/src/utils/previewSvg.ts @@ -0,0 +1,108 @@ +const parseDimension = (value: string | null): number | null => { + if (!value) return null; + const parsed = Number.parseFloat(value); + return Number.isFinite(parsed) && parsed > 0 ? parsed : null; +}; + +const parseCoordinate = (value: string | null): number | null => { + if (!value) return null; + const parsed = Number.parseFloat(value); + return Number.isFinite(parsed) ? parsed : null; +}; + +const parseViewBox = (value: string | null): { width: number; height: number } | null => { + if (!value) return null; + const parts = value.trim().split(/[,\s]+/).map((part) => Number.parseFloat(part)); + if (parts.length !== 4 || parts.some((part) => !Number.isFinite(part))) return null; + const [, , width, height] = parts; + if (width <= 0 || height <= 0) return null; + return { width, height }; +}; + +const isNear = (a: number, b: number, epsilon = 0.5): boolean => Math.abs(a - b) <= epsilon; + +const maybeRepairFlattenedImagePreview = (svg: SVGSVGElement) => { + const rootImage = Array.from(svg.children).find( + (child) => + child.tagName.toLowerCase() === "image" && + /^(100%|1(?:\.0+)?%?)$/i.test(child.getAttribute("width") ?? "") && + /^(100%|1(?:\.0+)?%?)$/i.test(child.getAttribute("height") ?? "") && + /^data:image\//i.test(child.getAttribute("href") ?? child.getAttribute("xlink:href") ?? "") + ); + if (!rootImage) return; + + const hasPattern = svg.querySelector("pattern") !== null; + const hasUrlFill = Array.from(svg.querySelectorAll("[fill]")).some((node) => + /^url\(#/i.test(node.getAttribute("fill") ?? "") + ); + if (hasPattern || hasUrlFill) return; + + const viewBox = parseViewBox(svg.getAttribute("viewBox")); + const fallbackWidth = parseDimension(svg.getAttribute("width")); + const fallbackHeight = parseDimension(svg.getAttribute("height")); + const canvasWidth = viewBox?.width ?? fallbackWidth; + const canvasHeight = viewBox?.height ?? fallbackHeight; + if (!canvasWidth || !canvasHeight) return; + + const candidateRect = Array.from(svg.children).find((child) => { + if (child.tagName.toLowerCase() !== "rect") return false; + const fill = (child.getAttribute("fill") || "").trim().toLowerCase(); + if (fill !== "#fff" && fill !== "#ffffff" && fill !== "white") return false; + const x = parseCoordinate(child.getAttribute("x")); + const y = parseCoordinate(child.getAttribute("y")); + const width = parseDimension(child.getAttribute("width")); + const height = parseDimension(child.getAttribute("height")); + if (x !== 0 || y !== 0 || !width || !height) return false; + return isNear(width, canvasWidth) && isNear(height, canvasHeight); + }); + + if (candidateRect) { + candidateRect.setAttribute("fill", "transparent"); + } +}; + +export const previewHasEmbeddedImages = ( + preview: string | null | undefined +): boolean => typeof preview === "string" && /]/i.test(preview); + +export const normalizePreviewSvg = (preview: string | null | undefined): string | null => { + if (typeof preview !== "string" || preview.trim().length === 0) { + return preview ?? null; + } + + if (typeof DOMParser === "undefined") { + return preview; + } + + try { + const doc = new DOMParser().parseFromString(preview, "image/svg+xml"); + const svg = doc.documentElement; + if (!svg || svg.tagName.toLowerCase() !== "svg") { + return preview; + } + + if (!svg.hasAttribute("viewBox")) { + const backgroundRect = svg.querySelector("rect[x='0'][y='0']"); + const width = + parseDimension(backgroundRect?.getAttribute("width") ?? null) ?? + parseDimension(svg.getAttribute("width")); + const height = + parseDimension(backgroundRect?.getAttribute("height") ?? null) ?? + parseDimension(svg.getAttribute("height")); + + if (width && height) { + svg.setAttribute("viewBox", `0 0 ${width} ${height}`); + } + } + + if (!svg.hasAttribute("preserveAspectRatio")) { + svg.setAttribute("preserveAspectRatio", "xMidYMid meet"); + } + + maybeRepairFlattenedImagePreview(svg as unknown as SVGSVGElement); + + return svg.outerHTML; + } catch { + return preview; + } +};