Desktop: Fixes #9588: Removed download button on mermaid diagrams in rich text editor #12055
Conversation
… work Added an additional verification that will only allow the mermaid diagram download button to appear if the user is using the markdown editor. It uses the document object, which represents the loaded html page, and the function querySelector to look for the first instance of use of the tox-tinymce class, exclusively used in the rich text editor. If the return value of this verification is not null, the constant exportButtonMarkup will simply be an empty string, instead of calling the exportGraphButton function. This way, the download button will not be visible in the rich text editor.
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
| @@ -62,7 +62,7 @@ export default { | |||
| return self.renderToken(tokens, idx, options, env, self); | |||
| }; | |||
|
|
|||
| const exportButtonMarkup = isDesktop(ruleOptions.platformName) ? exportGraphButton(ruleOptions) : ''; | |||
| const exportButtonMarkup = (isDesktop(ruleOptions.platformName) && !document.querySelector('.tox-tinymce')) ? exportGraphButton(ruleOptions) : ''; | |||
There was a problem hiding this comment.
We shouldn't have references to the document here as there's no guarantee it's always available. As you see we normally pass arguments, such as ruleOptions.platformName to inform how things should work within the package. So it should be a similar solution here
There was a problem hiding this comment.
My first approach was actually to alter RuleOptions so that it could pass the editor as an argument, however I did not find a way to detect which editor was currently in use. Since i didn't find much success with that approach I decided to go for the change you saw...
There was a problem hiding this comment.
Setting.value('editor.codeView') would tell you that
|
Thank you for working on this! For reference, I'm linking to a commit that takes an alternate approach that might fix this if we do eventually want to support the download button in the Rich Text Editor. (If I recall correctly, there was an issue with the linked commit (e.g. right-click menus opened with the keyboard didn't work?)). |
| @@ -1,4 +1,5 @@ | |||
| import { RuleOptions } from '../../MdToHtml'; | |||
| import Setting from '../../../lib/models/Setting'; | |||
There was a problem hiding this comment.
Unfortunately we also can't have this here because that introduces a dependency to settings which may or may not have been initialized at that point (or which may be irrelevant in some contexts). You've also created a circular loop so the project will randomly not compile.
I assume this is more challenging to fix than it seems. Whenever I want to change something in here I have to change 5 or 6 files to pass around options.
There was a problem hiding this comment.
Oh I see. I was finding it weird because i could run Joplin with the fix on my machine however the checks kept failing and I wasn't being able to find the cause through the logs.
|
The purpose of this PR was to remove the download button because it wasn't functional. However, now that it does show a menu (even if without download options), is there still a reason to remove the button? |
This PR fixes #9588 by removing the download button while hovering over a mermaid diagram in the rich text editor.
I simply added a verification that checks if the tox-tinymce class is used. If that is the case, the exportButtonMarkup constant will simply be an empty string, resulting in the download button not being visible. Since this class is only found in the rich text editor, the download button will remain visible and functional in the markdown editor.
Video.sem.titulo.-.Feito.com.Clipchamp.mp4