feat(client/menus): add Copy as Markdown#8808
Conversation
Summary of ChangesHello @yzx9, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new feature that enhances user productivity by allowing them to easily copy the content of a note as Markdown directly from the tree context menu. This streamlines the process of sharing or reusing note content in Markdown-compatible applications, improving the overall user experience. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
The pull request successfully adds the "Copy as markdown" feature to the tree context menu, providing a convenient way to export note content. The implementation is consistent with existing context menu actions and includes comprehensive translations for 17 languages. Note that there is a small typo in the PR title ("meun" instead of "menu"). The main area for improvement is the use of the native fetch API instead of the internal server service for the network request, which ensures proper header handling (e.g., CSRF tokens).
fb34274 to
5ed6282
Compare
5ed6282 to
a240c40
Compare
eliandoran
left a comment
There was a problem hiding this comment.
Thanks for the implementation, it better aligns with our functional expectations, but I still have a few topics that need addressing.
|
|
||
| { title: t("tree-context-menu.export"), command: "exportNote", uiIcon: "bx bx-export", enabled: notSearch && noSelectedNotes && notOptionsOrHelp }, | ||
|
|
||
| { title: t("tree-context-menu.copy-as-markdown"), command: "copyAsMarkdown", uiIcon: "bx bx-copy-alt", enabled: notSearch && noSelectedNotes && notOptionsOrHelp }, |
There was a problem hiding this comment.
To be honest, I don't see the point in having the option to copy as markdown as part of the tree. It's not a general editing pattern.
Let's keep the only the context menu implementation for now.
There was a problem hiding this comment.
The context menu is only available in the Electron version, whereas the tree context menu works in both the web and Electron versions.
There was a problem hiding this comment.
@yzx9 , I understand your concern but the fact remains that tree is for note-related options, there are no other content-copy operations in it.
My suggestion is to only keep the Electron variant for now. Should you wish to implement it for desktop as well, we can go for:
- Making the context menu available on the browser.
- Making it as a keyboard shortcut.
Either way, I would treat those as a separate PR to unblock this one. But removing it from the tree is necessary, regardless.
| const html = await webContents.executeJavaScript(` | ||
| (function() { | ||
| const selection = window.getSelection(); | ||
| if (!selection.rangeCount) return ''; | ||
|
|
||
| const range = selection.getRangeAt(0); | ||
| const div = document.createElement('div'); | ||
| div.appendChild(range.cloneContents()); | ||
| return div.innerHTML; | ||
| })() | ||
| `); | ||
|
|
There was a problem hiding this comment.
Can this be simplified, why is executeJavaScript required?
There was a problem hiding this comment.
Fixed. I overlooked the fact that this script runs in the Renderer process.
a240c40 to
15961cc
Compare
Summary
Screenshots
tree context menu, electron:
tree context menu, web:
electron context menu:
cc @eliandoran
close #5645 #8009