Desktop: Keyboard shortcut editor

.eslintignore

@@ -63,6 +63,8 @@ Modules/TinyMCE/langs/
 # AUTO-GENERATED - EXCLUDED TYPESCRIPT BUILD
 CliClient/app/LinkSelector.js
 CliClient/build/LinkSelector.js
+CliClient/tests-build/synchronizer_LockHandler.js
+CliClient/tests-build/synchronizer_MigrationHandler.js
 CliClient/tests/synchronizer_LockHandler.js
 CliClient/tests/synchronizer_MigrationHandler.js
 ElectronClient/commands/focusElement.js
@@ -71,6 +73,12 @@ ElectronClient/commands/stopExternalEditing.js
 ElectronClient/global.d.js
 ElectronClient/gui/ErrorBoundary.js
 ElectronClient/gui/Header/commands/focusSearch.js
+ElectronClient/gui/KeymapConfig/KeymapConfigScreen.js
+ElectronClient/gui/KeymapConfig/ShortcutRecorder.js
+ElectronClient/gui/KeymapConfig/styles/index.js
+ElectronClient/gui/KeymapConfig/utils/getLabel.js
+ElectronClient/gui/KeymapConfig/utils/useCommandStatus.js
+ElectronClient/gui/KeymapConfig/utils/useKeymap.js
 ElectronClient/gui/MainScreen/commands/editAlarm.js
 ElectronClient/gui/MainScreen/commands/exportPdf.js
 ElectronClient/gui/MainScreen/commands/hideModalMessage.js

.gitignore

@@ -54,6 +54,8 @@ Tools/commit_hook.txt
 # AUTO-GENERATED - EXCLUDED TYPESCRIPT BUILD
 CliClient/app/LinkSelector.js
 CliClient/build/LinkSelector.js
+CliClient/tests-build/synchronizer_LockHandler.js
+CliClient/tests-build/synchronizer_MigrationHandler.js
 CliClient/tests/synchronizer_LockHandler.js
 CliClient/tests/synchronizer_MigrationHandler.js
 ElectronClient/commands/focusElement.js
@@ -62,6 +64,12 @@ ElectronClient/commands/stopExternalEditing.js
 ElectronClient/global.d.js
 ElectronClient/gui/ErrorBoundary.js
 ElectronClient/gui/Header/commands/focusSearch.js
+ElectronClient/gui/KeymapConfig/KeymapConfigScreen.js
+ElectronClient/gui/KeymapConfig/ShortcutRecorder.js
+ElectronClient/gui/KeymapConfig/styles/index.js
+ElectronClient/gui/KeymapConfig/utils/getLabel.js
+ElectronClient/gui/KeymapConfig/utils/useCommandStatus.js
+ElectronClient/gui/KeymapConfig/utils/useKeymap.js
 ElectronClient/gui/MainScreen/commands/editAlarm.js
 ElectronClient/gui/MainScreen/commands/exportPdf.js
 ElectronClient/gui/MainScreen/commands/hideModalMessage.js

CliClient/tests/services_KeymapService.js

@@ -115,6 +115,7 @@ describe('services_KeymapService', () => {
 				{ command: 'focusElementNoteTitle', accelerator: 'Option+Shift+Cmd+T' },
 				{ command: 'focusElementNoteBody', accelerator: 'Ctrl+Option+Shift+Cmd+B' },
 			];
+
 			testCases_Darwin.forEach(({ command, accelerator }) => {
 				keymapService.setAccelerator(command, accelerator);
 				expect(keymapService.getAccelerator(command)).toEqual(accelerator);
@@ -131,17 +132,18 @@ describe('services_KeymapService', () => {
 				{ command: 'focusElementNoteTitle', accelerator: 'Ctrl+Alt+Shift+T' },
 				{ command: 'focusElementNoteBody', accelerator: 'Ctrl+Alt+Shift+B' },
 			];
+
 			testCases_Linux.forEach(({ command, accelerator }) => {
 				keymapService.setAccelerator(command, accelerator);
 				expect(keymapService.getAccelerator(command)).toEqual(accelerator);
 			});
 		});
 	});
 
