-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix(mind-map): show text in links between nodes on export #7938
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Summary of ChangesHello @lzinga, 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 overhauls the SVG and PNG export mechanisms within the application by integrating the Highlights
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces snapdom to fix an issue with mind map exports, which is a great improvement that also simplifies the download logic. The refactoring of the download functions is well-executed. I've identified a couple of high-severity bugs in SvgSplitEditor.tsx where filenames for exported files will have double extensions. Additionally, I've provided some medium-severity suggestions to improve code maintainability by reducing code duplication and fixing a minor style issue. Overall, these are solid changes that enhance the application's export functionality.
apps/client/src/widgets/type_widgets/helpers/SvgSplitEditor.tsx
Outdated
Show resolved
Hide resolved
apps/client/src/widgets/type_widgets/helpers/SvgSplitEditor.tsx
Outdated
Show resolved
Hide resolved
| const result = await snapdom(element, { | ||
| backgroundColor: "transparent", | ||
| scale: 2 | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is needed at this point, if we use it in more places then I would say lets put it into its own object.
| const result = await snapdom(apiRef.current.nodes, { | ||
| backgroundColor: "transparent", | ||
| scale: 2 | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is needed at this point, if we use it in more places then I would say lets put it into its own object.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This ended up being slightly larger than expected but I think it is an over all good change and enhanced the over all usage in my opinion.
I chose to go with https://github.com/zumerlab/snapdom over modern-screenshot due to its more active maintenance and richer feature set. You could technically use this to download any div/html from a page as an svg/png which may open the door for future exports. You can see some of the things it can capture here https://snapdom.dev/
Since I changed the download functions, I also had to update
apps\client\src\widgets\type_widgets\helpers\SvgSplitEditor.tsxto use the new functions. I searched the rest of the project for any usages ofdownloadSvgordownloadSvgAsPngand the changes are all I found. I would have kept the original functions but since they don't seem to be used in many places, I figured it was worthwhile to do a slightly larger refactor.This change also fixes these issues -

Fixes #7829 - here is an image directly exported from trilium using the new methods.