feat: add local image insertion with file picker, toolbar button, and hardened safety checks (#291)

- Add local image insertion support with file picker and toolbar button
- Harden image copy with COPYFILE_EXCL flag to prevent overwrites
- Add deconfliction loop limit (1000 iterations) to prevent hangs
- Force image re-render on filePath change for correct URL resolution
- Extract toolbar button into reusable RichMarkdownToolbarButton component
- Use setHeading (not toggleHeading) for idempotent slash commands
- Add path utilities (dirname, joinPath) with comprehensive tests
- Add image utility functions for safe copy destination handling
This commit is contained in:
Jinjing 2026-04-04 00:31:03 -07:00 committed by GitHub
parent 943ad1f3f1
commit e053b4f606
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 296 additions and 65 deletions

BIN
orca_3d.jpg Normal file

Binary file not shown.

After

Width:  |  Height:  |  Size: 311 KiB

View file

@ -1,5 +1,5 @@
import { ipcMain, shell } from 'electron'
import { stat } from 'node:fs/promises'
import { ipcMain, shell, dialog } from 'electron'
import { constants, copyFile, stat } from 'node:fs/promises'
import { isAbsolute, normalize } from 'node:path'
import { fileURLToPath } from 'node:url'
@ -81,4 +81,38 @@ export function registerShellHandlers(): void {
ipcMain.handle('shell:pathExists', async (_event, filePath: string): Promise<boolean> => {
return pathExists(filePath)
})
// Why: window.prompt() and <input type="file"> are unreliable in Electron,
// so we use the native OS dialog to let the user pick an image file.
ipcMain.handle('shell:pickImage', async (): Promise<string | null> => {
const result = await dialog.showOpenDialog({
properties: ['openFile'],
filters: [
{ name: 'Images', extensions: ['png', 'jpg', 'jpeg', 'gif', 'webp', 'svg', 'bmp', 'ico'] }
]
})
if (result.canceled || result.filePaths.length === 0) {
return null
}
return result.filePaths[0]
})
// Why: copying a picked image next to the markdown file lets us insert a
// relative path (e.g. `![](image.png)`) instead of embedding base64,
// keeping markdown files small and portable.
ipcMain.handle(
'shell:copyFile',
async (_event, args: { srcPath: string; destPath: string }): Promise<void> => {
const src = normalize(args.srcPath)
const dest = normalize(args.destPath)
if (!isAbsolute(src) || !isAbsolute(dest)) {
throw new Error('Both source and destination must be absolute paths')
}
// Why: COPYFILE_EXCL prevents silently overwriting an existing file.
// The renderer-side deconfliction loop already picks a unique name, so
// the dest should never exist — if it does, something is wrong and we
// should fail loudly rather than clobber data.
await copyFile(src, dest, constants.COPYFILE_EXCL)
}
)
}

View file

@ -98,6 +98,8 @@ type ShellApi = {
openFilePath: (path: string) => Promise<void>
openFileUri: (uri: string) => Promise<void>
pathExists: (path: string) => Promise<boolean>
pickImage: () => Promise<string | null>
copyFile: (args: { srcPath: string; destPath: string }) => Promise<void>
}
type HooksApi = {

View file

@ -222,7 +222,12 @@ const api = {
openFileUri: (uri: string): Promise<void> => ipcRenderer.invoke('shell:openFileUri', uri),
pathExists: (path: string): Promise<boolean> => ipcRenderer.invoke('shell:pathExists', path)
pathExists: (path: string): Promise<boolean> => ipcRenderer.invoke('shell:pathExists', path),
pickImage: (): Promise<string | null> => ipcRenderer.invoke('shell:pickImage'),
copyFile: (args: { srcPath: string; destPath: string }): Promise<void> =>
ipcRenderer.invoke('shell:copyFile', args)
},
hooks: {

View file

@ -636,7 +636,14 @@
}
.rich-markdown-editor ul[data-type='taskList'] li > label {
margin-top: 0.35em;
/* Vertically center the checkbox with the first line of text. */
flex-shrink: 0;
margin-top: 0.3em;
}
/* Remove paragraph margin inside task items so the text sits flush with the checkbox. */
.rich-markdown-editor ul[data-type='taskList'] li > div > p {
margin: 0;
}
.rich-markdown-editor blockquote {

View file

@ -105,6 +105,7 @@ export function EditorContent({
return (
<RichMarkdownEditor
content={currentContent}
filePath={activeFile.filePath}
onContentChange={handleContentChange}
onSave={handleSave}
/>

View file

@ -1,9 +1,12 @@
import React, { useEffect, useMemo, useRef, useState } from 'react'
import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'
import { EditorContent, useEditor } from '@tiptap/react'
import type { Editor } from '@tiptap/react'
import { List, ListOrdered, Quote } from 'lucide-react'
import { ImageIcon, List, ListOrdered, Quote } from 'lucide-react'
import { toast } from 'sonner'
import { cn } from '@/lib/utils'
import { RichMarkdownToolbarButton } from './RichMarkdownToolbarButton'
import { isMarkdownPreviewFindShortcut } from './markdown-preview-search'
import { extractIpcErrorMessage, getImageCopyDestination } from './rich-markdown-image-utils'
import { encodeRawMarkdownHtmlForRichEditor } from './raw-markdown-html'
import { createRichMarkdownExtensions } from './rich-markdown-extensions'
import { slashCommands } from './rich-markdown-commands'
@ -13,6 +16,7 @@ import { useRichMarkdownSearch } from './useRichMarkdownSearch'
type RichMarkdownEditorProps = {
content: string
filePath: string
onContentChange: (content: string) => void
onSave: (content: string) => void
}
@ -31,6 +35,7 @@ const richMarkdownExtensions = createRichMarkdownExtensions({
export default function RichMarkdownEditor({
content,
filePath,
onContentChange,
onSave
}: RichMarkdownEditorProps): React.JSX.Element {
@ -44,11 +49,14 @@ export default function RichMarkdownEditor({
const selectedCommandIndexRef = useRef(0)
const onContentChangeRef = useRef(onContentChange)
const onSaveRef = useRef(onSave)
const handleLocalImagePickRef = useRef<() => void>(() => {})
// Why: ProseMirror keeps the initial handleKeyDown closure, so `editor` stays
// stuck at the first-render null value unless we read the live instance here.
const editorRef = useRef<Editor | null>(null)
useEffect(() => {
onContentChangeRef.current = onContentChange
}, [onContentChange])
useEffect(() => {
onSaveRef.current = onSave
}, [onSave])
@ -71,7 +79,7 @@ export default function RichMarkdownEditor({
}
if (mod && event.key.toLowerCase() === 's') {
event.preventDefault()
const markdown = editor?.getMarkdown() ?? lastCommittedMarkdownRef.current
const markdown = editorRef.current?.getMarkdown() ?? lastCommittedMarkdownRef.current
onSaveRef.current(markdown)
return true
}
@ -86,7 +94,9 @@ export default function RichMarkdownEditor({
return false
}
const activeEditor = editor
// Why: handleKeyDown is frozen from the first render, so this closure
// must read editorRef to get the live editor instance.
const activeEditor = editorRef.current
if (!activeEditor) {
return false
}
@ -109,13 +119,13 @@ export default function RichMarkdownEditor({
}
if (event.key === 'Enter' || event.key === 'Tab') {
event.preventDefault()
// Why: ProseMirror keeps this key handler stable for the editor's
// lifetime, so reading React state directly here can use a stale
// slash-menu index and execute the wrong command after keyboard
// navigation. The ref mirrors the latest highlighted item.
// Why: this key handler is stable for the editor lifetime, so the ref
// mirrors the latest highlighted slash-menu item for keyboard picks.
const selectedCommand = currentFilteredSlashCommands[selectedCommandIndexRef.current]
if (selectedCommand) {
runSlashCommand(activeEditor, currentSlashMenu, selectedCommand)
runSlashCommand(activeEditor, currentSlashMenu, selectedCommand, () =>
handleLocalImagePickRef.current()
)
}
return true
}
@ -142,6 +152,45 @@ export default function RichMarkdownEditor({
}
})
useEffect(() => {
editorRef.current = editor ?? null
}, [editor])
// Why: the custom Image extension reads filePath from editor.storage to resolve
// relative image src values to file:// URLs for display. After updating the
// stored path we dispatch a no-op transaction so ProseMirror re-renders image
// nodes with the new resolved src (renderHTML reads storage at render time).
useEffect(() => {
if (editor) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
;(editor.storage as any).image.filePath = filePath
editor.view.dispatch(editor.state.tr)
}
}, [editor, filePath])
const handleLocalImagePick = useCallback(async () => {
if (!editor) {
return
}
try {
const srcPath = await window.api.shell.pickImage()
if (!srcPath) {
return
}
// Why: copy the image next to the markdown file and insert a relative path
// so the markdown stays portable and doesn't bloat with base64 data.
const { imageName, destPath } = await getImageCopyDestination(filePath, srcPath)
if (srcPath !== destPath) {
await window.api.shell.copyFile({ srcPath, destPath })
}
editor.chain().focus().setImage({ src: imageName }).run()
} catch (err) {
toast.error(extractIpcErrorMessage(err, 'Failed to insert image.'))
}
}, [editor, filePath])
useEffect(() => {
handleLocalImagePickRef.current = handleLocalImagePick
}, [handleLocalImagePick])
const {
activeMatchIndex,
closeSearch,
@ -180,7 +229,6 @@ export default function RichMarkdownEditor({
useEffect(() => {
selectedCommandIndexRef.current = selectedCommandIndex
}, [selectedCommandIndex])
useEffect(() => {
if (filteredSlashCommands.length === 0) {
setSelectedCommandIndex(0)
@ -215,48 +263,51 @@ export default function RichMarkdownEditor({
return (
<div ref={rootRef} className="rich-markdown-editor-shell">
<div className="rich-markdown-editor-toolbar">
<ToolbarButton
<RichMarkdownToolbarButton
active={editor?.isActive('bold') ?? false}
label="Bold"
onClick={() => editor?.chain().focus().toggleBold().run()}
>
B
</ToolbarButton>
<ToolbarButton
</RichMarkdownToolbarButton>
<RichMarkdownToolbarButton
active={editor?.isActive('italic') ?? false}
label="Italic"
onClick={() => editor?.chain().focus().toggleItalic().run()}
>
I
</ToolbarButton>
<ToolbarButton
</RichMarkdownToolbarButton>
<RichMarkdownToolbarButton
active={editor?.isActive('strike') ?? false}
label="Strike"
onClick={() => editor?.chain().focus().toggleStrike().run()}
>
S
</ToolbarButton>
<ToolbarButton
</RichMarkdownToolbarButton>
<RichMarkdownToolbarButton
active={editor?.isActive('bulletList') ?? false}
label="Bullet list"
onClick={() => editor?.chain().focus().toggleBulletList().run()}
>
<List className="size-3.5" />
</ToolbarButton>
<ToolbarButton
</RichMarkdownToolbarButton>
<RichMarkdownToolbarButton
active={editor?.isActive('orderedList') ?? false}
label="Numbered list"
onClick={() => editor?.chain().focus().toggleOrderedList().run()}
>
<ListOrdered className="size-3.5" />
</ToolbarButton>
<ToolbarButton
</RichMarkdownToolbarButton>
<RichMarkdownToolbarButton
active={editor?.isActive('blockquote') ?? false}
label="Quote"
onClick={() => editor?.chain().focus().toggleBlockquote().run()}
>
<Quote className="size-3.5" />
</ToolbarButton>
</RichMarkdownToolbarButton>
<RichMarkdownToolbarButton active={false} label="Image" onClick={handleLocalImagePick}>
<ImageIcon className="size-3.5" />
</RichMarkdownToolbarButton>
</div>
<RichMarkdownSearchBar
activeMatchIndex={activeMatchIndex}
@ -287,7 +338,9 @@ export default function RichMarkdownEditor({
index === selectedCommandIndex && 'is-active'
)}
onMouseDown={(event) => event.preventDefault()}
onClick={() => editor && runSlashCommand(editor, slashMenu, command)}
onClick={() =>
editor && runSlashCommand(editor, slashMenu, command, handleLocalImagePick)
}
>
<span className="rich-markdown-slash-icon">
<Icon className="size-3.5" />
@ -307,31 +360,6 @@ export default function RichMarkdownEditor({
)
}
function ToolbarButton({
active,
label,
onClick,
children
}: {
active: boolean
label: string
onClick: () => void
children: React.ReactNode
}): React.JSX.Element {
return (
<button
type="button"
className={cn('rich-markdown-toolbar-button', active && 'is-active')}
aria-label={label}
title={label}
onMouseDown={(event) => event.preventDefault()}
onClick={onClick}
>
{children}
</button>
)
}
function syncSlashMenu(
editor: Editor,
root: HTMLDivElement | null,
@ -376,7 +404,18 @@ function syncSlashMenu(
})
}
function runSlashCommand(editor: Editor, slashMenu: SlashMenuState, command: SlashCommand): void {
function runSlashCommand(
editor: Editor,
slashMenu: SlashMenuState,
command: SlashCommand,
onImageCommand?: () => void
): void {
editor.chain().focus().deleteRange({ from: slashMenu.from, to: slashMenu.to }).run()
// Why: image insertion cannot rely on window.prompt() in Electron, so this
// command is rerouted into the editor's local image picker flow.
if (command.id === 'image' && onImageCommand) {
onImageCommand()
return
}
command.run(editor)
}

View file

@ -0,0 +1,38 @@
import React from 'react'
import { cn } from '@/lib/utils'
import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '@/components/ui/tooltip'
type RichMarkdownToolbarButtonProps = {
active: boolean
label: string
onClick: () => void
children: React.ReactNode
}
export function RichMarkdownToolbarButton({
active,
label,
onClick,
children
}: RichMarkdownToolbarButtonProps): React.JSX.Element {
return (
<TooltipProvider>
<Tooltip>
<TooltipTrigger asChild>
<button
type="button"
className={cn('rich-markdown-toolbar-button', active && 'is-active')}
aria-label={label}
onMouseDown={(event) => event.preventDefault()}
onClick={onClick}
>
{children}
</button>
</TooltipTrigger>
<TooltipContent side="bottom" sideOffset={4}>
{label}
</TooltipContent>
</Tooltip>
</TooltipProvider>
)
}

View file

@ -42,7 +42,9 @@ export const slashCommands: SlashCommand[] = [
icon: Heading1,
description: 'Large section heading.',
run: (editor) => {
editor.chain().focus().toggleHeading({ level: 1 }).run()
// Use setHeading (not toggleHeading) so the slash command is idempotent —
// invoking "/h1" on an existing H1 should keep it as H1, not revert to paragraph.
editor.chain().focus().setHeading({ level: 1 }).run()
}
},
{
@ -52,7 +54,9 @@ export const slashCommands: SlashCommand[] = [
icon: Heading2,
description: 'Medium section heading.',
run: (editor) => {
editor.chain().focus().toggleHeading({ level: 2 }).run()
// Use setHeading (not toggleHeading) so the slash command is idempotent —
// invoking "/h2" on an existing H2 should keep it as H2, not revert to paragraph.
editor.chain().focus().setHeading({ level: 2 }).run()
}
},
{
@ -62,7 +66,9 @@ export const slashCommands: SlashCommand[] = [
icon: Heading3,
description: 'Small section heading.',
run: (editor) => {
editor.chain().focus().toggleHeading({ level: 3 }).run()
// Use setHeading (not toggleHeading) so the slash command is idempotent —
// invoking "/h3" on an existing H3 should keep it as H3, not revert to paragraph.
editor.chain().focus().setHeading({ level: 3 }).run()
}
},
{
@ -130,13 +136,11 @@ export const slashCommands: SlashCommand[] = [
label: 'Image',
aliases: ['image', 'img'],
icon: ImageIcon,
description: 'Insert an image from a URL.',
description: 'Insert an image from your computer.',
// Why: window.prompt() is not supported in Electron's renderer process,
// so image URL input is handled by an inline input bar in RichMarkdownEditor.
run: (editor) => {
const src = window.prompt('Image URL')
if (!src) {
return
}
editor.chain().focus().setImage({ src }).run()
editor.chain().focus().run()
}
}
]

View file

@ -2,6 +2,7 @@ import type { AnyExtension } from '@tiptap/core'
import StarterKit from '@tiptap/starter-kit'
import Link from '@tiptap/extension-link'
import Image from '@tiptap/extension-image'
import type { DOMOutputSpec } from '@tiptap/pm/model'
import Placeholder from '@tiptap/extension-placeholder'
import TaskList from '@tiptap/extension-task-list'
import TaskItem from '@tiptap/extension-task-item'
@ -10,6 +11,7 @@ import { TableCell } from '@tiptap/extension-table-cell'
import { TableHeader } from '@tiptap/extension-table-header'
import { TableRow } from '@tiptap/extension-table-row'
import { Markdown } from '@tiptap/markdown'
import { getMarkdownPreviewImageSrc } from './markdown-preview-links'
import { RawMarkdownHtmlBlock, RawMarkdownHtmlInline } from './raw-markdown-html'
const RICH_MARKDOWN_PLACEHOLDER = 'Write markdown… Type / for blocks.'
@ -29,7 +31,24 @@ export function createRichMarkdownExtensions({
Link.configure({
openOnClick: false
}),
Image,
// Why: the default Image extension renders <img src="image.png"> which
// resolves against the app origin (localhost), not the file system. This
// custom extension overrides renderHTML to resolve relative src values to
// file:// URLs using the exact same resolver as preview mode, so nested
// paths and Windows drive roots stay consistent across both surfaces.
Image.extend({
addStorage() {
return { filePath: '' }
},
renderHTML({ HTMLAttributes }) {
const src = HTMLAttributes.src as string | undefined
const filePath = this.storage.filePath as string
const resolvedSrc = filePath ? getMarkdownPreviewImageSrc(src, filePath) : src
return ['img', { ...HTMLAttributes, src: resolvedSrc }] satisfies DOMOutputSpec
}
}).configure({
allowBase64: true
}),
TaskList,
TaskItem.configure({
nested: true

View file

@ -0,0 +1,47 @@
import { basename, dirname, joinPath } from '@/lib/path'
export function extractIpcErrorMessage(err: unknown, fallback: string): string {
if (!(err instanceof Error)) {
return fallback
}
const match = err.message.match(/Error invoking remote method '[^']*': (?:Error: )?(.+)/)
return match ? match[1] : err.message
}
function splitFileExtension(fileName: string): { stem: string; extension: string } {
const extensionStart = fileName.lastIndexOf('.')
if (extensionStart <= 0) {
return { stem: fileName, extension: '' }
}
return {
stem: fileName.slice(0, extensionStart),
extension: fileName.slice(extensionStart)
}
}
export async function getImageCopyDestination(
markdownFilePath: string,
sourceImagePath: string
): Promise<{ imageName: string; destPath: string }> {
const originalImageName = basename(sourceImagePath)
const markdownDir = dirname(markdownFilePath)
const { stem, extension } = splitFileExtension(originalImageName)
let imageName = originalImageName
let destPath = joinPath(markdownDir, imageName)
let suffix = 1
const MAX_DECONFLICT_ATTEMPTS = 1000
// Why: picking "diagram.png" from elsewhere should not silently replace an
// existing sibling asset in the note's directory. We deconflict the copy
// target and keep the inserted markdown pointing at the unique name.
while (destPath !== sourceImagePath && (await window.api.shell.pathExists(destPath))) {
if (suffix >= MAX_DECONFLICT_ATTEMPTS) {
throw new Error(`Too many name collisions for "${originalImageName}".`)
}
imageName = `${stem}-${suffix}${extension}`
destPath = joinPath(markdownDir, imageName)
suffix += 1
}
return { imageName, destPath }
}

View file

@ -0,0 +1,22 @@
import { describe, expect, it } from 'vitest'
import { dirname, joinPath } from './path'
describe('dirname', () => {
it('keeps the POSIX root when resolving a file in the filesystem root', () => {
expect(dirname('/README.md')).toBe('/')
})
it('keeps the POSIX root when given the root path directly', () => {
expect(dirname('/')).toBe('/')
})
it('keeps the Windows drive root when resolving a file in the drive root', () => {
expect(dirname('C:\\README.md')).toBe('C:')
})
})
describe('joinPath', () => {
it('joins onto a Windows drive root returned by dirname', () => {
expect(joinPath(dirname('C:\\README.md'), 'image.png')).toBe('C:/image.png')
})
})

View file

@ -26,15 +26,28 @@ export function basename(path: string): string {
export function dirname(path: string): string {
const normalizedPath = stripTrailingSeparators(path)
if (!normalizedPath) {
return getSeparator(path)
}
if (/^[A-Za-z]:$/.test(normalizedPath)) {
return normalizedPath
}
const lastSeparatorIndex = Math.max(
normalizedPath.lastIndexOf('/'),
normalizedPath.lastIndexOf('\\')
)
if (lastSeparatorIndex <= 0) {
if (lastSeparatorIndex === -1) {
return '.'
}
if (lastSeparatorIndex === 0) {
return normalizedPath[0]
}
return normalizedPath.slice(0, lastSeparatorIndex)
}