-	describe('resetAccelerator', () => {
+	describe('getDefaultAccelerator', () => {
 		beforeEach(() => keymapService.initialize());
 
-		it('should reset the Accelerator', () => {
+		it('should return the default accelerator', () => {
 			const testCases = [
 				{ command: 'newNote', accelerator: 'Ctrl+Alt+Shift+N' },
 				{ command: 'synchronize', accelerator: null /* Disabled */ },
@@ -154,26 +156,22 @@ describe('services_KeymapService', () => {
 			];
 
 			testCases.forEach(({ command, accelerator }) => {
-				// Remember the default Accelerator value
-				const prevAccelerator = keymapService.getAccelerator(command);
+				// Remember the real default Accelerator value
+				const defaultAccelerator = keymapService.getAccelerator(command);
 
-				// Update the Accelerator,
+				// Update the Accelerator and then retrieve the default accelerator
 				keymapService.setAccelerator(command, accelerator);
-				expect(keymapService.getAccelerator(command)).toEqual(accelerator);
-
-				// and then reset it..
-				keymapService.resetAccelerator(command);
-				expect(keymapService.getAccelerator(command)).toEqual(prevAccelerator);
+				expect(keymapService.getDefaultAccelerator(command)).toEqual(defaultAccelerator);
 			});
 		});
 	});
 
-	describe('setKeymap', () => {
+	describe('overrideKeymap', () => {
 		beforeEach(() => keymapService.initialize());
 
 		it('should update the keymap', () => {
 			keymapService.initialize('darwin');
-			const customKeymap_Darwin = [
+			const customKeymapItems_Darwin = [
 				{ command: 'newNote', accelerator: 'Option+Shift+Cmd+N' },
 				{ command: 'synchronize', accelerator: 'F11' },
 				{ command: 'textBold', accelerator: 'Shift+F5' },
@@ -187,14 +185,14 @@ describe('services_KeymapService', () => {
 				{ command: 'focusElementNoteList', accelerator: 'Shift+Cmd+S' /* Default of focusElementSideBar */ },
 			];
 
-			expect(() => keymapService.setKeymap(customKeymap_Darwin)).not.toThrow();
-			customKeymap_Darwin.forEach(({ command, accelerator }) => {
+			expect(() => keymapService.overrideKeymap(customKeymapItems_Darwin)).not.toThrow();
+			customKeymapItems_Darwin.forEach(({ command, accelerator }) => {
 				// Also check if keymap is updated or not
 				expect(keymapService.getAccelerator(command)).toEqual(accelerator);
 			});
 
 			keymapService.initialize('win32');
-			const customKeymap_Win32 = [
+			const customKeymapItems_Win32 = [
 				{ command: 'newNote', accelerator: 'Ctrl+Alt+Shift+N' },
 				{ command: 'synchronize', accelerator: 'F11' },
 				{ command: 'textBold', accelerator: 'Shift+F5' },
@@ -208,8 +206,8 @@ describe('services_KeymapService', () => {
 				{ command: 'focusElementNoteList', accelerator: 'Ctrl+Shift+S' /* Default of focusElementSideBar */ },
 			];
 
-			expect(() => keymapService.setKeymap(customKeymap_Win32)).not.toThrow();
-			customKeymap_Win32.forEach(({ command, accelerator }) => {
+			expect(() => keymapService.overrideKeymap(customKeymapItems_Win32)).not.toThrow();
+			customKeymapItems_Win32.forEach(({ command, accelerator }) => {
 				// Also check if keymap is updated or not
 				expect(keymapService.getAccelerator(command)).toEqual(accelerator);
 			});
@@ -240,30 +238,30 @@ describe('services_KeymapService', () => {
 			];
 
 			for (let i = 0; i < customKeymaps.length; i++) {
-				const customKeymap = customKeymaps[i];
-				expect(() => keymapService.setKeymap(customKeymap)).toThrow();
+				const customKeymapItems = customKeymaps[i];
+				expect(() => keymapService.overrideKeymap(customKeymapItems)).toThrow();
 			}
 		});
 
 		it('should throw when the provided Accelerators are invalid', () => {
 			// Only one test case is provided since KeymapService.validateAccelerator() is already tested
-			const customKeymap = [
+			const customKeymapItems = [
 				{ command: 'gotoAnything', accelerator: 'Ctrl+Shift+G' },
 				{ command: 'print', accelerator: 'Alt+P' },
 				{ command: 'focusElementNoteTitle', accelerator: 'Ctrl+Alt+Shift+J+O+P+L+I+N' },
 			];
 
-			expect(() => keymapService.setKeymap(customKeymap)).toThrow();
+			expect(() => keymapService.overrideKeymap(customKeymapItems)).toThrow();
 		});
 
 		it('should throw when the provided commands are invalid', () => {
-			const customKeymap = [
+			const customKeymapItems = [
 				{ command: 'totallyInvalidCommand', accelerator: 'Ctrl+Shift+G' },
 				{ command: 'print', accelerator: 'Alt+P' },
 				{ command: 'focusElementNoteTitle', accelerator: 'Ctrl+Alt+Shift+J' },
 			];
 
-			expect(() => keymapService.setKeymap(customKeymap)).toThrow();
+			expect(() => keymapService.overrideKeymap(customKeymapItems)).toThrow();
 		});
 
 		it('should throw when duplicate accelerators are provided', () => {
@@ -281,14 +279,8 @@ describe('services_KeymapService', () => {
 			];
 
 			for (let i = 0; i < customKeymaps_Darwin.length; i++) {
-				const customKeymap = customKeymaps_Darwin[i];
-				const defaultAccelerators = customKeymap.map(({ command }) => keymapService.getAccelerator(command));
-
-				expect(() => keymapService.setKeymap(customKeymap)).toThrow();
-				// All items should be reset to default values
-				for (let j = 0; j < customKeymap.length; j++) {
-					expect(keymapService.getAccelerator(customKeymap[j].command)).toEqual(defaultAccelerators[j]);
-				}
+				const customKeymapItems = customKeymaps_Darwin[i];
+				expect(() => keymapService.overrideKeymap(customKeymapItems)).toThrow();
 			}
 
 			const customKeymaps_Linux = [
@@ -305,14 +297,8 @@ describe('services_KeymapService', () => {
 			];
 
 			for (let i = 0; i < customKeymaps_Linux.length; i++) {
-				const customKeymap = customKeymaps_Linux[i];
-				const defaultAccelerators = customKeymap.map(({ command }) => keymapService.getAccelerator(command));
-
-				expect(() => keymapService.setKeymap(customKeymap)).toThrow();
-
-				for (let j = 0; j < customKeymap.length; j++) {
-					expect(keymapService.getAccelerator(customKeymap[j].command)).toEqual(defaultAccelerators[j]);
-				}
+				const customKeymapItems = customKeymaps_Linux[i];
+				expect(() => keymapService.overrideKeymap(customKeymapItems)).toThrow();
 			}
 		});
 	});

ElectronClient/app.js

@@ -30,7 +30,7 @@ const PluginManager = require('lib/services/PluginManager');
 const RevisionService = require('lib/services/RevisionService');
 const MigrationService = require('lib/services/MigrationService');
 const CommandService = require('lib/services/CommandService').default;
-const KeymapService = require('lib/services/KeymapService.js').default;
+const KeymapService = require('lib/services/KeymapService').default;
 const TemplateUtils = require('lib/TemplateUtils');
 const CssUtils = require('lib/CssUtils');
 const resourceEditWatcherReducer = require('lib/services/ResourceEditWatcher/reducer').default;
@@ -110,6 +110,8 @@ class Application extends BaseApplication {
 
 		this.commandService_commandsEnabledStateChange = this.commandService_commandsEnabledStateChange.bind(this);
 		CommandService.instance().on('commandsEnabledStateChange', this.commandService_commandsEnabledStateChange);
+
+		KeymapService.instance().on('keymapChange', this.refreshMenu.bind(this));
 	}
 
 	commandService_commandsEnabledStateChange() {
@@ -569,7 +571,7 @@ class Application extends BaseApplication {
 		const toolsItemsWindowsLinux = toolsItemsFirst.concat([{
 			label: _('Options'),
 			visible: !shim.isMac(),
-			accelerator: shim.isMac() ? null : keymapService.getAccelerator('config'),
+			accelerator: !shim.isMac() && keymapService.getAccelerator('config'),
 			click: () => {
 				this.dispatch({
 					type: 'NAV_GO',
@@ -631,7 +633,7 @@ class Application extends BaseApplication {
 			}, {
 				label: _('Preferences...'),
 				visible: shim.isMac() ? true : false,
-				accelerator: shim.isMac() ? keymapService.getAccelerator('config') : null,
+				accelerator: shim.isMac() && keymapService.getAccelerator('config'),
 				click: () => {
 					this.dispatch({
 						type: 'NAV_GO',
@@ -680,7 +682,7 @@ class Application extends BaseApplication {
 			}, {
 				label: _('Hide %s', 'Joplin'),
 				platforms: ['darwin'],
-				accelerator: shim.isMac() ? keymapService.getAccelerator('hideApp') : null,
+				accelerator: shim.isMac() && keymapService.getAccelerator('hideApp'),
 				click: () => { bridge().electronApp().hide(); },
 			}, {
 				type: 'separator',
@@ -700,7 +702,7 @@ class Application extends BaseApplication {
 				newNotebookItem, {
 					label: _('Close Window'),
 					platforms: ['darwin'],
-					accelerator: shim.isMac() ? keymapService.getAccelerator('closeWindow') : null,
+					accelerator: shim.isMac() && keymapService.getAccelerator('closeWindow'),
 					selector: 'performClose:',
 				}, {
 					type: 'separator',
@@ -1087,7 +1089,7 @@ class Application extends BaseApplication {
 		const keymapService = KeymapService.instance();
 
 		try {
-			await KeymapService.instance().loadKeymap(`${dir}/keymap-desktop.json`);
+			await keymapService.loadCustomKeymap(`${dir}/keymap-desktop.json`);
 		} catch (err) {
 			bridge().showErrorMessageBox(err.message);
 		}

ElectronClient/gui/ConfigScreen.jsx

@@ -10,6 +10,7 @@ const shared = require('lib/components/shared/config-shared.js');
 const ConfigMenuBar = require('./ConfigMenuBar.min.js');
 const { EncryptionConfigScreen } = require('./EncryptionConfigScreen.min');
 const { ClipperConfigScreen } = require('./ClipperConfigScreen.min');
+const { KeymapConfigScreen } = require('./KeymapConfig/KeymapConfigScreen');
 
 class ConfigScreenComponent extends React.Component {
 	constructor() {
@@ -68,6 +69,7 @@ class ConfigScreenComponent extends React.Component {
 	screenFromName(screenName) {
 		if (screenName === 'encryption') return <EncryptionConfigScreen theme={this.props.theme}/>;
 		if (screenName === 'server') return <ClipperConfigScreen theme={this.props.theme}/>;
+		if (screenName === 'keymap') return <KeymapConfigScreen themeId={this.props.theme}/>;
 
 		throw new Error(`Invalid screen name: ${screenName}`);
 	}