-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
disable hyperlink for attachments #2474
Conversation
@@ -52,7 +52,8 @@ | |||
|
|||
if (className.indexOf('cm-url') !== -1) { | |||
const match = /^\((.*)\)|\[(.*)\]|(.*)$/.exec(el.textContent) | |||
return match[1] || match[2] || match[3] | |||
const url = match[1] || match[2] || match[3] | |||
return /^:storage\//.test(url) ? null : url |
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.
do you mind to use attachmentManagement.STORAGE_FOLDER_PLACEHOLDER instead of hard-coding :storage
here?
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.
@ehhc it's not possible
require('../browser/main/lib/dataApi/attachmentManagement')
throws the error Cannot find module 'browser/lib/findStorage'
hyperlink.js
is loaded by Chromium
attachmentManagement.js
is written to be loaded by node
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.
if it is really not possible (i am not a nodejs/electron pro, so i'm not sure about that) could you at least add an comment pointing to that file? That would help a lot in case somebody will refactor it again (e.g. in a few years..)
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.
@ehhc sure, I can do that
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.
LGTM 🎉
This change disables the following popover on attached images: