Plugins: Commands would not show up in keymap editor when no shortcut was associated with them

Ref: https://discourse.joplinapp.org/t/plugin-note-tabs/12752/39

.eslintignore

@@ -106,6 +106,9 @@ packages/app-cli/tests/models_Note.js.map
 packages/app-cli/tests/models_Setting.d.ts
 packages/app-cli/tests/models_Setting.js
 packages/app-cli/tests/models_Setting.js.map
+packages/app-cli/tests/services/plugins/api/JoplinViewMenuItem.d.ts
+packages/app-cli/tests/services/plugins/api/JoplinViewMenuItem.js
+packages/app-cli/tests/services/plugins/api/JoplinViewMenuItem.js.map
 packages/app-cli/tests/services/plugins/api/JoplinWorkspace.d.ts
 packages/app-cli/tests/services/plugins/api/JoplinWorkspace.js
 packages/app-cli/tests/services/plugins/api/JoplinWorkspace.js.map

.gitignore

@@ -95,6 +95,9 @@ packages/app-cli/tests/models_Note.js.map
 packages/app-cli/tests/models_Setting.d.ts
 packages/app-cli/tests/models_Setting.js
 packages/app-cli/tests/models_Setting.js.map
+packages/app-cli/tests/services/plugins/api/JoplinViewMenuItem.d.ts
+packages/app-cli/tests/services/plugins/api/JoplinViewMenuItem.js
+packages/app-cli/tests/services/plugins/api/JoplinViewMenuItem.js.map
 packages/app-cli/tests/services/plugins/api/JoplinWorkspace.d.ts
 packages/app-cli/tests/services/plugins/api/JoplinWorkspace.js
 packages/app-cli/tests/services/plugins/api/JoplinWorkspace.js.map

packages/app-cli/tests/services/plugins/api/JoplinViewMenuItem.ts

@@ -0,0 +1,57 @@
+import KeymapService from '@joplin/lib/services/KeymapService';
+import PluginService from '@joplin/lib/services/plugins/PluginService';
+const { newPluginService, newPluginScript, setupDatabaseAndSynchronizer, switchClient, afterEachCleanUp } = require('../../../test-utils');
+
+describe('JoplinViewMenuItem', () => {
+
+	beforeEach(async (done) => {
+		await setupDatabaseAndSynchronizer(1);
+		await switchClient(1);
+		done();
+	});
+
+	afterEach(async () => {
+		await afterEachCleanUp();
+	});
+
+	test('should register commands with the keymap service', async () => {
+		const service = new newPluginService() as PluginService;
+
+		KeymapService.instance().initialize();
+
+		const pluginScript = newPluginScript(`			
+			joplin.plugins.register({
+				onStart: async function() {
+					await joplin.commands.register({
+						name: 'testCommand1',
+						label: 'My Test Command 1',
+						iconName: 'fas fa-music',
+						execute: async () => {},
+					});
+
+					await joplin.views.menuItems.create('myMenuItem1', 'testCommand1', 'tools', { accelerator: 'CmdOrCtrl+Alt+Shift+B' });
+
+					await joplin.commands.register({
+						name: 'testCommand2',
+						label: 'My Test Command 2',
+						iconName: 'fas fa-music',
+						execute: async () => {},
+					});
+
+					await joplin.views.menuItems.create('myMenuItem2', 'testCommand2', 'tools');
+				},
+			});
+		`);
+
+		const plugin = await service.loadPluginFromJsBundle('', pluginScript);
+		await service.runPlugin(plugin);
+
+		const commandNames = KeymapService.instance().getCommandNames();
+
+		expect(commandNames.includes('testCommand1')).toBe(true);
+		expect(commandNames.includes('testCommand2')).toBe(true);
+
+		await service.destroy();
+	});
+
+});

packages/app-cli/tests/test-utils.js

@@ -41,6 +41,7 @@ const DecryptionWorker = require('@joplin/lib/services/DecryptionWorker.js');
 const ResourceService = require('@joplin/lib/services/ResourceService').default;
 const RevisionService = require('@joplin/lib/services/RevisionService.js');
 const ResourceFetcher = require('@joplin/lib/services/ResourceFetcher.js');
+const KeymapService = require('@joplin/lib/services/KeymapService').default;
 const KvStore = require('@joplin/lib/services/KvStore').default;
 const WebDavApi = require('@joplin/lib/WebDavApi');
 const DropboxApi = require('@joplin/lib/DropboxApi');
@@ -141,11 +142,12 @@ setSyncTargetName('memory');
 
 const syncDir = `${__dirname}/../tests/sync/${suiteName_}`;
 
-// TODO: Should probably update this for Jest?
+// 90 seconds now that the tests are running in parallel and have been
-
+// split into smaller suites might not be necessary but for now leave it
-// let defaultJasmineTimeout = 90 * 1000;
+// anyway.
-// if (isNetworkSyncTarget_) defaultJasmineTimeout = 60 * 1000 * 10;
+let defaultJestTimeout = 90 * 1000;
-// if (typeof jasmine !== 'undefined') jasmine.DEFAULT_TIMEOUT_INTERVAL = defaultJasmineTimeout;
+if (isNetworkSyncTarget_) defaultJestTimeout = 60 * 1000 * 10;
+jest.setTimeout(defaultJestTimeout);
 
 const dbLogger = new Logger();
 dbLogger.addTarget('console');
@@ -206,6 +208,7 @@ function currentClientId() {
 
 async function afterEachCleanUp() {
 	await ItemChange.waitForAllSaved();
+	KeymapService.destroyInstance();
 }
 
 async function switchClient(id, options = null) {

packages/lib/services/plugins/api/JoplinViewsMenuItems.ts

@@ -14,7 +14,7 @@ export default class JoplinViewsMenuItems {
 	private store: any;
 	private plugin: Plugin;
 
-	constructor(plugin: Plugin, store: any) {
+	public constructor(plugin: Plugin, store: any) {
 		this.store = store;
 		this.plugin = plugin;
 	}
@@ -22,7 +22,7 @@ export default class JoplinViewsMenuItems {
 	/**
 	 * Creates a new menu item and associate it with the given command. You can specify under which menu the item should appear using the `location` parameter.
 	 */
-	async create(id: string, commandName: string, location: MenuItemLocation = MenuItemLocation.Tools, options: CreateMenuItemOptions = null) {
+	public async create(id: string, commandName: string, location: MenuItemLocation = MenuItemLocation.Tools, options: CreateMenuItemOptions = null) {
 		if (typeof location !== 'string') {
 			this.plugin.deprecationNotice('1.5', 'Creating a view without an ID is deprecated. To fix it, change your call to `joplin.views.menuItem.create("my-unique-id", ...)`');
 			options = location as any;
@@ -35,8 +35,17 @@ export default class JoplinViewsMenuItems {
 		const controller = new MenuItemController(handle, this.plugin.id, this.store, commandName, location);
 		this.plugin.addViewController(controller);
 
+		// Register the command with the keymap service - not that if no
+		// accelerator is provided, we still register the command, so that
+		// it appears in the keymap editor, which will allow the user to
+		// set a custom shortcut.
+		//
+		// https://discourse.joplinapp.org/t/plugin-note-tabs/12752/39
+
 		if (options && options.accelerator) {
 			KeymapService.instance().registerCommandAccelerator(commandName, options.accelerator);
+		} else {
+			KeymapService.instance().registerCommandAccelerator(commandName, null);
 		}
 	}