Desktop: Improved resource hover title #1965
Conversation
|
That looks good I think, but please could you show an example of how the HTML would look before, and how it would look now? Just to get a better sense of what's changed. I guess we should have test units for this, but currently we don't unfortunately. This change seems good in itself, however it's not required to get custom icons working. This can be derived from the resource mime type. I'm not sure the title is always the filename - it might depend on how the resource was imported. One might even build a custom importer using the API and forget to set the title. On the other hand, the mime type is automatically set by the app when a resource is imported (via the API or anywhere) so it's more likely to be set and accurate. |
|
Here's an example of an attached word document The old html has a title, but it isn't equal to anything The new html includes a title with the original (uploaded filename) The above images show off change #2, for #1 the only difference is that I've enabled the title attribute of markdown for regular http(s) links. It was a bug in joplin that it didn't work before. Would become And it will now correctly pick up the title And in the new version if you use the same link as above without the title attribute, it will correctly render as |
|
That's a good point about the mimeType, I pushed another commit that adds mimetype to resource links because I figured it would be valuable to add as much info as possible. If it's out of scope for this pull (or you just don't like it) I can pull the commit, no problem. Here is an example of what a resource link will look now edit: it seems like mimetype is always completely specific, for example when a .docx is attached |
| @@ -37,7 +43,7 @@ function installRule(markdownIt, mdOptions, ruleOptions) { | |||
|
|
|||
| let js = `${ruleOptions.postMessageSyntax}(${JSON.stringify(href)}); return false;`; | |||
| if (hrefAttr.indexOf('#') === 0 && href.indexOf('#') === 0) js = ''; // If it's an internal anchor, don't add any JS since the webview is going to handle navigating to the right place | |||
| return `<a data-from-md ${resourceIdAttr} title='${htmlentities(title)}' href='${hrefAttr}' onclick='${js}'>${icon}`; | |||
| return `<a data-from-md ${resourceIdAttr} title='${htmlentities(title)}' href='${hrefAttr}' onclick='${js}' type='${mime}'>${icon}`; | |||
There was a problem hiding this comment.
Like the title, the mime type should be escaped with htmlentities()
|
Good idea to add the mime type there. I didn't know it was a valid attribute so indeed we may as well put it there. Regarding docx, acccording to mime-utils.js, it should have been detected as Otherwise if you could just fix the mime type escaping, it's good to merge. |
|
Thanks @laurent22 I've escaped the mime. I attached from the desktop app, I've added the file below in case you want to try and reproduce it. |
|
Perfect, thanks @CalebJohn. I've also fixed the mime type issue (turns out I was using the simplified version of a package that didn't the vendor types (vnd-*)). |
|
Thanks for fixing it so quickly! |
* Improved resource hover title * Add mimetype to resource links * Escape mime type for resource links
This pull does 2 things.
Actually allows the user to set a markdown link title for resource URLs and internal links
e.g
[internal link](:/51bee0f048994f81a7cbca4e7627ad13 "title")Display the stored resource title for resources on hover.
From what I can tell, this field is always the original filename, is that correct @laurent22 ? This will be useful because it will allow users to hover over a resource to get some additional info.
My other motivation for adding that is it will allow for joplin to display the filetype instead of the resource icon when filetypes are known. Users can do this manually using css, or if you're open to it, I can make a pull request that will make it a feature in Joplin.