Skip to content

fix: make installing libraries from the library browser work - #1436

Open
scottscott wants to merge 2 commits into
nextcloud:mainfrom
scottscott:fix/library-add-from-browser
Open

scottscott wants to merge 2 commits into
nextcloud:mainfrom
scottscott:fix/library-add-from-browser

Conversation

@scottscott

Copy link
Copy Markdown

Symptom

Library → "Browse libraries" → "Add to your Excalidraw" on libraries.excalidraw.com silently does nothing. The library is neither imported into the session nor persisted.

Root cause

libraries.excalidraw.com does not push libraries via postMessage; its "Add" button is a link back to the referrer URL with an addLibrary fragment ({referrer}#addLibrary={libraryUrl}, or ?addLibrary= without useHash). The receiving app is expected to handle that fragment — excalidraw.com does this via useHandleLibrary, and this app already calls it in App.tsx.

The problem is libraryReturnUrl: the editor runs inside the Files app viewer (no iframe, no dedicated URL), so window.location.href is the Files app URL. The "Add" link therefore loads the Files app with an addLibrary fragment in a new tab — a page that mounts no Excalidraw editor and never runs useHandleLibrary. Silent no-op, on both the legacy viewer (NC ≤ 35) and the new viewer element (NC 36+).

Fix

  • New standalone editor page GET /apps/whiteboard/standalone/{fileId} (PageController::standalone) that renders the existing directEditing template with a server-generated JWT and a standalonePage initial-state flag; main.ts gains a matching runtime that boots the editor into #whiteboard-app outside the Files app.
  • libraryReturnUrl now points at that page (absolute URL, still passed pre-encoded for the browse-button template). On return from the libraries site, the editor boots, useHandleLibrary imports the library (prompting for confirmation since the instance token differs), and the existing onLibraryChange → updateLibraryItems flow persists it server-side.
  • The CSP listener now also matches /apps/whiteboard/standalone so connect-src https://libraries.excalidraw.com applies there (the libraries origin was already whitelisted for whiteboard pages; the new path just wasn't matched by isWhiteboardPage()).
  • Public shares keep the old behavior: the standalone page requires a session-authenticated user, and the library APIs are user-scoped. Making the flow work on public shares would need token-based auth for the page and is left as follow-up.

Tested

  • npm run build, eslint src/App.tsx src/main.ts, php -l on changed files.
  • End-to-end on a real Nextcloud 35 instance with the app built from this branch:
    1. "Browse libraries" href now carries referrer=…/apps/whiteboard/standalone/{fileId}.
    2. The libraries site renders "Add" as standalone/{fileId}#addLibrary=…; clicking it loads the standalone editor.
    3. The confirmation prompt ("This will add 2 shape(s) to your library") appears, the items show up in the library panel, and they survive a full page reload (server-side persistence).

Part of #26 (libraries did not persist across reloads at all; this fix wires the import path into the server-backed library store).

Clicking "Add to your Excalidraw" on libraries.excalidraw.com silently
did nothing: the site navigates back to the referrer URL with an
"addLibrary" fragment, but the configured libraryReturnUrl was the
Files app URL (the editor runs inside the viewer), which mounts no
editor that handles the fragment via useHandleLibrary.

Add a standalone editor page (PageController::standalone) that boots
the editor outside the Files app and point libraryReturnUrl at it, so
the return navigation loads an editor, imports the library and
persists it through the existing onLibraryChange flow.

Public shares keep the previous behavior: the standalone page needs a
session-authenticated user.

Part of nextcloud#26
The initial library load replaces the library wholesale (merge: false)
and its effect re-fires when fileName/isLoading settle, wiping any
library imported via an addLibrary URL fragment (or otherwise added)
after the first load.

Handle addLibrary fragments only once the initial load has finished,
and make the load effect idempotent per file/editor instance so late
dependency changes cannot re-run the wholesale replace. File changes
still reset the load via the existing fileId-change effect.

Part of nextcloud#26
@scottscott

Copy link
Copy Markdown
Author

Update: second race fixed (the wipe)

While verifying the standalone page end-to-end I found a second race that caused libraries installed from the browser to appear and then vanish within a second. This was actually the original symptom in #26, and it's why the earlier fix looked right in the code but still wiped on the real deployment.

Root cause

The initial library load in App.tsx calls excalidrawAPI.updateLibrary({ merge: false }) — a wholesale replace of the library. The useEffect running this load had fileName and isLoading in its dependency array, which settle after the board data manager finishes. So the effect ran once at mount, and then ran again when those values resolved, repeating the wholesale replace and wiping anything that had been added (e.g. via an addLibrary URL fragment) in between.

On top of that, useHandleLibrary ran at mount in parallel with the first load, so an import that landed before/around the replace was also wiped.

Fix (second commit, ea942ae1)

  1. Gate the import handler: useHandleLibrary({ excalidrawAPI: isLibraryLoaded ? excalidrawAPI : null }) — addLibrary fragments are only processed after the initial library load has finished.
  2. Make the load effect idempotent per file/editor instance: guard if (isLoading || !excalidrawAPI || isLibraryLoaded) return and add isLibraryLoaded to the deps. File changes still reset the load through the existing fileId-change effect, which sets isLibraryLoaded(false).

Verification (deployed patch on a live Nextcloud 2.1.2 + this branch)

  • Reproduced the wipe 10x at 1s intervals before the fix (imported shapes visible at t=0, gone by t=1s).
  • After the fix: imported 2-shape library appears in the panel and stays stable across 10x1s samples.
  • Full page reload: items still present (server persistence confirmed, so the wipe was client-side only).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant