Dekstop: Refactor: Remove templates feature #5115
Conversation
|
Maybe it's worth keeping until the plugin is ready? |
|
Yup, but I made the PR so that the PR can be reviewed properly before getting merged. Also (https://discourse.joplinapp.org/t/template-plugin/17470/41?u=nishantwrp) |
|
What happens if the user has defined a custom shortcut for "insertTemplate", and then upgrades to this version? I'm not sure actually if it will just do nothing, or crash. Could you double-check this please? |
| @@ -16,7 +15,7 @@ export const runtime = (): CommandRuntime => { | |||
| const folderId = Setting.value('activeFolderId'); | |||
| if (!folderId) return; | |||
|
|
|||
| const body = template ? TemplateUtils.render(template) : ''; | |||
| const body = template ? template : ''; | |||
There was a problem hiding this comment.
Please remove anything related to templates from the code. That includes this variable and the signature of this function. Also make sure you update all code that calls "newNote", such as the "newTodo" command.
There was a problem hiding this comment.
Oh ok, missed it. Done.
| @@ -555,9 +516,6 @@ function useMenu(props: Props) { | |||
| selector: 'performClose:', | |||
| }, { | |||
| type: 'separator', | |||
| }, { | |||
| label: _('Templates'), | |||
| submenu: templateItems, | |||
There was a problem hiding this comment.
Did you test the app after having made your changes? Because if you remove this, there will be two successive two separators in the menu, so you need to remove one separator. Please test the code. Check all the parts where you've changed something.
There was a problem hiding this comment.
Actually, I did test it and I couldn't see two separators in the UI. But anyways, removed it.
| }, { | ||
| label: _('Templates'), | ||
| visible: shim.isMac() ? false : true, | ||
| submenu: templateItems, |
|
You also need to remove any mention of templates from README.md, although you should probably copy the Note Templates section and add it to your plugin doc. |
Done. I've copied it and will add it there in the future when the plugin is complete/ |
Yes done. I created a custom shortcut to insertTemplate, and switched to this branch and saw that there were no console errors. Although when I saw the custom keyboard map and searched for insertTemplate the results were as follows
You can see that we have insertTemplate mapped but it's marked as invalid. |
|
Ok it's merge now. I've put the changes in this branch so if you want to test without the template feature, please use this: |
This PR removes the templates feature from Joplin as a separate plugin for that is being developed here (https://github.com/joplin/plugin-templates).