Add image drag and drop between editor blocks - #202
Open
Norkep wants to merge 7 commits into
Open
Conversation
📝 WalkthroughWalkthroughThe change adds workspace-aware routing, revision-checked persistence, draft recovery, window sessions, scoped settings, Notion-style editor and table features, image handling, sidebar controls, and Vitest coverage. ChangesWorkspace, persistence, and window lifecycle
Editor and table features
Sidebar, settings, and tooling
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/preview/PreviewApp.tsx (1)
103-133: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGuard the state writes in the load effect against stale resolution.
This effect awaits
readFileDirectandgetDraftCheckpoint, then calls setters. IffilePathchanges before both promises settle, the older run writes stale content, title, and revision. Add a cancellation flag.🛠️ Proposed fix
useEffect(() => { + let cancelled = false; filesService .readFileDirect(filePath) .then(async (result) => { const checkpoint = await draftCheckpointService .getDraftCheckpoint(filePath) .catch(() => null); + if (cancelled) return; const recovered = checkpoint && checkpoint.markdown !== result.content ? checkpoint.markdown : result.content; @@ .catch((error) => { + if (cancelled) return; console.error("Failed to load file:", error); toast.error(`Failed to load file: ${error}`); }); + return () => { + cancelled = true; + }; }, [filePath]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/preview/PreviewApp.tsx` around lines 103 - 133, Update the file-loading useEffect for PreviewApp so it tracks cancellation for each filePath run and checks that flag before applying recovered content, title, modified state, or revision updates and before related checkpoint/conflict notifications. Set the flag in the effect cleanup so stale readFileDirect or getDraftCheckpoint resolutions cannot update state after filePath changes, while preserving the existing active-load behavior.Source: Linters/SAST tools
🟡 Minor comments (14)
src/App.css-1364-1376 (1)
1364-1376: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winLowercase the
currentcolorkeyword to satisfy Stylelint.Stylelint reports
value-keyword-caseerrors on bothcurrentColorvalues. CSS-wide keywords are expected in lowercase by this rule.🎨 Proposed fix for the keyword casing
.notion-table-edge-resize::before { content: ""; width: 12px; height: 4px; - border-block: 1px solid currentColor; + border-block: 1px solid currentcolor; } .notion-table-edge-resize.is-column::before { width: 4px; height: 12px; border-block: 0; - border-inline: 1px solid currentColor; + border-inline: 1px solid currentcolor; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/App.css` around lines 1364 - 1376, Update both border declarations in .notion-table-edge-resize::before and .notion-table-edge-resize.is-column::before to use the lowercase currentcolor keyword, preserving all other styling unchanged.Source: Linters/SAST tools
src/App.css-1470-1479 (1)
1470-1479: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReplace the deprecated
clipproperty withclip-path.Stylelint reports
property-no-deprecatedforclip..notion-table-announcementis a visually hidden live region. Useclip-path: inset(50%)for the same result with a supported property.🎨 Proposed fix for the visually hidden announcement region
.notion-table-announcement { position: fixed; width: 1px; height: 1px; padding: 0; overflow: hidden; - clip: rect(0 0 0 0); + clip-path: inset(50%); white-space: nowrap; border: 0; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/App.css` around lines 1470 - 1479, Update the .notion-table-announcement visually hidden styles by removing the deprecated clip declaration and using clip-path: inset(50%) instead, while preserving the existing dimensions, overflow, whitespace, padding, and border behavior.Source: Linters/SAST tools
src/components/editor/notion/tableClipboard.ts-47-77 (1)
47-77: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTreat
"as a quote character only at the start of a cell.The loop toggles
quotedfor any", including quotes inside an unquoted cell. A paste ofhe said "hi" today<TAB>footherefore loses both quote characters. RFC 4180 style parsers only enter quoted mode when the cell is empty.🐛 Proposed fix
const character = text[index]; - if (character === '"') { + if (character === '"' && (quoted || cell === "")) { if (quoted && text[index + 1] === '"') {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/editor/notion/tableClipboard.ts` around lines 47 - 77, Update the quote handling in the table text parsing loop so an unquoted double quote is appended as cell content unless it appears at the start of an empty cell; only then should it enter quoted mode. Preserve doubled-quote escaping and existing delimiter/newline handling within quoted cells, including quotes in content such as `he said "hi" today`.src/components/editor/notion/tableTransactions.ts-593-625 (1)
593-625: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject inserting above the pinned header row.
insertTableRowcan receiverowIndex === 0, for example from an explicit test call. In that case the inserted row is spliced intorows[0], thenrecreateTableWithStructuralHeadersappliestableHeaderto the new first row and converts the previous header cells totableCell, so the header labels become table data. Add the samerowIndex === 0guarddeleteTableRowuses for pinned header-row tables.🐛 Proposed guard
if ( !Number.isInteger(rowIndex) || rowIndex < 0 || - rowIndex > table.childCount + rowIndex > table.childCount || + (hasPinnedTableHeaderRow(table) && rowIndex === 0) ) { return false; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/editor/notion/tableTransactions.ts` around lines 593 - 625, Update insertTableRow to reject rowIndex === 0 when the table uses a pinned header row, matching the guard in deleteTableRow. Add this validation before modifying rows so the existing header row remains unchanged, while preserving insertion behavior for valid non-header indices.src/components/editor/notion/tableExtensions.ts-21-26 (1)
21-26: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGive the user feedback when a nested-table edit is rejected.
filterTransactiondrops the whole transaction when the resulting document contains a nested table. If the user pastes content that contains a table into a cell, the paste disappears with no message and no undo entry. The user cannot tell whether the paste failed or the app is unresponsive.Consider normalizing the incoming content with
normalizeNestedTablesInJsonin the paste handler, or surfacing a short notice when the plugin rejects a transaction.As per coding guidelines: "Implement error handling with user-friendly messages".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/editor/notion/tableExtensions.ts` around lines 21 - 26, Update the nested-table protection around ScratchTableRow and its scratchTableIntegrity plugin so rejected paste edits provide user feedback instead of silently disappearing. Prefer normalizing pasted content with normalizeNestedTablesInJson in the paste handler; otherwise surface a brief user-friendly notice when filterTransaction rejects the transaction, while preserving the existing integrity enforcement.Source: Coding guidelines
src/components/editor/notion/tableRowResize.ts-29-31 (1)
29-31: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse
toLowerCase()instead oftoLocaleLowerCase()for tag names.
toLocaleLowerCase()applies locale rules. Under a Turkish locale,"LI"becomes"lı"and"HTML"becomes"html"only by chance for the letters involved. Any tag name that containsIproduces a dotlessıand the generated selector matches nothing. The row height preview then silently fails for users with a Turkish locale.Tag names are ASCII, so use the locale-invariant method.
🐛 Proposed fix
segments.unshift( - `${current.tagName.toLocaleLowerCase()}:nth-child(${siblingIndex})`, + `${current.tagName.toLowerCase()}:nth-child(${siblingIndex})`, );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/editor/notion/tableRowResize.ts` around lines 29 - 31, In the selector construction within the table row resize logic, replace current.tagName.toLocaleLowerCase() with the locale-invariant toLowerCase() method. Preserve the existing nth-child selector formatting and segment insertion behavior.src/components/editor/notion/tableRowResize.ts-20-36 (1)
20-36: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUse a stable row selector for the height preview.
getExactElementSelectorencodes the ancestor chain with:nth-child, so DOM changes or sibling reordering make the!importantheight rule stop applying or target the wrong row. Since the row DOM can be replaced by ProseMirror, anchor the preview to an unchanging row attribute and target that attribute selector instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/editor/notion/tableRowResize.ts` around lines 20 - 36, Update getExactElementSelector to return a selector based on the row’s stable, unchanging attribute instead of constructing an ancestor chain with :nth-child. Ensure the height preview’s !important rule continues targeting the same row when ProseMirror replaces or reorders DOM nodes.src/components/editor/notion/tableView.ts-44-92 (1)
44-92: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
restoredoes not remove thecolelements that the preview added.Lines 44-46 append
colelements when the colgroup holds fewer columns thanbaselineWidths.restoreonly restores thestyleattribute of the sliced columns, so the appended elements stay in the DOM. The table then renders extra empty columns until the nextupdateScratchTableColumnscall trims them. Track the appended elements and remove them inrestore. Also ignoreapplyafterrestore, so a late pointer event cannot reapply preview styles that nothing removes.🐛 Proposed fix
- while (colgroup.children.length < normalizedBaseline.length) { - colgroup.appendChild(colgroup.ownerDocument.createElement("col")); - } + const appendedColumns: HTMLTableColElement[] = []; + while (colgroup.children.length < normalizedBaseline.length) { + const column = colgroup.ownerDocument.createElement("col"); + colgroup.appendChild(column); + appendedColumns.push(column); + } @@ const apply = (requestedWidth: number): number => { + if (restored) return minimumWidth; const normalized = normalizeTableColumnWidth( @@ restored = true; restoreAttribute(table, "style", tableStyle); restoreAttribute(table, "data-fit-to-width", fitToWidth); restoreAttribute(table, "data-column-resizing", resizing); columns.forEach((column, index) => restoreAttribute(column, "style", columnStyles[index]), ); + appendedColumns.forEach((column) => column.remove()); },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/editor/notion/tableView.ts` around lines 44 - 92, Update the preview setup around the colgroup expansion and returned apply/restore methods to track every appended col element, remove those elements during restore, and make apply a no-op after restore. Preserve restoration of the original table and existing column attributes while preventing late pointer events from reapplying preview styles.src/components/preview/PreviewApp.tsx-83-90 (1)
83-90: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winTell the user when the draft went to recovery storage.
This call ignores the resolved
SafeWindowCloseResult. Ifflush()fails andpersistRecoverySnapshotsucceeds, the window closes silently and the user does not learn where the draft was written.src/App.tsxreportsrecoveredToin the same situation. Add the same report here.🛠️ Proposed fix
- }).catch((error) => { + }) + .then((result) => { + if (result.recoveredTo && !disposed) { + toast.warning(`Draft recovered to ${result.recoveredTo}`); + } + }) + .catch((error) => { closeInProgressRef.current = false; if (!disposed) { toast.error( `Window kept open because the draft could not be saved: ${error}`, ); } });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/preview/PreviewApp.tsx` around lines 83 - 90, Update the close/flush handling around the SafeWindowCloseResult in PreviewApp so successful recovery-storage persistence reports the returned recoveredTo location to the user, matching the reporting behavior in App.tsx. Preserve the existing error toast for unsaved drafts and reset closeInProgressRef as before.src/context/NotesContext.tsx-341-363 (1)
341-363: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAvoid re-sending a save conflict while the conflict UI is active.
A save conflict throws before updating
noteRevisionByIdRefwith the remote revision, so each 300ms auto-save retry uses the same stale revision and repeats the same failure. SuppresssetErrorwhen a conflict already exists for the current note, and clear it only after the conflict is resolved or reloaded. The auto-save callers already catch and log the rejection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/context/NotesContext.tsx` around lines 341 - 363, Update the save-conflict handling around setError and noteRevisionByIdRef so an active conflict for the current note suppresses repeated error updates from auto-save retries. Preserve the conflict UI state, and clear the suppression only when the conflict is resolved or the note is reloaded; keep the existing auto-save rejection logging unchanged.src/lib/windowSession.ts-132-144 (1)
132-144: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winA re-queued patch has no retry timer, so a failed write can be lost.
When
write(patch)rejects, the catch handler restores the patch intopendingand rethrows. No timer is scheduled for the restored patch.flushonly runs again whenqueue()is called or when the caller flushes explicitly. If no further state change occurs, the re-queued patch is never written, and the session update is silently lost after a transient backend failure.Schedule a retry when the write fails.
🛡️ Proposed fix
+ function scheduleFlush(): void { + if (cancelled || timer !== null) return; + timer = setTimeout(() => { + timer = null; + void flush().catch((error: unknown) => options.onError?.(error)); + }, delayMs); + } + async function flush(): Promise<void> { @@ activeWrite = write(patch) .catch((error: unknown) => { if (!cancelled) { pending = pending ? { ...patch, ...pending } : patch; + scheduleFlush(); } throw error; })If the intent is that the unmount flush in
useWindowSessionPersistenceis the only retry, state that in a comment so a reader does not treat the re-queue as an automatic retry.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/windowSession.ts` around lines 132 - 144, Update the write failure handling in the flush logic around activeWrite so that, after restoring the failed patch to pending, it schedules a retry for that pending patch when the session is not cancelled. Reuse the existing queue or timer mechanism rather than relying on a later queue call, and preserve the current error propagation and activeWrite cleanup behavior.src/context/NotesContext.tsx-193-203 (1)
193-203: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win**Resolve
sourcePathfrom the draft note.
persistCurrentDraftRecoveryusesdraft.noteIdasnoteIdbut readssourcePathfromcurrentNoteRef.current. If the draft belongs to a different note, the recovery metadata points to the wrong file. ResolvecurrentNotefromdraft.noteIdbefore building the snapshot, rather than passingnoteIdandsourcePathin from different sources.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/context/NotesContext.tsx` around lines 193 - 203, Update persistCurrentDraftRecovery to resolve the note associated with draft.noteId before constructing the recovery snapshot, and derive both noteId and sourcePath from that same draft note. Do not use currentNoteRef.current for sourcePath, and preserve the existing early return for clean drafts or missing note IDs.src/lib/windowSession.ts-56-75 (1)
56-75: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winNormalize persisted session fields in the frontend restore path.
restoreWindowSessioncoerces restored values intoRestoredWindowSession, so older records are not required to have the Rust#[default]values. UseDEFAULT_RESTORED_WINDOW_SESSIONdefaults for missing falsy flags and reject malformedgeometrybefore returning it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/windowSession.ts` around lines 56 - 75, Update restoreWindowSession to normalize persisted sidebarVisible and focusMode values using the corresponding DEFAULT_RESTORED_WINDOW_SESSION defaults when fields are missing, while preserving valid explicit false values. Validate saved.geometry before returning it; if malformed, return the safe default geometry or the complete DEFAULT_RESTORED_WINDOW_SESSION result. Keep the existing workspace and selectedNoteId validation behavior.src/context/ThemeContext.tsx-472-492 (1)
472-492: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd error handling to
resetEditorFontSettingsandupdateTitleBarNoteInfo.Both functions call
updateGlobalSettingswithout atry/catch. Every other setter added in this diff (setEditorWidthResizeEnabled,setEditorToolbarVisible,setSidebarWidthPx,setCustomEditorWidthPx) catches the error, logs it, and (in some cases) surfaces it. Without a catch here, a failedupdateGlobalSettingscall produces an unhandled promise rejection and no user-facing feedback, while the local state has already been optimistically updated.Add the same catch-and-log pattern used by the sibling setters in this file.
🛠️ Proposed fix
setCustomColorsLightState({}); setCustomColorsDarkState({}); - await updateGlobalSettings({ - editorFont: defaultEditorFontSettings, - textDirection: "auto", - editorWidth: "normal", - interfaceZoom: 1.0, - customEditorWidthPx: null, - editorWidthResizeEnabled: null, - editorToolbarVisible: null, - titleBarModifiedDateVisible: null, - titleBarFilenameVisible: null, - sidebarWidthPx: null, - customColorsLight: null, - customColorsDark: null, - }); + try { + await updateGlobalSettings({ + editorFont: defaultEditorFontSettings, + textDirection: "auto", + editorWidth: "normal", + interfaceZoom: 1.0, + customEditorWidthPx: null, + editorWidthResizeEnabled: null, + editorToolbarVisible: null, + titleBarModifiedDateVisible: null, + titleBarFilenameVisible: null, + sidebarWidthPx: null, + customColorsLight: null, + customColorsDark: null, + }); + } catch (error) { + console.error("Failed to reset editor settings:", error); + } }, [applyTitleBarNoteInfoVisibility]);applyTitleBarNoteInfoVisibility(next); - void updateGlobalSettings({ - titleBarModifiedDateVisible: next.modifiedDateVisible, - titleBarFilenameVisible: next.filenameVisible, - }); + updateGlobalSettings({ + titleBarModifiedDateVisible: next.modifiedDateVisible, + titleBarFilenameVisible: next.filenameVisible, + }).catch((error) => { + console.error("Failed to save title bar visibility:", error); + }); }, [applyTitleBarNoteInfoVisibility], );Also applies to: 569-583
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/context/ThemeContext.tsx` around lines 472 - 492, Update resetEditorFontSettings and updateTitleBarNoteInfo to wrap their updateGlobalSettings calls in the same try/catch pattern used by sibling setters such as setEditorWidthResizeEnabled and setEditorToolbarVisible. Log failures consistently and surface user-facing feedback where the existing setter pattern does so, preventing unhandled promise rejections after optimistic local state updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/src/draft_checkpoint.rs`:
- Around line 120-128: Update the checkpoint iteration in list_checkpoints to
treat read, JSON parsing, and ensure_identity_matches failures as invalid
entries: skip those entries and continue collecting valid checkpoints instead of
propagating the error. Preserve propagation of directory-entry path errors and
return the successfully collected checkpoints.
In `@src-tauri/src/lib.rs`:
- Around line 3954-3959: Update the import-selection logic in
import_file_to_folder to emit select-note to window.label() instead of the
hardcoded "main" target, then retrieve, show, and focus that same invoking
window rather than the main window.
- Around line 2514-2563: Update write_recovery_snapshot so a Conflict from
persistence::save_if_revision does not immediately return an error: retry with a
newly generated unique sequence or suffix and recompute the Markdown and
metadata paths until the snapshot is saved, then return the successful recovery
path. Preserve propagation of other persistence errors and only report failure
after the collision-retry strategy is exhausted.
In `@src/components/editor/notion/imageInteractions.ts`:
- Around line 8-34: Update getImageOpenTarget to validate resolved asset paths
against the workspace root, configured assets, or app-scoped preview-file
directories before returning a path target; reject all other absolute paths.
Also restrict the asset.localhost handling to approved http/https URL forms,
preserving normal external http/https URL handling while preventing edited asset
URLs from reaching openPath for arbitrary local files.
In `@src/components/editor/notion/TableControls.tsx`:
- Around line 1043-1064: Move the sourceIndex < indexOffset guard in the pointer
interaction handler before preventDefault, stopPropagation, clearHideTimer, and
activeInteractionRef.current assignment. Ensure invalid pinned-header-row
interactions return without mutating interaction state or showing a proximity
target, while valid interactions retain the existing flow.
- Around line 765-795: Update the row resize setup in the handler containing
applyPreview to validate rowIndex against layout.rowRects.length before
accessing the row rectangle. Use the validated row rectangle consistently for
startHeight and the origin.style.top calculation, preventing applyPreview from
dereferencing a missing layout.rowRects[rowIndex] while preserving the existing
default height behavior only where appropriate.
- Around line 1801-1812: Update the evaluatePointer flow to refresh layout
whenever the newly measured geometry differs, keeping it synchronized with
showProximityTarget indices. In the column-resize render branch, validate that
proximityTarget.index is within layout.columnRects before reading its entry, and
avoid rendering the resize button when the index is out of bounds.
In `@src/components/editor/notion/tableExtensions.ts`:
- Around line 27-32: Replace the transaction.doc.toJSON() call in
filterTransaction with a node-based document check, adding and reusing a
docContainsNestedTable function alongside containsNestedTable in
tableIntegrity.ts. Ensure the traversal exits early once a nested table is found
and preserves the existing filter behavior for unchanged documents and documents
without nested tables.
In `@src/components/layout/Sidebar.tsx`:
- Around line 98-110: Update handleSwitchWorkspace to handle switchWorkspace
separately from the post-switch reloadSettings and refreshWorkspaces operations.
Show “Workspace switch cancelled” only when switchWorkspace fails; if either
post-switch refresh step fails after a successful switch, log the error and
display an accurate post-switch refresh failure message.
In `@src/context/NotesContext.tsx`:
- Around line 815-837: Update loadWorkspaceState to wrap the workspace-loading
operations—notesService.listNotes, restoreWorkspaceSession, and
notesService.startFileWatcher—in a try/finally block, and reset
isWindowSessionRestored to true in finally so failures do not permanently
disable session persistence.
- Around line 458-462: Update the checkpoint-clearing flow around
clearDraftCheckpoint in NotesContext so it does not pass an empty windowLabel
that targets the preview checkpoint. Preserve the winning checkpoint’s stored
window label by using an invoke path that retains the checkpoint key, or perform
the invocation from the window associated with the won checkpoint, while keeping
draft.noteId unchanged.
- Around line 890-900: Update syncNotesFolder to use one workspace transition
queue path: flush the current draft, then enqueue loadWorkspaceState through
queueWorkspaceTransition instead of allowing overlapping transitions. Preserve
its existing error reporting and propagation. Remove the repeated
startFileWatcher call from loadWorkspaceState, relying on the existing watcher
when the backend command is a no-op.
In `@src/lib/draftCheckpoint.ts`:
- Around line 97-110: Update the scheduler around schedule and markDirty to
track the timestamp of the first dirty edit in the current window, then call
nextCheckpointCaptureDelay with the elapsed time so repeated edits cannot extend
the wait beyond the helper’s maximum. Reset the tracked timestamp only after a
successful flush/write, while preserving existing cancellation and error
handling.
In `@src/lib/standaloneRecreation.ts`:
- Around line 8-20: Implement the production standalone recreation adapter in
the real backend command path, using the `recreateDeletedStandaloneDraft`
contract to invoke the backend recreation operation and return its file result.
When the backend reports a conflict, preserve the conflict by attaching the
current file/content to the returned conflict result rather than throwing, so
the PreviewApp call path remains connected.
In `@src/lib/useWindowSessionPersistence.ts`:
- Around line 181-184: Update the returned persist callback in
useWindowSessionPersistence to catch and ignore failures from
geometryCaptureRef.current(), then always execute writer.flush() so
runSafeWindowClose can proceed to closeWindow even when captureGeometry geometry
reads reject.
In `@src/lib/windowClose.ts`:
- Around line 12-26: Update runSafeWindowClose to treat an undefined
persistRecovery result as an unrecovered draft and prevent silent window
closure, then update src/App.tsx lines 104-145 to toast result.saveError when
recoveredTo is absent and update src/components/preview/PreviewApp.tsx lines
83-90 to inspect the result and report both recoveredTo and unrecovered
saveError.
---
Outside diff comments:
In `@src/components/preview/PreviewApp.tsx`:
- Around line 103-133: Update the file-loading useEffect for PreviewApp so it
tracks cancellation for each filePath run and checks that flag before applying
recovered content, title, modified state, or revision updates and before related
checkpoint/conflict notifications. Set the flag in the effect cleanup so stale
readFileDirect or getDraftCheckpoint resolutions cannot update state after
filePath changes, while preserving the existing active-load behavior.
---
Minor comments:
In `@src/App.css`:
- Around line 1364-1376: Update both border declarations in
.notion-table-edge-resize::before and
.notion-table-edge-resize.is-column::before to use the lowercase currentcolor
keyword, preserving all other styling unchanged.
- Around line 1470-1479: Update the .notion-table-announcement visually hidden
styles by removing the deprecated clip declaration and using clip-path:
inset(50%) instead, while preserving the existing dimensions, overflow,
whitespace, padding, and border behavior.
In `@src/components/editor/notion/tableClipboard.ts`:
- Around line 47-77: Update the quote handling in the table text parsing loop so
an unquoted double quote is appended as cell content unless it appears at the
start of an empty cell; only then should it enter quoted mode. Preserve
doubled-quote escaping and existing delimiter/newline handling within quoted
cells, including quotes in content such as `he said "hi" today`.
In `@src/components/editor/notion/tableExtensions.ts`:
- Around line 21-26: Update the nested-table protection around ScratchTableRow
and its scratchTableIntegrity plugin so rejected paste edits provide user
feedback instead of silently disappearing. Prefer normalizing pasted content
with normalizeNestedTablesInJson in the paste handler; otherwise surface a brief
user-friendly notice when filterTransaction rejects the transaction, while
preserving the existing integrity enforcement.
In `@src/components/editor/notion/tableRowResize.ts`:
- Around line 29-31: In the selector construction within the table row resize
logic, replace current.tagName.toLocaleLowerCase() with the locale-invariant
toLowerCase() method. Preserve the existing nth-child selector formatting and
segment insertion behavior.
- Around line 20-36: Update getExactElementSelector to return a selector based
on the row’s stable, unchanging attribute instead of constructing an ancestor
chain with :nth-child. Ensure the height preview’s !important rule continues
targeting the same row when ProseMirror replaces or reorders DOM nodes.
In `@src/components/editor/notion/tableTransactions.ts`:
- Around line 593-625: Update insertTableRow to reject rowIndex === 0 when the
table uses a pinned header row, matching the guard in deleteTableRow. Add this
validation before modifying rows so the existing header row remains unchanged,
while preserving insertion behavior for valid non-header indices.
In `@src/components/editor/notion/tableView.ts`:
- Around line 44-92: Update the preview setup around the colgroup expansion and
returned apply/restore methods to track every appended col element, remove those
elements during restore, and make apply a no-op after restore. Preserve
restoration of the original table and existing column attributes while
preventing late pointer events from reapplying preview styles.
In `@src/components/preview/PreviewApp.tsx`:
- Around line 83-90: Update the close/flush handling around the
SafeWindowCloseResult in PreviewApp so successful recovery-storage persistence
reports the returned recoveredTo location to the user, matching the reporting
behavior in App.tsx. Preserve the existing error toast for unsaved drafts and
reset closeInProgressRef as before.
In `@src/context/NotesContext.tsx`:
- Around line 341-363: Update the save-conflict handling around setError and
noteRevisionByIdRef so an active conflict for the current note suppresses
repeated error updates from auto-save retries. Preserve the conflict UI state,
and clear the suppression only when the conflict is resolved or the note is
reloaded; keep the existing auto-save rejection logging unchanged.
- Around line 193-203: Update persistCurrentDraftRecovery to resolve the note
associated with draft.noteId before constructing the recovery snapshot, and
derive both noteId and sourcePath from that same draft note. Do not use
currentNoteRef.current for sourcePath, and preserve the existing early return
for clean drafts or missing note IDs.
In `@src/context/ThemeContext.tsx`:
- Around line 472-492: Update resetEditorFontSettings and updateTitleBarNoteInfo
to wrap their updateGlobalSettings calls in the same try/catch pattern used by
sibling setters such as setEditorWidthResizeEnabled and setEditorToolbarVisible.
Log failures consistently and surface user-facing feedback where the existing
setter pattern does so, preventing unhandled promise rejections after optimistic
local state updates.
In `@src/lib/windowSession.ts`:
- Around line 132-144: Update the write failure handling in the flush logic
around activeWrite so that, after restoring the failed patch to pending, it
schedules a retry for that pending patch when the session is not cancelled.
Reuse the existing queue or timer mechanism rather than relying on a later queue
call, and preserve the current error propagation and activeWrite cleanup
behavior.
- Around line 56-75: Update restoreWindowSession to normalize persisted
sidebarVisible and focusMode values using the corresponding
DEFAULT_RESTORED_WINDOW_SESSION defaults when fields are missing, while
preserving valid explicit false values. Validate saved.geometry before returning
it; if malformed, return the safe default geometry or the complete
DEFAULT_RESTORED_WINDOW_SESSION result. Keep the existing workspace and
selectedNoteId validation behavior.
---
Nitpick comments:
In `@src-tauri/src/lib.rs`:
- Around line 1257-1274: Remove the final self.workspace_session(window_label)
lookup from workspace_for_window and return the existing “Workspace session not
found” error directly after the fallback handling, preserving the current
successful session and main-workspace return paths.
In `@src-tauri/src/persistence.rs`:
- Around line 121-136: Document the concurrency guarantee on save_if_revision
with a short doc comment stating that its compare-and-swap protection is limited
to callers within the same process and does not prevent interleaving writes from
other processes or external editors.
- Around line 270-311: Consolidate the duplicated SHA-256 implementation used by
persistence.rs lines 270-311 and draft_checkpoint.rs lines 325-410. Move sha256
and sha256_compress into one shared module as pub(crate) helpers, then update
both call sites to use that implementation; alternatively, replace both with the
maintained crate::sha2::Sha256 API. Remove the duplicate constants, padding, and
round logic while preserving existing hash results.
In `@src/components/editor/Editor.tsx`:
- Around line 1048-1064: Move the assignment to
queueCheckpointCaptureRef.current out of the render path and into an appropriate
React effect, while preserving the existing debounce, timer cleanup, and
persistCurrentCrashCheckpoint behavior. Ensure the callback is updated only
after the render commits and remains available to consumers using the ref.
- Around line 1696-1731: The drag-event handler around currentWindow.scaleFactor
should stop awaiting the async IPC call for every event. Resolve the scale
factor once per drag or effect run, store it in a ref, and have the position
conversion reuse the cached value; ensure the cache is initialized before
handling events and reset or refreshed at the appropriate drag boundary to
prevent stale indicator updates.
In `@src/components/editor/editorHistory.ts`:
- Around line 36-45: Add a concise comment immediately before the
EditorState.create call in the editor state reload flow, identifying that
recreating the state intentionally resets all plugin states after loading a note
because those states are transient. Leave the existing schema, document,
selection, and plugin initialization unchanged.
In `@src/components/editor/imageDrop.ts`:
- Around line 75-100: Extract the shared bounds predicate into an
isPointInsideRect helper near resolveImageDropPosition and use it in both
resolveImageDropPosition and resolveBlockDropTarget, replacing their duplicated
isInsideEditor calculations while preserving the existing null-return behavior.
In `@src/components/editor/notion/interfaceGeometry.test.ts`:
- Around line 20-25: Update the test "falls back to one for a missing or invalid
zoom" so the invalid-zoom scenario exercises the intended path rather than
reusing an empty CSSOM value; use a stubbed style with setProperty("zoom",
"invalid") or verify the assigned style.zoom and adjust the setup accordingly,
while preserving the existing fallback assertions.
In `@src/components/editor/notion/markdownDocument.test.ts`:
- Around line 575-600: The nested-table test does not reach the guard because
its metadata lacks cellMarkdownSourceBase64. Update the scratch-table metadata
in “rejects preserved cell metadata that would create a nested table” to include
a matching cellMarkdownSourceBase64 entry encoding the visible cell content,
while preserving the existing nested-table cellMarkdownBase64 payload and
assertions.
In `@src/components/editor/notion/markdownDocument.ts`:
- Around line 71-93: Update parseEncodedCellMatrix to validate each string entry
using only its allowed length and character-set rules, without calling
decodeBase64Utf8. Preserve the existing dimension and null validation, and rely
on parsePreservedCellBlocks to decode each accepted entry once and reject
decoding failures.
In `@src/components/editor/notion/selectionDecoration.test.ts`:
- Around line 61-64: Update the appStyles fixture loading in
selectionDecoration.test.ts to resolve src/App.css relative to the test module
location rather than process.cwd(). Preserve the existing UTF-8 file read and
ensure the path remains valid when Vitest runs from a subdirectory.
In `@src/components/editor/notion/TableControls.test.tsx`:
- Around line 211-232: Add positive DOM assertions to the absence-only tests in
the shown blocks and the corresponding tests around the later referenced ranges.
After rendering or pointer interaction, assert that a known required
TableControls element, such as the row or column resize handle queried by
subsequent tests, exists before asserting that the prohibited controls are
absent.
In `@src/components/editor/notion/tableEdgeDrag.ts`:
- Around line 23-28: Update the table edge drag calculation around the steps and
delta values to introduce a finite upper bound for growth, while preserving the
existing minimum removal clamp and zero-normalization behavior. Clamp positive
drag steps to the chosen maximum before returning the delta so large gestures
cannot add an unbounded number of rows or columns.
In `@src/components/editor/notion/tableExtensions.test.ts`:
- Around line 227-229: In the table focus-decoration test, update the assertion
on the “.scratch-table-cell-focused” query to require exactly zero matches with
toBe(0), preserving the existing CellSelection regression coverage.
In `@src/components/editor/notion/tablePerformance.test.ts`:
- Around line 58-115: The wall-clock performance assertions are too
environment-dependent for correctness tests. In
src/components/editor/notion/tablePerformance.test.ts#L58-L115, remove the
elapsed/budgetMs gating (or move it to a non-gating benchmark) while preserving
the structural, content, and nested-table assertions; in
src/components/editor/notion/tableIntegrity.test.ts#L145-L170, likewise remove
or relocate the 500ms timing check while retaining the toHaveLength, every, and
sourceText equality validations.
In `@src/components/editor/notion/tableProximity.ts`:
- Around line 74-79: In the table proximity logic surrounding the table-zone
check, extract the literal offsets 48 and 18 into clearly named constants such
as TABLE_ZONE_MAX_OFFSET and TABLE_ZONE_MIN_OFFSET, then use those constants in
both between calls while preserving the existing bounds and behavior.
In `@src/components/editor/notion/tableRowResize.ts`:
- Around line 11-18: Update normalizeTableRowHeight in tableRowResize.ts to
reuse the exported normalizeTableRowHeight from tableExtensions.ts instead of
duplicating rounding and clamping. Retain only this module’s non-finite-height
fallback, passing finite values to the shared function and preserving the
existing numeric return behavior.
In `@src/components/editor/notion/tableTransactions.ts`:
- Line 807: Update the columnWidth calculation near the table transaction logic
to use the imported MIN_TABLE_COLUMN_WIDTH constant instead of the literal 80,
extending the existing import from tableMetadata.ts as needed while preserving
the current Math.max behavior.
In `@src/components/editor/notion/tableView.test.ts`:
- Around line 126-141: Update the CSS test around the appStyles, wrapperRule,
and tableStyleSection symbols to resolve App.css relative to import.meta.url
rather than process.cwd(), verifying the correct relative path from this test
file. Assert that both regex matches are defined before checking their contents,
so missing CSS sections fail explicitly; keep the existing content assertions
unchanged and avoid relying on comment text for locating the table-style
section.
In `@src/components/layout/Sidebar.tsx`:
- Around line 261-271: Filter SETTINGS_CHANGED_DOM_EVENT by workspace in
Sidebar.tsx around the useEffect at lines 261-271: read event.detail and call
loadWorkspaceSettings() only when detail.scope is "global" or detail.workspace
matches notesFolder. Apply the same filtering in NoteList.tsx around lines
301-309 by destructuring notesFolder from useNotes(), reading event.detail, and
calling refreshSettings() only under that condition.
In `@src/context/NotesContext.tsx`:
- Around line 135-138: Move the render-time assignments to
currentNoteRef.current and notesFolderRef.current into an effect in the
NotesContext component, syncing both refs after commit. Preserve the existing
imperative assignments in async callbacks and ensure no synchronous pre-commit
caller depends on these refs.
- Around line 489-498: Refactor the selected-note clearing logic around
setSelectedNoteId so the prevId === id condition is computed before invoking the
updater, then perform the selectedNoteIdRef, currentNoteRef, setCurrentNote, and
setNoteConflict side effects outside the updater. Keep the updater pure by only
returning the appropriate selected note ID.
- Around line 782-788: Replace the orphaned-checkpoint notification’s use of
setError in the recovery-checkpoint flow with a dedicated informational notice
state or exposed orphaned-checkpoint data. Preserve the message and count, and
ensure the UI can retain the recovery notice and offer an appropriate recovery
action without routing it through the application error surface.
- Around line 82-87: Update the registration behavior for
registerWorkspaceTransitionFlush and registerOpenNoteDraft in NotesContext so
multiple simultaneous components do not overwrite each other: store
registrations in collections, invoke every registered handler, and remove only
the unsubscribed handler. If single-editor mounting is an intentional invariant
instead, document that constraint in the interface comment.
- Line 1096: Update the file-change listener effect in NotesContext to store
noteConflict in a ref and read the ref inside the listener callback, rather than
capturing noteConflict directly. Remove noteConflict from the effect dependency
array while preserving refreshNotes as the remaining dependency, so the Tauri
listener is not re-registered during conflict transitions.
In `@src/context/ThemeContext.tsx`:
- Around line 130-137: Split ThemeContext following the existing NotesContext
dual-context pattern: place theme/settings state in a data context and setter
functions in a separate actions context, update ThemeContext.Provider to supply
both values, and adjust useTheme and related consumers to read from the
appropriate context while preserving the existing API behavior.
In `@src/lib/conflictResolution.test.ts`:
- Around line 4-5: Add a clean-draft test in the conflict resolution test suite
using the existing draft/remote fixtures, setting draft.dirty to false and
making persistRecovery return undefined. Assert the clean branch skips recovery
snapshot persistence and completes without throwing, covering the false path of
the dirty check in conflictResolution.
In `@src/lib/draftCheckpoint.ts`:
- Around line 151-176: Move the type-only Note import to the top of the file
alongside the other module-level imports, while leaving reconcileDraftCheckpoint
and its behavior unchanged.
In `@src/lib/useWindowShortcuts.ts`:
- Around line 14-17: Move the current-value assignments for interfaceZoomRef and
openPreferencesRef out of the render body and into an effect in
useWindowShortcuts. Ensure the effect updates both refs after commit while
preserving their existing use by the shortcut handlers.
In `@src/lib/windowClose.test.ts`:
- Around line 24-46: Extend the runSafeWindowClose tests with a case where
flushDraft fails and persistRecovery resolves to undefined; verify closeWindow
still runs, the result contains saveError, and recoveredTo is omitted. Preserve
the existing ordering assertions to ensure recovery is attempted before closing.
In `@src/lib/windowCloseCallsites.test.ts`:
- Around line 14-19: Broaden the assertions in the test around
`closeWindowAfterSave` to reject any direct `.close()` or `.destroy()` window
method call, regardless of the receiver identifier. Preserve the existing
requirement that the source contains `closeWindowAfterSave`.
In `@src/lib/windowSession.test.ts`:
- Around line 18-87: Add a test within the restoreWindowSession suite covering a
saved session whose workspace differs from the requested workspace, and assert
that restoreWindowSession returns the same safe defaults used for an unavailable
session without restoring the saved selection or session state.
- Around line 102-134: Add tests for createWindowSessionPatchWriter covering
failed writes and cancellation: verify a rejected write re-queues the failed
patch while preserving newer pending fields via the existing merge order, and
verify cancel drops pending work and causes subsequent queue calls to be
rejected. Use fake timers and the existing write mock to assert retries and
write counts.
In `@src/lib/windowShortcutCallsites.test.ts`:
- Around line 22-31: Update the test case “keeps Back only for in-window
Settings navigation” to explicitly assert that preferencesStart and
preferencesEnd are not -1 immediately after locating the function markers,
before calling source.slice. Keep the existing source-content assertions
unchanged.
In `@src/lib/windowShortcuts.test.ts`:
- Around line 5-22: Add at least one Ctrl-modified entry to the parameterized
cases for resolveWindowShortcut, setting ctrlKey to true and metaKey to false
while asserting the expected shortcut action. Preserve the existing Cmd cases
and test the Windows/Linux modifier path.
In `@src/lib/workspaceSwitch.test.ts`:
- Around line 5-23: Update the test using runWorkspaceSwitch so
switchBackendWorkspace returns a path different from the requested
"/notes/client" value, and assert that loadWorkspace receives the
backend-returned active path. Preserve the existing flush, switch, and load
ordering assertion.
In `@src/services/draftCheckpoint.ts`:
- Around line 23-27: Update clearDraftCheckpoint to accept only the noteId value
it forwards to clear_draft_checkpoint, removing the misleading
DraftCheckpointKey parameter. Adjust callers such as
DraftCheckpointScheduler.handleSaveOutcome to pass noteId directly; only retain
a key parameter there if required, with a brief comment stating that the backend
derives windowLabel.
In `@src/services/files.test.ts`:
- Around line 9-38: Extend the saveFileDirect tests to mock a response with
status "conflict" containing distinct content and revision values, then assert
the service returns that response data through the conflict path. Keep the
existing saved-status request assertion unchanged and cover both returned
content and revision fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 35e8bb68-aac5-4069-832b-b8b94c457c7f
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (121)
package.jsonsrc-tauri/capabilities/default.jsonsrc-tauri/src/draft_checkpoint.rssrc-tauri/src/lib.rssrc-tauri/src/note_persistence_tests.rssrc-tauri/src/persistence.rssrc-tauri/src/watcher_debounce.rssrc/App.csssrc/App.tsxsrc/components/editor/Editor.tsxsrc/components/editor/EditorWidthHandle.test.tsxsrc/components/editor/EditorWidthHandle.tsxsrc/components/editor/editorHistory.test.tssrc/components/editor/editorHistory.tssrc/components/editor/imageDrop.test.tssrc/components/editor/imageDrop.tssrc/components/editor/notion/NotionMenus.test.tsxsrc/components/editor/notion/NotionMenus.tsxsrc/components/editor/notion/TableControls.test.tsxsrc/components/editor/notion/TableControls.tsxsrc/components/editor/notion/imageInteractions.test.tssrc/components/editor/notion/imageInteractions.tssrc/components/editor/notion/interfaceGeometry.test.tssrc/components/editor/notion/interfaceGeometry.tssrc/components/editor/notion/markdownDocument.test.tssrc/components/editor/notion/markdownDocument.tssrc/components/editor/notion/markdownMarks.test.tssrc/components/editor/notion/markdownMarks.tssrc/components/editor/notion/selectionDecoration.test.tssrc/components/editor/notion/selectionDecoration.tssrc/components/editor/notion/tableAutoScroll.test.tssrc/components/editor/notion/tableAutoScroll.tssrc/components/editor/notion/tableClipboard.test.tssrc/components/editor/notion/tableClipboard.tssrc/components/editor/notion/tableEdgeDrag.test.tssrc/components/editor/notion/tableEdgeDrag.tssrc/components/editor/notion/tableExtensions.test.tssrc/components/editor/notion/tableExtensions.tssrc/components/editor/notion/tableIntegrity.test.tssrc/components/editor/notion/tableIntegrity.tssrc/components/editor/notion/tableMetadata.tssrc/components/editor/notion/tableNonRegression.test.tssrc/components/editor/notion/tablePerformance.test.tssrc/components/editor/notion/tablePointerDrag.test.tssrc/components/editor/notion/tablePointerDrag.tssrc/components/editor/notion/tableProximity.test.tssrc/components/editor/notion/tableProximity.tssrc/components/editor/notion/tableRowResize.test.tssrc/components/editor/notion/tableRowResize.tssrc/components/editor/notion/tableTransactions.test.tssrc/components/editor/notion/tableTransactions.tssrc/components/editor/notion/tableView.test.tssrc/components/editor/notion/tableView.tssrc/components/layout/Sidebar.tsxsrc/components/layout/SidebarControls.test.tsxsrc/components/layout/SidebarControls.tsxsrc/components/layout/SidebarFolderSection.test.tsxsrc/components/layout/SidebarFolderSection.tsxsrc/components/layout/WorkspaceMenu.test.tsxsrc/components/layout/WorkspaceMenu.tsxsrc/components/notes/FolderTreeView.test.tsxsrc/components/notes/FolderTreeView.tsxsrc/components/notes/NoteList.tsxsrc/components/preview/PreviewApp.tsxsrc/components/settings/EditorSettingsSection.test.tsxsrc/components/settings/EditorSettingsSection.tsxsrc/components/settings/SettingsPage.test.tsxsrc/components/settings/SettingsPage.tsxsrc/context/GitContext.tsxsrc/context/NotesContext.tsxsrc/context/ThemeContext.tsxsrc/lib/conflictResolution.test.tssrc/lib/conflictResolution.tssrc/lib/documentMutationSafety.test.tssrc/lib/documentMutationSafety.tssrc/lib/draftCheckpoint.test.tssrc/lib/draftCheckpoint.tssrc/lib/editorToolbar.test.tssrc/lib/editorToolbar.tssrc/lib/editorWidthResize.test.tssrc/lib/editorWidthResize.tssrc/lib/folderTree.test.tssrc/lib/folderTree.tssrc/lib/noteSync.test.tssrc/lib/noteSync.tssrc/lib/serializedWriter.test.tssrc/lib/serializedWriter.tssrc/lib/settingsScope.test.tssrc/lib/settingsScope.tssrc/lib/standaloneRecreation.test.tssrc/lib/standaloneRecreation.tssrc/lib/standaloneReload.test.tssrc/lib/standaloneReload.tssrc/lib/titleBarNoteInfo.test.tssrc/lib/titleBarNoteInfo.tssrc/lib/useWindowSessionPersistence.tssrc/lib/useWindowShortcuts.tssrc/lib/windowClose.test.tssrc/lib/windowClose.tssrc/lib/windowCloseCallsites.test.tssrc/lib/windowSession.test.tssrc/lib/windowSession.tssrc/lib/windowShortcutCallsites.test.tssrc/lib/windowShortcuts.test.tssrc/lib/windowShortcuts.tssrc/lib/workspace.test.tssrc/lib/workspace.tssrc/lib/workspaceSwitch.test.tssrc/lib/workspaceSwitch.tssrc/services/draftCheckpoint.test.tssrc/services/draftCheckpoint.tssrc/services/files.test.tssrc/services/files.tssrc/services/notes.test.tssrc/services/notes.tssrc/services/windowLifecycle.test.tssrc/services/windowLifecycle.tssrc/services/windowSession.test.tssrc/services/windowSession.tssrc/types/note.tsvitest.config.ts
Comment on lines
+8
to
+34
| export function getImageOpenTarget(source: string): ImageOpenTarget | null { | ||
| try { | ||
| const url = new URL(source); | ||
| const isTauriAsset = | ||
| url.protocol === "asset:" || url.hostname === "asset.localhost"; | ||
|
|
||
| if (isTauriAsset) { | ||
| const encodedPath = url.pathname.replace(/^\//, ""); | ||
| const filePath = decodeURIComponent(encodedPath); | ||
| if ( | ||
| filePath.startsWith("/") || | ||
| /^[a-zA-Z]:[\\/]/.test(filePath) | ||
| ) { | ||
| return { kind: "path", value: filePath }; | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| if (url.protocol === "http:" || url.protocol === "https:") { | ||
| return { kind: "url", value: url.toString() }; | ||
| } | ||
| } catch { | ||
| return null; | ||
| } | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find consumers of getImageOpenTarget and the native open commands they use.
set -euo pipefail
rg -n -C8 'getImageOpenTarget' src
rg -n -C4 'opener|shell_open|open_path|reveal_in|invoke\("open' src src-tauri/srcRepository: erictli/scratch
Length of output: 17345
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== imageInteractions.ts =="
cat -n src/components/editor/notion/imageInteractions.ts
echo
echo "== Editor image double-click handling =="
sed -n '3130,3185p' src/components/editor/Editor.tsx | cat -n
echo
echo "== src-tauri open-related commands =="
rg -n -C5 'open_file_preview|open_url_safe|open_file|openPath|open_path|open_in_file_manager|invoke_handler|#[tauri::command]' src-tauri/src | head -n 220
echo
echo "== Tauri plugin opener imports/usages =="
rg -n -C3 'plugin-opener|tauri_plugin_opener|opener|openPath|openUrl' src-tauri/src Cargo.toml pnpm-lock.yaml package.json | head -n 160Repository: erictli/scratch
Length of output: 10209
Restrict resolved image paths before opening them
getImageOpenTarget accepts any encoded absolute file path, and the editor context menu passes the path directly to openPath. Scope this to the workspace, configured assets, or app-scoped preview files; otherwise an edited asset: URL can open arbitrary local files. Limit the asset.localhost branch to http:/https: URLs as well.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/editor/notion/imageInteractions.ts` around lines 8 - 34,
Update getImageOpenTarget to validate resolved asset paths against the workspace
root, configured assets, or app-scoped preview-file directories before returning
a path target; reject all other absolute paths. Also restrict the
asset.localhost handling to approved http/https URL forms, preserving normal
external http/https URL handling while preventing edited asset URLs from
reaching openPath for arbitrary local files.
Norkep
added a commit
to Norkep/scratch
that referenced
this pull request
Aug 5, 2026
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/layout/Sidebar.tsx (1)
133-147: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRoute directory selection through a Tauri command.
open()calls the native dialog plugin directly from the frontend. Define the directory-selection operation insrc-tauri/src/lib.rs, then call it throughnotesServicewithinvoke().As per coding guidelines, “All backend operations must go through Tauri commands defined in
src-tauri/src/lib.rs. Frontend calls them viainvoke()from@tauri-apps/api/core.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Sidebar.tsx` around lines 133 - 147, Update handleAddWorkspace in Sidebar to stop calling the native open dialog directly; add a directory-selection Tauri command in the backend lib.rs and expose it through notesService, then invoke that service operation from the frontend using `@tauri-apps/api/core` invoke. Preserve single-directory selection and pass the returned path to handleSwitchWorkspace, retaining the existing error handling and toast behavior.Source: Coding guidelines
src/components/editor/notion/tableClipboard.ts (1)
49-76: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject text after a closing quoted field.
At Line 49,
parseTsv('"a"b\tc')returns[['ab', 'c']]. A closing quote must be followed by a tab, a line break, or end of input. Track the closing quote and returnnullif another ordinary character follows it. Add a regression test for this input.Proposed fix
let row: string[] = []; let cell = ""; let quoted = false; + let closedQuote = false; const pushCell = () => { row.push(cell); cell = ""; + closedQuote = false; }; if (character === '"' && (quoted || cell === "")) { if (quoted && text[index + 1] === '"') { cell += '"'; index += 1; } else { quoted = !quoted; + closedQuote = !quoted; } continue; } if (quoted) { // ... continue; } + if ( + closedQuote && + character !== "\t" && + character !== "\n" && + character !== "\r" + ) { + return null; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/editor/notion/tableClipboard.ts` around lines 49 - 76, Update the TSV parser around its quote-handling state to track when a quoted field has closed, and reject the input by returning null if an ordinary character follows before a tab, line break, or end of input. Preserve escaped quotes and valid delimiters, and add a regression test covering parseTsv('"a"b\tc').
🧹 Nitpick comments (7)
src/lib/workspaceSwitch.test.ts (1)
74-103: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winBlock the first switch before checking serialization.
Line 99 releases
firstLoadbefore/notes/areachesawait firstLoad. The test can only verify final completion order. Resolve afirstLoadStartedpromise immediately before the first load waits. Then assert that/notes/bhas not started before releasing the first load.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/workspaceSwitch.test.ts` around lines 74 - 103, Update the serialization test around createSerializedTaskQueue and runWorkspaceSwitch to track when the first load reaches its blocking await. Resolve a firstLoadStarted promise immediately before awaiting firstLoad, await that signal before releasing firstLoad, and assert loaded still excludes /notes/b; then release the first load and preserve the final ordered result assertion.src/context/NotesContext.tsx (1)
100-104: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the duplicated registration comment.
The same comment appears at Line 100 and Line 104 for two adjacent members. Move one comment above both declarations.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/context/NotesContext.tsx` around lines 100 - 104, Remove the duplicate “NotesProvider owns exactly one editor” comment around registerWorkspaceTransitionFlush and the adjacent declaration, leaving a single comment immediately above both members.src/lib/windowCloseCallsites.test.ts (1)
23-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the recovery assertion tolerant of formatting.
The test asserts exact source text, including
.then((result) => {and the template literal. A Prettier reflow or a reworded toast insrc/components/preview/PreviewApp.tsxbreaks this test without any behavior change.closeHandlers[1]also couples the test to array order. Use a named path constant and whitespace-tolerant patterns.♻️ Proposed refactor
-const closeHandlers = [ - resolve(process.cwd(), "src/App.tsx"), - resolve(process.cwd(), "src/components/preview/PreviewApp.tsx"), -]; +const appHandler = resolve(process.cwd(), "src/App.tsx"); +const previewHandler = resolve( + process.cwd(), + "src/components/preview/PreviewApp.tsx", +); +const closeHandlers = [appHandler, previewHandler]; @@ it("reports standalone recovery paths before the preview window closes", () => { - const source = readFileSync(closeHandlers[1], "utf8"); + const source = readFileSync(previewHandler, "utf8"); - expect(source).toContain(".then((result) => {"); - expect(source).toContain("result.recoveredTo"); - expect(source).toContain("Draft recovered to ${result.recoveredTo}"); + expect(source).toMatch(/\.then\(\s*\(\s*result\s*\)\s*=>/); + expect(source).toMatch(/result\.recoveredTo/); + expect(source).toMatch(/recovered to \$\{result\.recoveredTo\}/i); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/windowCloseCallsites.test.ts` around lines 23 - 29, Update the recovery-path test around closeHandlers[1] to use a named path constant instead of relying on array position, and replace exact source-string assertions with whitespace-tolerant patterns that still verify result.recoveredTo and the recovery message in PreviewApp.tsx.src-tauri/src/editor_image_open.rs (1)
33-45: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueConsider syncing the assets directory after the copy.
copy_image_to_assets_create_onlycallstarget.sync_all()for the file, but it does not sync the parent directory entry.src-tauri/src/persistence.rssyncs the parent directory after publication. If the process crashes right after the copy, the new asset name can be missing while the Markdown link already points to it. Reuse the same parent-directory sync helper for parity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/editor_image_open.rs` around lines 33 - 45, Update copy_image_to_assets_create_only after the successful target.sync_all() to sync the parent assets directory using the existing parent-directory sync helper from persistence.rs. Preserve the current cleanup and error handling, and only return the new target path after both file and directory synchronization succeed.src/components/editor/notion/TableControls.tsx (1)
88-109: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCompare
rowElementsidentity inlayoutsEquivalent.
layoutsEquivalentcomparestablePosand rectangles only. When ProseMirror re-renders the table with identical geometry, the new layout is equivalent, soevaluatePointerat Line 605 keeps the previous layout.layoutRef.currentthen holds detachedtrelements. The observer effect at Line 754 observes those detached rows, and the keyboard focus effect reads the same stale layout.resizeRowremeasures, so the resize path is unaffected.Add an identity comparison for
rowElements.♻️ Proposed comparison
if ( current.rowRects.length !== next.rowRects.length || current.columnRects.length !== next.columnRects.length || + current.rowElements.length !== next.rowElements.length || !rectsEquivalent(current.tableRect, next.tableRect) ) { return false; } return ( + current.rowElements.every( + (element, index) => element === next.rowElements[index], + ) && current.rowRects.every((rect, index) => rectsEquivalent(rect, next.rowRects[index]), ) &&🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/editor/notion/TableControls.tsx` around lines 88 - 109, Update layoutsEquivalent to compare current.rowElements and next.rowElements by identity, returning false when their lengths differ or any corresponding row element is not the same object. Keep the existing table position and rectangle comparisons unchanged so identical geometry only remains equivalent when the rendered row elements are also reused.src/lib/draftCheckpoint.ts (1)
97-111: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBound the retry count or back off between retries.
The retry delay is bounded, but the retry count is not. When
storage.writekeeps failing, the catch branch restorespending, resetsfirstDirtyAt, and callsschedule()again. With the editor configuration ofdelayMs: 250, this repeats a failed write every 250 ms indefinitely, andEditor.tsxwrites aconsole.errorline for each attempt.Add an attempt counter and increase the delay after consecutive failures, or stop retrying after a fixed number of attempts and keep
pendingfor the nextmarkDirtyorflush.♻️ Sketch of a bounded retry
let queuedWrites = 0; + let consecutiveFailures = 0; let disposed = false; @@ try { await storage.write(checkpoint); + consecutiveFailures = 0; if (!pending) firstDirtyAt = undefined; } catch (error) { + consecutiveFailures += 1; if (!pending) { pending = checkpoint; firstDirtyAt = now(); } - if (!disposed) schedule(); + if (!disposed && consecutiveFailures <= MAX_CHECKPOINT_RETRIES) { + schedule(); + } throw error;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/draftCheckpoint.ts` around lines 97 - 111, Bound retries in the write flow around storage.write: track consecutive failed attempts and either apply increasing backoff or stop scheduling after a fixed limit. Preserve pending so the next markDirty or flush can retry, reset the attempt state after a successful write, and avoid the current indefinite schedule() loop in the catch branch.src-tauri/src/sha256.rs (1)
110-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd SHA-256 vectors for the second padding block and multi-chunk compression.
The current test only covers padding input that fits in one block, leaving the
remainder.len() >= 56path and the main chunk loop untested. Add a 56-byte input and an input longer than 64 bytes so regressions in those paths cannot change digest results without failing the tests.💚 Proposed additional vectors
assert_eq!( hex_digest(b"abc"), "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad", ); + // 56 bytes forces the second padding block. + assert_eq!( + hex_digest(&[b'a'; 56]), + "b35439a4ac6f0948b6d6f9e3c6af0f5f590ce20f1bde7090ef7970686ec6738a", + ); + // Multi-chunk input exercises the compression loop. + assert_eq!( + hex_digest(b"abcdbcdecdefdefgefghfghighijhijkijkljklmklmnlmnomnopnopq"), + "248d6a61d20638b8e5c026930c3e6039a33ce45964ff2167f6ecedd419db06c1", + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/src/sha256.rs` around lines 110 - 121, Extend matches_standard_lowercase_sha256_vectors with assertions for a 56-byte input and a separate input longer than 64 bytes, using their known standard SHA-256 digests. Keep the existing empty and abc vectors unchanged, ensuring the new cases exercise the second padding block and multi-chunk compression loop.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/editor/imageDrop.ts`:
- Around line 66-104: Update the image-drop failure catch block after
scaleFactorController.enter() to call scaleFactorController.reset() after
logging and showing the error toast. Also invoke scaleFactorController.reset()
in the disposal or cleanup path so failed or ended drags clear the cached lookup
and later drags re-read the window scale factor.
In `@src/components/layout/Sidebar.tsx`:
- Around line 122-127: Update refreshWorkspaces to return a failure result when
listWorkspaces fails instead of resolving as success, then inspect that result
in the workspace-switch flow alongside reloadSettings so the existing
post-switch error log and toast are shown when workspace refresh fails.
In `@src/context/NotesContext.tsx`:
- Around line 727-736: Post-mutation selection reload failures must not prevent
notes refresh. In renameFolderAction at src/context/NotesContext.tsx lines
727-736, set selectedNoteIdRef.current to newId before independently catching
readNote(newId) failures; apply the same isolated readNote error handling in
moveNoteAction at lines 758-765 and moveFolderAction at lines 795-804, ensuring
each handler always reaches refreshNotes() and preserves the completed mutation
state.
---
Outside diff comments:
In `@src/components/editor/notion/tableClipboard.ts`:
- Around line 49-76: Update the TSV parser around its quote-handling state to
track when a quoted field has closed, and reject the input by returning null if
an ordinary character follows before a tab, line break, or end of input.
Preserve escaped quotes and valid delimiters, and add a regression test covering
parseTsv('"a"b\tc').
In `@src/components/layout/Sidebar.tsx`:
- Around line 133-147: Update handleAddWorkspace in Sidebar to stop calling the
native open dialog directly; add a directory-selection Tauri command in the
backend lib.rs and expose it through notesService, then invoke that service
operation from the frontend using `@tauri-apps/api/core` invoke. Preserve
single-directory selection and pass the returned path to handleSwitchWorkspace,
retaining the existing error handling and toast behavior.
---
Nitpick comments:
In `@src-tauri/src/editor_image_open.rs`:
- Around line 33-45: Update copy_image_to_assets_create_only after the
successful target.sync_all() to sync the parent assets directory using the
existing parent-directory sync helper from persistence.rs. Preserve the current
cleanup and error handling, and only return the new target path after both file
and directory synchronization succeed.
In `@src-tauri/src/sha256.rs`:
- Around line 110-121: Extend matches_standard_lowercase_sha256_vectors with
assertions for a 56-byte input and a separate input longer than 64 bytes, using
their known standard SHA-256 digests. Keep the existing empty and abc vectors
unchanged, ensuring the new cases exercise the second padding block and
multi-chunk compression loop.
In `@src/components/editor/notion/TableControls.tsx`:
- Around line 88-109: Update layoutsEquivalent to compare current.rowElements
and next.rowElements by identity, returning false when their lengths differ or
any corresponding row element is not the same object. Keep the existing table
position and rectangle comparisons unchanged so identical geometry only remains
equivalent when the rendered row elements are also reused.
In `@src/context/NotesContext.tsx`:
- Around line 100-104: Remove the duplicate “NotesProvider owns exactly one
editor” comment around registerWorkspaceTransitionFlush and the adjacent
declaration, leaving a single comment immediately above both members.
In `@src/lib/draftCheckpoint.ts`:
- Around line 97-111: Bound retries in the write flow around storage.write:
track consecutive failed attempts and either apply increasing backoff or stop
scheduling after a fixed limit. Preserve pending so the next markDirty or flush
can retry, reset the attempt state after a successful write, and avoid the
current indefinite schedule() loop in the catch branch.
In `@src/lib/windowCloseCallsites.test.ts`:
- Around line 23-29: Update the recovery-path test around closeHandlers[1] to
use a named path constant instead of relying on array position, and replace
exact source-string assertions with whitespace-tolerant patterns that still
verify result.recoveredTo and the recovery message in PreviewApp.tsx.
In `@src/lib/workspaceSwitch.test.ts`:
- Around line 74-103: Update the serialization test around
createSerializedTaskQueue and runWorkspaceSwitch to track when the first load
reaches its blocking await. Resolve a firstLoadStarted promise immediately
before awaiting firstLoad, await that signal before releasing firstLoad, and
assert loaded still excludes /notes/b; then release the first load and preserve
the final ordered result assertion.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b628b5a8-b586-41bf-946e-a745a871cdac
📒 Files selected for processing (78)
src-tauri/capabilities/default.jsonsrc-tauri/src/draft_checkpoint.rssrc-tauri/src/editor_image_open.rssrc-tauri/src/git.rssrc-tauri/src/lib.rssrc-tauri/src/note_persistence_tests.rssrc-tauri/src/persistence.rssrc-tauri/src/sha256.rssrc/App.csssrc/App.tsxsrc/components/editor/Editor.tsxsrc/components/editor/editorHistory.test.tssrc/components/editor/editorHistory.tssrc/components/editor/imageDrop.test.tssrc/components/editor/imageDrop.tssrc/components/editor/notion/TableControls.test.tsxsrc/components/editor/notion/TableControls.tsxsrc/components/editor/notion/imageInteractions.test.tssrc/components/editor/notion/imageInteractions.tssrc/components/editor/notion/interfaceGeometry.test.tssrc/components/editor/notion/interfaceGeometry.tssrc/components/editor/notion/markdownDocument.test.tssrc/components/editor/notion/markdownDocument.tssrc/components/editor/notion/selectionDecoration.test.tssrc/components/editor/notion/tableClipboard.test.tssrc/components/editor/notion/tableClipboard.tssrc/components/editor/notion/tableEdgeDrag.test.tssrc/components/editor/notion/tableEdgeDrag.tssrc/components/editor/notion/tableExtensions.test.tssrc/components/editor/notion/tableExtensions.tssrc/components/editor/notion/tableIntegrity.test.tssrc/components/editor/notion/tableIntegrity.tssrc/components/editor/notion/tablePerformance.test.tssrc/components/editor/notion/tableProximity.tssrc/components/editor/notion/tableRowResize.test.tssrc/components/editor/notion/tableRowResize.tssrc/components/editor/notion/tableTransactions.test.tssrc/components/editor/notion/tableTransactions.tssrc/components/editor/notion/tableView.test.tssrc/components/editor/notion/tableView.tssrc/components/layout/Sidebar.error.test.tssrc/components/layout/Sidebar.tsxsrc/components/notes/NoteList.tsxsrc/components/preview/PreviewApp.tsxsrc/components/settings/SettingsPage.test.tsxsrc/context/NotesContext.tsxsrc/context/ThemeContext.test.tsxsrc/context/ThemeContext.tsxsrc/lib/conflictResolution.test.tssrc/lib/draftCheckpoint.test.tssrc/lib/draftCheckpoint.tssrc/lib/editorCheckpointCleanup.test.tssrc/lib/editorCheckpointCleanup.tssrc/lib/noteSync.test.tssrc/lib/noteSync.tssrc/lib/standaloneRecreation.test.tssrc/lib/standaloneRecreation.tssrc/lib/standaloneReload.test.tssrc/lib/standaloneReload.tssrc/lib/useWindowSessionPersistence.test.tssrc/lib/useWindowSessionPersistence.tssrc/lib/useWindowShortcuts.test.tsxsrc/lib/useWindowShortcuts.tssrc/lib/windowClose.test.tssrc/lib/windowClose.tssrc/lib/windowCloseCallsites.test.tssrc/lib/windowSession.test.tssrc/lib/windowSession.tssrc/lib/windowShortcutCallsites.test.tssrc/lib/windowShortcuts.test.tssrc/lib/workspaceSwitch.test.tssrc/services/draftCheckpoint.test.tssrc/services/draftCheckpoint.tssrc/services/files.test.tssrc/services/notes.test.tssrc/services/notes.tssrc/services/windowLifecycle.test.tssrc/services/windowLifecycle.ts
💤 Files with no reviewable changes (1)
- src-tauri/capabilities/default.json
🚧 Files skipped from review as they are similar to previous changes (31)
- src/lib/conflictResolution.test.ts
- src/components/editor/notion/interfaceGeometry.test.ts
- src/components/editor/notion/interfaceGeometry.ts
- src/lib/standaloneRecreation.test.ts
- src/components/editor/notion/tableProximity.ts
- src/components/editor/notion/tablePerformance.test.ts
- src/lib/windowClose.ts
- src/services/windowLifecycle.test.ts
- src/services/draftCheckpoint.ts
- src/lib/useWindowShortcuts.ts
- src/components/editor/notion/tableExtensions.ts
- src-tauri/src/draft_checkpoint.rs
- src/components/editor/notion/tableTransactions.ts
- src/components/editor/notion/tableExtensions.test.ts
- src/services/windowLifecycle.ts
- src/services/notes.ts
- src/components/editor/notion/markdownDocument.test.ts
- src/App.css
- src/services/draftCheckpoint.test.ts
- src/lib/windowSession.ts
- src/components/preview/PreviewApp.tsx
- src/components/notes/NoteList.tsx
- src/App.tsx
- src/lib/windowShortcutCallsites.test.ts
- src/components/editor/notion/markdownDocument.ts
- src/lib/windowClose.test.ts
- src/context/ThemeContext.tsx
- src/components/editor/Editor.tsx
- src/components/editor/notion/tableEdgeDrag.test.ts
- src-tauri/src/lib.rs
- src/components/editor/notion/selectionDecoration.test.ts
Comment on lines
+66
to
+104
| export function createImageDragScaleFactorController( | ||
| scaleFactorRef: ScaleFactorRef, | ||
| loadScaleFactor: () => Promise<number>, | ||
| ) { | ||
| let generation = 0; | ||
| let pendingLookup: Promise<number | null> | null = null; | ||
|
|
||
| return { | ||
| enter(): Promise<number | null> { | ||
| if (pendingLookup) return pendingLookup; | ||
| const lookupGeneration = generation; | ||
| let loadedScaleFactor: Promise<number>; | ||
| try { | ||
| loadedScaleFactor = loadScaleFactor(); | ||
| } catch { | ||
| loadedScaleFactor = Promise.resolve(1); | ||
| } | ||
| pendingLookup = loadedScaleFactor | ||
| .then(normalizeScaleFactor) | ||
| .catch(() => 1) | ||
| .then((scaleFactor) => { | ||
| if (generation !== lookupGeneration) { | ||
| return null; | ||
| } | ||
| scaleFactorRef.current = scaleFactor; | ||
| return scaleFactor; | ||
| }); | ||
| return pendingLookup; | ||
| }, | ||
| current(): number { | ||
| return normalizeScaleFactor(scaleFactorRef.current); | ||
| }, | ||
| reset(): void { | ||
| generation += 1; | ||
| pendingLookup = null; | ||
| scaleFactorRef.current = 1; | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect image-drag controller lifecycle calls in Editor.tsx.
rg -n -C 6 'createImageDragScaleFactorController|imageDragScaleFactor|\.enter\(\)|\.reset\(\)|drag-leave|onDragDropEvent|imageDragActiveRef' src/components/editor/Editor.tsxRepository: erictli/scratch
Length of output: 4100
🏁 Script executed:
#!/bin/bash
# Inspect the drag/drop handling and error paths around the scaleFactorController usage.
sed -n '1670,1850p' src/components/editor/Editor.tsx | cat -nRepository: erictli/scratch
Length of output: 7652
Reset scaleFactorController after failed image drops.
The catch block after scaleFactorController.enter() only logs and shows an error toast, so the drag can remain active with the same cached scale factor. Add scaleFactorController.reset() there, and reset on disposal, so later drags re-read the window scale factor.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/editor/imageDrop.ts` around lines 66 - 104, Update the
image-drop failure catch block after scaleFactorController.enter() to call
scaleFactorController.reset() after logging and showing the error toast. Also
invoke scaleFactorController.reset() in the disposal or cleanup path so failed
or ended drags clear the cached lookup and later drags re-read the window scale
factor.
Comment on lines
+122
to
+127
| try { | ||
| await reloadSettings(); | ||
| await refreshWorkspaces(); | ||
| } catch (error) { | ||
| console.error("Failed to refresh after workspace switch:", error); | ||
| toast.error("Workspace switched, but the sidebar could not refresh"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Report a workspace-list refresh failure.
refreshWorkspaces() catches listWorkspaces() errors and resolves. Therefore, the catch block on Lines 125-127 does not run when only the workspace-list refresh fails. Return a failure result from refreshWorkspaces() and show the post-switch refresh error when that result indicates failure.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/layout/Sidebar.tsx` around lines 122 - 127, Update
refreshWorkspaces to return a failure result when listWorkspaces fails instead
of resolving as success, then inspect that result in the workspace-switch flow
alongside reloadSettings so the existing post-switch error log and toast are
shown when workspace refresh fails.
Comment on lines
+727
to
+736
| // Update selectedNoteId if it was inside the renamed folder. | ||
| const selectedId = selectedNoteIdRef.current; | ||
| if (selectedId?.startsWith(oldPrefix)) { | ||
| const newId = newPrefix + selectedId.substring(oldPrefix.length); | ||
| const note = await notesService.readNote(newId); | ||
| selectedNoteIdRef.current = newId; | ||
| currentNoteRef.current = note; | ||
| setSelectedNoteId(newId); | ||
| setCurrentNote(note); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
A failed post-mutation read aborts refreshNotes() after the mutation already succeeded. In all three handlers the selection reload runs inside the same try block as refreshNotes(). If notesService.readNote rejects, the notes list keeps stale ids, the selection keeps the old id, and the user sees a failure message for a completed rename or move. Isolate the selection reload with its own error handling in each handler.
src/context/NotesContext.tsx#L727-L736: inrenameFolderAction, set the new selected id first, then wrapreadNote(newId)in its own try/catch sorefreshNotes()at Line 738 always runs.src/context/NotesContext.tsx#L758-L765: inmoveNoteAction, wrapreadNote(newId)in its own try/catch sorefreshNotes()at Line 766 always runs.src/context/NotesContext.tsx#L795-L804: inmoveFolderAction, wrapreadNote(newId)in its own try/catch sorefreshNotes()at Line 806 always runs.
📍 Affects 1 file
src/context/NotesContext.tsx#L727-L736(this comment)src/context/NotesContext.tsx#L758-L765src/context/NotesContext.tsx#L795-L804
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/context/NotesContext.tsx` around lines 727 - 736, Post-mutation selection
reload failures must not prevent notes refresh. In renameFolderAction at
src/context/NotesContext.tsx lines 727-736, set selectedNoteIdRef.current to
newId before independently catching readNote(newId) failures; apply the same
isolated readNote error handling in moveNoteAction at lines 758-765 and
moveFolderAction at lines 795-804, ensuring each handler always reaches
refreshNotes() and preserves the completed mutation state.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Existing image pipeline preserved
This PR reuses the current pipeline without backend changes:
copy_image_to_assets->get_notes_folder->convertFileSrc-> TipTap image nodeThe frontend allowlist exactly matches the Rust backend:
jpg,jpeg,png,gif,webp,svg,bmp,tiff,tif,ico,avifScope
This is PR 6 of the Scratch 1.0.1 compatibility series.
It intentionally excludes editor block drag and drop,
BlockDragControls,blockDrag,editorBehavior, and the TipTap drag-handle extension. Those remain isolated for PR7.Dependencies
This branch is directly stacked on #201, which is itself stacked on the earlier compatibility PRs #197, #198, #199, and #200.
The PR targets
main; its diff will shrink as dependencies merge.Validation
imageDrop.ts,imageDrop.test.ts,imageInteractions.ts, andimageInteractions.test.tsmatch the 1.0.1 donor byte-for-byte-D warningspassedgit diff --checkpassedPackaged macOS validation used the isolated
com.scratch.pr6.validationbundle and a workspace under/private/tmp:assetsfolder-1suffixSummary by CodeRabbit