feat: add Markdown copy functionality and enhance UI interactions#1150
feat: add Markdown copy functionality and enhance UI interactions#1150yilmazemrepala wants to merge 1 commit intoblinkospace:mainfrom
Conversation
- Added "Copy as Markdown" and "Copy" options in the context menu for notes. - Implemented NoteCopyDropdown component for copying note content and attachments. - Updated translations for English, Chinese (TODO: verify) and Turkish, and to include new copy options. - Refactored BlinkoCard and FullscreenEditor components to integrate the new copy functionality. - Enhanced CommentButton to support toolbar grouping for better UI consistency.
blinko-space
left a comment
There was a problem hiding this comment.
Review: Changes requested 🔧
Thanks for this — the core design is nice: clean lib/component split, lazy-loaded marked, proper ClipboardItem fallback chain. A few things to address before merge:
Blockers
-
Unrelated change in
server/package.json— adding theprisma:generatescript isn't part of "markdown copy". Please remove from this PR and open a separate one if you want it. -
Non-English comments in shared code —
app/src/lib/noteClipboard.ts:73,83has Turkish JSDoc (Zengin (HTML + düz metin) not kopyası,Yalnızca ham markdown). Please translate to English so future contributors can read it. -
New
markeddependency (~200KB) — the app already bundlesvditorwhich has its own markdown parser. Please check whether you can reuse that (or another already-present parser) before adding a second one. If a separate parser is genuinely needed, briefly justify it. -
i18n is incomplete — you've added
copy-markdown,copy-standard,copy-successtoen,tr,zhonly, but the project ships 15 locales. Users onde/fr/es/ja/ko/...will see raw translation keys. Either:- Add at least English-fallback copies of these keys to every locale file, or
- Confirm in a comment that i18next is configured with EN fallback for missing keys (I can check if you want).
-
No tests — per
CLAUDE.local.mdthis project requires test coverage for feature PRs. Please add at least:- Unit tests for
buildFullPlainTextForNote,buildAttachmentUrlsFromNote,markdownToHtmlFragment(pure functions — easy) - An integration sanity check that the clipboard lib doesn't throw on empty notes / notes with no attachments / very large notes.
- Unit tests for
Nits
z-[10060](NoteCopyDropdown/index.tsx:43) — hard-coded z-index. If there's a project z-index token, use it.- Clipboard HTML sanitization —
marked.parse(...)output is written to the clipboard unsanitized. Low real-world risk because most paste targets (Gmail, Word, Docs) sanitize incoming HTML, but if notes can contain untrusted content (shared/public notes, AI-generated content), consider running the HTML through DOMPurify before handing it toClipboardItem. - Chinese translations — PR body says they need native-speaker verification. Please resolve before merge (happy to ask the maintainer team).
commentButton.tsxtoolbarGroupedprop — borderline scope creep but acceptable since the new toolbar layout needs it.
Runtime verification
I'll run the full feature in the Blinko dev server with browser-use on the next review round once the code-level issues are addressed. Happy to verify: Word paste, Obsidian/Typora paste, attachment link appending, light/dark mode, keyboard fallback, and Tauri desktop webview (where ClipboardItem support is spotty).
Once the blockers are resolved, ping me and I'll re-review + do the live testing. 👍
Summary
Adds multi-format copy options for notes: HTML (rich text), plain text, and Markdown.
Changes
NoteCopyDropdowncomponentBlinkoCardandFullscreenEditorfor new copy functionalityCommentButtontoolbar groupingTesting
Notes
Chinese translations need verification by native speaker.