From f09a896187e04dcdb7c86c2a273e01e2fca8a389 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Fri, 10 Jul 2020 08:42:21 +0530 Subject: [PATCH 01/48] Blank config screen --- .eslintignore | 1 + .gitignore | 1 + ElectronClient/gui/ConfigScreen.jsx | 2 + ElectronClient/gui/KeymapConfigScreen.tsx | 41 +++++++++++++++++++ .../lib/components/shared/config-shared.js | 6 +++ ReactNativeClient/lib/models/Setting.js | 2 + 6 files changed, 53 insertions(+) create mode 100644 ElectronClient/gui/KeymapConfigScreen.tsx diff --git a/.eslintignore b/.eslintignore index 560ef419a83..da3dbcbe7b1 100644 --- a/.eslintignore +++ b/.eslintignore @@ -67,6 +67,7 @@ ElectronClient/commands/stopExternalEditing.js ElectronClient/global.d.js ElectronClient/gui/ErrorBoundary.js ElectronClient/gui/Header/commands/focusSearch.js +ElectronClient/gui/KeymapConfigScreen.js ElectronClient/gui/MainScreen/commands/editAlarm.js ElectronClient/gui/MainScreen/commands/exportPdf.js ElectronClient/gui/MainScreen/commands/hideModalMessage.js diff --git a/.gitignore b/.gitignore index 0b1d67e6b4e..9157a482b9e 100644 --- a/.gitignore +++ b/.gitignore @@ -58,6 +58,7 @@ ElectronClient/commands/stopExternalEditing.js ElectronClient/global.d.js ElectronClient/gui/ErrorBoundary.js ElectronClient/gui/Header/commands/focusSearch.js +ElectronClient/gui/KeymapConfigScreen.js ElectronClient/gui/MainScreen/commands/editAlarm.js ElectronClient/gui/MainScreen/commands/exportPdf.js ElectronClient/gui/MainScreen/commands/hideModalMessage.js diff --git a/ElectronClient/gui/ConfigScreen.jsx b/ElectronClient/gui/ConfigScreen.jsx index 2347c2ad3e7..57983d6f9c2 100644 --- a/ElectronClient/gui/ConfigScreen.jsx +++ b/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('./KeymapConfigScreen.js'); class ConfigScreenComponent extends React.Component { constructor() { @@ -68,6 +69,7 @@ class ConfigScreenComponent extends React.Component { screenFromName(screenName) { if (screenName === 'encryption') return ; if (screenName === 'server') return ; + if (screenName === 'keymap') return ; throw new Error(`Invalid screen name: ${screenName}`); } diff --git a/ElectronClient/gui/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfigScreen.tsx new file mode 100644 index 00000000000..aa7c2cc85c4 --- /dev/null +++ b/ElectronClient/gui/KeymapConfigScreen.tsx @@ -0,0 +1,41 @@ +const React = require('react'); +const { themeStyle } = require('lib/theme'); +// const { bridge } = require('electron').remote.require('./bridge'); +// const { _ } = require('lib/locale.js'); +// const Setting = require('lib/models/Setting'); + +class KeymapConfigScreen extends React.Component { + constructor() { + super(); + } + + render() { + const theme = themeStyle(this.props.theme); + + const containerStyle = Object.assign({}, theme.containerStyle, { + overflowY: 'scroll', + }); + + // const buttonStyle = Object.assign({}, theme.buttonStyle, { marginRight: 10 }); + + // const stepBoxStyle = { + // border: '1px solid', + // borderColor: theme.dividerColor, + // padding: 15, + // paddingTop: 0, + // marginBottom: 15, + // backgroundColor: theme.backgroundColor, + // }; + + return ( +
+
+
+
+
+
+ ); + } +} + +module.exports = { KeymapConfigScreen }; diff --git a/ReactNativeClient/lib/components/shared/config-shared.js b/ReactNativeClient/lib/components/shared/config-shared.js index 614257c0725..304ce534de0 100644 --- a/ReactNativeClient/lib/components/shared/config-shared.js +++ b/ReactNativeClient/lib/components/shared/config-shared.js @@ -156,6 +156,12 @@ shared.settingsSections = createSelector( isScreen: true, }); + output.push({ + name: 'keymap', + metadatas: [], + isScreen: true, + }); + return output; } ); diff --git a/ReactNativeClient/lib/models/Setting.js b/ReactNativeClient/lib/models/Setting.js index 28f75736eba..d06a182f7f8 100644 --- a/ReactNativeClient/lib/models/Setting.js +++ b/ReactNativeClient/lib/models/Setting.js @@ -1252,6 +1252,7 @@ class Setting extends BaseModel { if (name === 'revisionService') return _('Note History'); if (name === 'encryption') return _('Encryption'); if (name === 'server') return _('Web Clipper'); + if (name === 'keymap') return _('Keyboard Shortcuts'); return name; } @@ -1271,6 +1272,7 @@ class Setting extends BaseModel { if (name === 'revisionService') return 'fas fa-history'; if (name === 'encryption') return 'fas fa-key'; if (name === 'server') return 'far fa-hand-scissors'; + if (name === 'keymap') return 'fa fa-keyboard'; return name; } From 476315719fe1dd6757ba1224ef84efbca97d8b83 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Sun, 19 Jul 2020 00:20:22 +0530 Subject: [PATCH 02/48] Basic UI structure --- ElectronClient/gui/KeymapConfigScreen.tsx | 218 ++++++++++++++++++---- 1 file changed, 185 insertions(+), 33 deletions(-) diff --git a/ElectronClient/gui/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfigScreen.tsx index aa7c2cc85c4..5682638547e 100644 --- a/ElectronClient/gui/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfigScreen.tsx @@ -1,41 +1,193 @@ -const React = require('react'); +import * as React from 'react'; const { themeStyle } = require('lib/theme'); +const { _ } = require('lib/locale.js'); +const { shim } = require('lib/shim'); // const { bridge } = require('electron').remote.require('./bridge'); -// const { _ } = require('lib/locale.js'); // const Setting = require('lib/models/Setting'); -class KeymapConfigScreen extends React.Component { - constructor() { - super(); - } +const KeymapConfigScreen = (props: { theme: Object }) => { + const [accelerator, setAccelerator] = React.useState('Ctrl+X'); + const [filter, setFilter] = React.useState(''); + + const theme = themeStyle(props.theme); + const containerStyle = { + ...theme.containerStyle, + padding: 10, + overflow: 'auto', + }; + + const handleKeydown = ( + e: React.KeyboardEvent + ) => { + e.preventDefault(); + setAccelerator(DOMtoElectronAccelerator(e)); + }; - render() { - const theme = themeStyle(this.props.theme); - - const containerStyle = Object.assign({}, theme.containerStyle, { - overflowY: 'scroll', - }); - - // const buttonStyle = Object.assign({}, theme.buttonStyle, { marginRight: 10 }); - - // const stepBoxStyle = { - // border: '1px solid', - // borderColor: theme.dividerColor, - // padding: 15, - // paddingTop: 0, - // marginBottom: 15, - // backgroundColor: theme.backgroundColor, - // }; - - return ( -
-
-
-
-
+ return ( +
+ + + +
+
- ); +
+ ); +}; + +const TopActions = (props: { + theme: any, + filter: string, + setFilter: Function +}) => { + const theme = themeStyle(props.theme); + const topActionsStyle = { + ...theme.containerStyle, + marginTop: 10, + display: 'flex', + flexDirection: 'row', + }; + const inlineButtonStyle = { + ...theme.buttonStyle, + height: theme.inputStyle.height, + padding: 0, + marginLeft: 12, + }; + const filterInputStyle = { + ...theme.inputStyle, + flexGrow: 1, + }; + + return ( +
+ { props.setFilter(e.target.value); }} + /> + + +
+ ); +}; + +const KeymapTable = (props: { + theme: any +}) => { + const theme = themeStyle(props.theme); + const tableStyle = { + ...theme.containerStyle, + marginTop: 10, + overflow: 'auto', + width: '100%', + }; + + return ( + + + + + + + + + + + + + + + + + +
{_('Command')}{_('Shortcut')}
{_('New note')}{_('Ctrl+N')}
{_('New todo')}{_('Ctrl+T')}
+ ); +}; + +const ShortcutRecorder = (props: { + handleKeydown: (e: React.KeyboardEvent) => void, + accelerator: string, + theme: any, +}) => { + const theme = themeStyle(props.theme); + const inlineButtonStyle = { + ...theme.buttonStyle, + height: theme.inputStyle.height, + padding: 0, + marginLeft: 12, + }; + + return ( +
+ + + +
+ ); +}; + +const DOMtoElectronAccelerator = (e: React.KeyboardEvent) => { + const { key, ctrlKey, metaKey, altKey, shiftKey } = e; + const modifiersPresent = ctrlKey || metaKey || altKey || shiftKey; + + const parts = []; + if (modifiersPresent) { + // First, the modifiers + if (ctrlKey) parts.push('Ctrl'); + if (shim.isMac()) { + if (altKey) parts.push('Option'); + if (shiftKey) parts.push('Shift'); + if (metaKey) parts.push('Cmd'); + } else { + if (altKey) parts.push('Alt'); + if (shiftKey) parts.push('Shift'); + } + // Finally, the key + const _key = DOMtoElectronKey(key); + if (_key) parts.push(_key); + } else if (key === 'Enter') { + alert('Save'); + } else if (key === 'Escape') { + alert('Discard'); + } else { + // Finally, the key + const _key = DOMtoElectronKey(key); + if (_key) parts.push(_key); } -} + return parts.join('+'); +}; + +const DOMtoElectronKey = (key: string) => { + if (/^([A-Z0-9]|F1*[1-9]|F10|F2[0-4]|[`~!@#$%^&*()\-_=[\]\\{}|;':",./<>?]|Enter|Tab|Backspace|Delete|Insert|Home|End|PageUp|PageDown|Escape|MediaStop|MediaPlayPause|PrintScreen])$/.test(key)) return key; + if (/^([a-z])$/.test(key)) return key.toUpperCase(); + if (/^Arrow(Up|Down|Left|Right)|Audio(VolumeUp|VolumeDown|VolumeMute)$/.test(key)) return key.slice(5); + + switch (key) { + case ' ': + return 'Space'; + case '+': + return 'Plus'; + case 'MediaTrackNext': + return 'MediaNextTrack'; + case 'MediaTrackPrevious': + return 'MediaPreviousTrack'; + } + + console.warn(`Ignoring ${key}`); + return null; +}; -module.exports = { KeymapConfigScreen }; +export { KeymapConfigScreen, DOMtoElectronAccelerator, DOMtoElectronKey }; From 2b8bf90d67f3d81370af35aedd50745623bc9c3c Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Mon, 20 Jul 2020 11:33:26 +0530 Subject: [PATCH 03/48] Initial keyboard shortcut editing workflow --- .eslintignore | 1 + .gitignore | 1 + ElectronClient/gui/ConfigScreen.jsx | 2 +- ElectronClient/gui/KeymapConfigScreen.tsx | 196 +++++++++++------- ReactNativeClient/lib/KeymapUtils.ts | 43 ++++ .../lib/services/CommandService.ts | 6 + 6 files changed, 168 insertions(+), 81 deletions(-) create mode 100644 ReactNativeClient/lib/KeymapUtils.ts diff --git a/.eslintignore b/.eslintignore index da3dbcbe7b1..eaf8ca072a3 100644 --- a/.eslintignore +++ b/.eslintignore @@ -147,6 +147,7 @@ ReactNativeClient/lib/joplin-renderer/MdToHtml/rules/fence.js ReactNativeClient/lib/joplin-renderer/MdToHtml/rules/mermaid.js ReactNativeClient/lib/joplin-renderer/MdToHtml/rules/sanitize_html.js ReactNativeClient/lib/JoplinServerApi.js +ReactNativeClient/lib/KeymapUtils.js ReactNativeClient/lib/services/CommandService.js ReactNativeClient/lib/services/keychain/KeychainService.js ReactNativeClient/lib/services/keychain/KeychainServiceDriver.dummy.js diff --git a/.gitignore b/.gitignore index 9157a482b9e..67edfdb77cb 100644 --- a/.gitignore +++ b/.gitignore @@ -138,6 +138,7 @@ ReactNativeClient/lib/joplin-renderer/MdToHtml/rules/fence.js ReactNativeClient/lib/joplin-renderer/MdToHtml/rules/mermaid.js ReactNativeClient/lib/joplin-renderer/MdToHtml/rules/sanitize_html.js ReactNativeClient/lib/JoplinServerApi.js +ReactNativeClient/lib/KeymapUtils.js ReactNativeClient/lib/services/CommandService.js ReactNativeClient/lib/services/keychain/KeychainService.js ReactNativeClient/lib/services/keychain/KeychainServiceDriver.dummy.js diff --git a/ElectronClient/gui/ConfigScreen.jsx b/ElectronClient/gui/ConfigScreen.jsx index 57983d6f9c2..f819f890441 100644 --- a/ElectronClient/gui/ConfigScreen.jsx +++ b/ElectronClient/gui/ConfigScreen.jsx @@ -10,7 +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('./KeymapConfigScreen.js'); +const KeymapConfigScreen = require('./KeymapConfigScreen.js'); class ConfigScreenComponent extends React.Component { constructor() { diff --git a/ElectronClient/gui/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfigScreen.tsx index 5682638547e..25ffc454634 100644 --- a/ElectronClient/gui/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfigScreen.tsx @@ -1,12 +1,48 @@ import * as React from 'react'; +const CommandService = require('lib/services/CommandService').default; +const { DOMtoElectronAccelerator } = require('lib/KeymapUtils.js'); const { themeStyle } = require('lib/theme'); const { _ } = require('lib/locale.js'); -const { shim } = require('lib/shim'); // const { bridge } = require('electron').remote.require('./bridge'); // const Setting = require('lib/models/Setting'); +interface KeymapItem { + label: string, + accelerator: string, + command: string + isEditing: boolean, +} + +// Temporary values +const commandNames = [ + 'textCopy', + 'textCut', + 'textPaste', +]; +const getAccelerator = (commandName: string) => { + switch (commandName) { + case 'textCopy': + return 'Ctrl+C'; + case 'textCut': + return 'Ctrl+X'; + case 'textPaste': + return 'Ctrl+V'; + default: + throw new Error('Invalid command'); + } +}; + +const initialKeymap = commandNames.map(commandName => { + return { + label: CommandService.instance().label(commandName), + accelerator: getAccelerator(commandName), + command: commandName, + isEditing: false, + }; +}); + const KeymapConfigScreen = (props: { theme: Object }) => { - const [accelerator, setAccelerator] = React.useState('Ctrl+X'); + const [keymap, setKeymap] = React.useState([...initialKeymap]); const [filter, setFilter] = React.useState(''); const theme = themeStyle(props.theme); @@ -16,21 +52,35 @@ const KeymapConfigScreen = (props: { theme: Object }) => { overflow: 'auto', }; - const handleKeydown = ( - e: React.KeyboardEvent - ) => { - e.preventDefault(); - setAccelerator(DOMtoElectronAccelerator(e)); + const updateAccelerator = (commandName: string, accelerator: string) => { + const _keymap = [...keymap]; + + _keymap.find(item => item.command === commandName).accelerator = accelerator; + _keymap.find(item => item.command === commandName).isEditing = false; + setKeymap(_keymap); + }; + + const updateIsEditing = (commandName: string, isEditing: boolean) => { + const _keymap = [...keymap]; + + _keymap.forEach(item => item.isEditing = false); + _keymap.find(item => item.command === commandName).isEditing = isEditing; + setKeymap(_keymap); }; return (
- - - -
- -
+ +
); }; @@ -38,7 +88,7 @@ const KeymapConfigScreen = (props: { theme: Object }) => { const TopActions = (props: { theme: any, filter: string, - setFilter: Function + setFilter: (filter: string) => void }) => { const theme = themeStyle(props.theme); const topActionsStyle = { @@ -64,8 +114,9 @@ const TopActions = (props: { style={filterInputStyle} placeholder={_('Search..')} value={props.filter} - onChange={e => { props.setFilter(e.target.value); }} + onChange={e => props.setFilter(e.target.value)} /> + @@ -77,7 +128,10 @@ const TopActions = (props: { }; const KeymapTable = (props: { - theme: any + theme: any, + keymap: KeymapItem[], + updateAccelerator: (commandName: string, accelerator: string) => void + updateIsEditing: (commandName: string, isEditing: boolean) => void }) => { const theme = themeStyle(props.theme); const tableStyle = { @@ -87,33 +141,46 @@ const KeymapTable = (props: { width: '100%', }; + const tableRows = props.keymap.map(item => + + {item.label} + + { + item.isEditing ? + props.updateAccelerator(item.command, accelerator)} + theme={props.theme} + /> + : +
props.updateIsEditing(item.command, true)}> + {item.accelerator.length ? item.accelerator : _('Disabled')} +
+ } + + + ); + return ( - + - - - - - - - - + {...tableRows}
{_('Command')}{_('Shortcut')}{_('Keyboard Shortcut')}
{_('New note')}{_('Ctrl+N')}
{_('New todo')}{_('Ctrl+T')}
); }; const ShortcutRecorder = (props: { - handleKeydown: (e: React.KeyboardEvent) => void, - accelerator: string, + updateAccelerator: (accelerator: string) => void, theme: any, }) => { + const [newAccelerator, setNewAccelerator] = React.useState(''); + const theme = themeStyle(props.theme); const inlineButtonStyle = { ...theme.buttonStyle, @@ -122,72 +189,41 @@ const ShortcutRecorder = (props: { marginLeft: 12, }; + + const handleKeydown = ( + e: React.KeyboardEvent + ) => { + e.preventDefault(); + + const _newAccelerator = DOMtoElectronAccelerator(e); + switch (_newAccelerator) { + case 'Enter': + return props.updateAccelerator(newAccelerator); + case 'Escape': + return alert('Toggle isEditing'); + default: + setNewAccelerator(_newAccelerator); + } + }; + return (
-
); }; -const DOMtoElectronAccelerator = (e: React.KeyboardEvent) => { - const { key, ctrlKey, metaKey, altKey, shiftKey } = e; - const modifiersPresent = ctrlKey || metaKey || altKey || shiftKey; - - const parts = []; - if (modifiersPresent) { - // First, the modifiers - if (ctrlKey) parts.push('Ctrl'); - if (shim.isMac()) { - if (altKey) parts.push('Option'); - if (shiftKey) parts.push('Shift'); - if (metaKey) parts.push('Cmd'); - } else { - if (altKey) parts.push('Alt'); - if (shiftKey) parts.push('Shift'); - } - // Finally, the key - const _key = DOMtoElectronKey(key); - if (_key) parts.push(_key); - } else if (key === 'Enter') { - alert('Save'); - } else if (key === 'Escape') { - alert('Discard'); - } else { - // Finally, the key - const _key = DOMtoElectronKey(key); - if (_key) parts.push(_key); - } - return parts.join('+'); -}; - -const DOMtoElectronKey = (key: string) => { - if (/^([A-Z0-9]|F1*[1-9]|F10|F2[0-4]|[`~!@#$%^&*()\-_=[\]\\{}|;':",./<>?]|Enter|Tab|Backspace|Delete|Insert|Home|End|PageUp|PageDown|Escape|MediaStop|MediaPlayPause|PrintScreen])$/.test(key)) return key; - if (/^([a-z])$/.test(key)) return key.toUpperCase(); - if (/^Arrow(Up|Down|Left|Right)|Audio(VolumeUp|VolumeDown|VolumeMute)$/.test(key)) return key.slice(5); - - switch (key) { - case ' ': - return 'Space'; - case '+': - return 'Plus'; - case 'MediaTrackNext': - return 'MediaNextTrack'; - case 'MediaTrackPrevious': - return 'MediaPreviousTrack'; - } - console.warn(`Ignoring ${key}`); - return null; -}; -export { KeymapConfigScreen, DOMtoElectronAccelerator, DOMtoElectronKey }; +export = KeymapConfigScreen; diff --git a/ReactNativeClient/lib/KeymapUtils.ts b/ReactNativeClient/lib/KeymapUtils.ts new file mode 100644 index 00000000000..4e1bada6104 --- /dev/null +++ b/ReactNativeClient/lib/KeymapUtils.ts @@ -0,0 +1,43 @@ +const { shim } = require('lib/shim'); + +export const DOMtoElectronAccelerator = (e: React.KeyboardEvent) => { + const parts = []; + const { key, ctrlKey, metaKey, altKey, shiftKey } = e; + + // First, the modifiers + if (ctrlKey) parts.push('Ctrl'); + if (shim.isMac()) { + if (altKey) parts.push('Option'); + if (shiftKey) parts.push('Shift'); + if (metaKey) parts.push('Cmd'); + } else { + if (altKey) parts.push('Alt'); + if (shiftKey) parts.push('Shift'); + } + + // Finally, the key + const _key = DOMtoElectronKey(key); + if (_key) parts.push(_key); + + return parts.join('+'); +}; + +export const DOMtoElectronKey = (key: string) => { + if (/^([A-Z0-9]|F1*[1-9]|F10|F2[0-4]|[`~!@#$%^&*()\-_=[\]\\{}|;':",./<>?]|Enter|Tab|Backspace|Delete|Insert|Home|End|PageUp|PageDown|Escape|MediaStop|MediaPlayPause|PrintScreen])$/.test(key)) return key; + if (/^([a-z])$/.test(key)) return key.toUpperCase(); + if (/^Arrow(Up|Down|Left|Right)|Audio(VolumeUp|VolumeDown|VolumeMute)$/.test(key)) return key.slice(5); + + switch (key) { + case ' ': + return 'Space'; + case '+': + return 'Plus'; + case 'MediaTrackNext': + return 'MediaNextTrack'; + case 'MediaTrackPrevious': + return 'MediaPreviousTrack'; + } + + console.warn(`Ignoring ${key}`); + return null; +}; diff --git a/ReactNativeClient/lib/services/CommandService.ts b/ReactNativeClient/lib/services/CommandService.ts index 26ebe2dd375..e5de9adda03 100644 --- a/ReactNativeClient/lib/services/CommandService.ts +++ b/ReactNativeClient/lib/services/CommandService.ts @@ -244,6 +244,12 @@ export default class CommandService extends BaseService { return command.runtime.title(command.runtime.props); } + label(commandName:string):string { + const command = this.commandByName(commandName); + if (!command) return null; + return command.declaration.label(); + } + private extractExecuteArgs(command:Command, executeArgs:any) { if (executeArgs) return executeArgs; if (!command.runtime) throw new Error(`Command: ${command.declaration.name}: Runtime is not defined - make sure it has been registered.`); From 5ba4c907037f47636075ca3b4ea42fd9eaa569de Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Fri, 24 Jul 2020 18:01:21 +0530 Subject: [PATCH 04/48] Remove KeymapUtils.ts in favor of including domToElectronKey, domToElectronAccelerator functions in KeymapService itself --- .eslintignore | 1 - .gitignore | 1 - ElectronClient/gui/KeymapConfigScreen.tsx | 93 ++++++++++++++----- ReactNativeClient/lib/KeymapUtils.ts | 43 --------- .../lib/services/CommandService.ts | 2 +- 5 files changed, 71 insertions(+), 69 deletions(-) delete mode 100644 ReactNativeClient/lib/KeymapUtils.ts diff --git a/.eslintignore b/.eslintignore index eaf8ca072a3..da3dbcbe7b1 100644 --- a/.eslintignore +++ b/.eslintignore @@ -147,7 +147,6 @@ ReactNativeClient/lib/joplin-renderer/MdToHtml/rules/fence.js ReactNativeClient/lib/joplin-renderer/MdToHtml/rules/mermaid.js ReactNativeClient/lib/joplin-renderer/MdToHtml/rules/sanitize_html.js ReactNativeClient/lib/JoplinServerApi.js -ReactNativeClient/lib/KeymapUtils.js ReactNativeClient/lib/services/CommandService.js ReactNativeClient/lib/services/keychain/KeychainService.js ReactNativeClient/lib/services/keychain/KeychainServiceDriver.dummy.js diff --git a/.gitignore b/.gitignore index 67edfdb77cb..9157a482b9e 100644 --- a/.gitignore +++ b/.gitignore @@ -138,7 +138,6 @@ ReactNativeClient/lib/joplin-renderer/MdToHtml/rules/fence.js ReactNativeClient/lib/joplin-renderer/MdToHtml/rules/mermaid.js ReactNativeClient/lib/joplin-renderer/MdToHtml/rules/sanitize_html.js ReactNativeClient/lib/JoplinServerApi.js -ReactNativeClient/lib/KeymapUtils.js ReactNativeClient/lib/services/CommandService.js ReactNativeClient/lib/services/keychain/KeychainService.js ReactNativeClient/lib/services/keychain/KeychainServiceDriver.dummy.js diff --git a/ElectronClient/gui/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfigScreen.tsx index 25ffc454634..c08abdb0668 100644 --- a/ElectronClient/gui/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfigScreen.tsx @@ -1,25 +1,13 @@ import * as React from 'react'; const CommandService = require('lib/services/CommandService').default; -const { DOMtoElectronAccelerator } = require('lib/KeymapUtils.js'); const { themeStyle } = require('lib/theme'); +const { shim } = require('lib/shim'); const { _ } = require('lib/locale.js'); // const { bridge } = require('electron').remote.require('./bridge'); -// const Setting = require('lib/models/Setting'); -interface KeymapItem { - label: string, - accelerator: string, - command: string - isEditing: boolean, -} - -// Temporary values -const commandNames = [ - 'textCopy', - 'textCut', - 'textPaste', -]; -const getAccelerator = (commandName: string) => { +// Placeholders for current KeymapService members +const KeymapService_keyCodes = /^([0-9A-Z)!@#$%^&*(:+<_>?~{|}";=,\-./`[\\\]']|F1*[1-9]|F10|F2[0-4]|Plus|Space|Tab|Backspace|Delete|Insert|Return|Enter|Up|Down|Left|Right|Home|End|PageUp|PageDown|Escape|Esc|VolumeUp|VolumeDown|VolumeMute|MediaNextTrack|MediaPreviousTrack|MediaStop|MediaPlayPause|PrintScreen)$/; +const KeymapService_instance_getAccelerator = (commandName: string) => { switch (commandName) { case 'textCopy': return 'Ctrl+C'; @@ -31,11 +19,72 @@ const getAccelerator = (commandName: string) => { throw new Error('Invalid command'); } }; +const KeymapService_instance_getCommands = () => [ + 'textCopy', + 'textCut', + 'textPaste', +]; + +// To be moved to KeymapService +const KeymapService_domToElectronAccelerator = (event: React.KeyboardEvent) => { + const parts = []; + const { key, ctrlKey, metaKey, altKey, shiftKey } = event; + + // First, the modifiers + if (ctrlKey) parts.push('Ctrl'); + if (shim.isMac()) { + if (altKey) parts.push('Option'); + if (shiftKey) parts.push('Shift'); + if (metaKey) parts.push('Cmd'); + } else { + if (altKey) parts.push('Alt'); + if (shiftKey) parts.push('Shift'); + } + + // Finally, the key + const _key = KeymapService_domToElectronKey(key); + if (_key) parts.push(_key); + + return parts.join('+'); +}; + +// To be moved to KeymapService +const KeymapService_domToElectronKey = (domKey: string) => { + let electronKey; + + if (/^([a-z])$/.test(domKey)) electronKey = domKey.toUpperCase(); + if (/^Arrow(Up|Down|Left|Right)|Audio(VolumeUp|VolumeDown|VolumeMute)$/.test(domKey)) electronKey = domKey.slice(5); + + switch (domKey) { + case ' ': + electronKey = 'Space'; + break; + case '+': + electronKey = 'Plus'; + break; + case 'MediaTrackNext': + electronKey = 'MediaNextTrack'; + break; + case 'MediaTrackPrevious': + electronKey = 'MediaPreviousTrack'; + break; + } + + if (KeymapService_keyCodes.test(electronKey)) return electronKey; + else return null; +}; -const initialKeymap = commandNames.map(commandName => { +interface KeymapItem { + label: string, + accelerator: string, + command: string + isEditing: boolean, +} + +const initialKeymap = KeymapService_instance_getCommands().map(commandName => { return { label: CommandService.instance().label(commandName), - accelerator: getAccelerator(commandName), + accelerator: KeymapService_instance_getAccelerator(commandName), command: commandName, isEditing: false, }; @@ -191,11 +240,11 @@ const ShortcutRecorder = (props: { const handleKeydown = ( - e: React.KeyboardEvent + event: React.KeyboardEvent ) => { - e.preventDefault(); + event.preventDefault(); - const _newAccelerator = DOMtoElectronAccelerator(e); + const _newAccelerator = KeymapService_domToElectronAccelerator(event); switch (_newAccelerator) { case 'Enter': return props.updateAccelerator(newAccelerator); @@ -224,6 +273,4 @@ const ShortcutRecorder = (props: { ); }; - - export = KeymapConfigScreen; diff --git a/ReactNativeClient/lib/KeymapUtils.ts b/ReactNativeClient/lib/KeymapUtils.ts deleted file mode 100644 index 4e1bada6104..00000000000 --- a/ReactNativeClient/lib/KeymapUtils.ts +++ /dev/null @@ -1,43 +0,0 @@ -const { shim } = require('lib/shim'); - -export const DOMtoElectronAccelerator = (e: React.KeyboardEvent) => { - const parts = []; - const { key, ctrlKey, metaKey, altKey, shiftKey } = e; - - // First, the modifiers - if (ctrlKey) parts.push('Ctrl'); - if (shim.isMac()) { - if (altKey) parts.push('Option'); - if (shiftKey) parts.push('Shift'); - if (metaKey) parts.push('Cmd'); - } else { - if (altKey) parts.push('Alt'); - if (shiftKey) parts.push('Shift'); - } - - // Finally, the key - const _key = DOMtoElectronKey(key); - if (_key) parts.push(_key); - - return parts.join('+'); -}; - -export const DOMtoElectronKey = (key: string) => { - if (/^([A-Z0-9]|F1*[1-9]|F10|F2[0-4]|[`~!@#$%^&*()\-_=[\]\\{}|;':",./<>?]|Enter|Tab|Backspace|Delete|Insert|Home|End|PageUp|PageDown|Escape|MediaStop|MediaPlayPause|PrintScreen])$/.test(key)) return key; - if (/^([a-z])$/.test(key)) return key.toUpperCase(); - if (/^Arrow(Up|Down|Left|Right)|Audio(VolumeUp|VolumeDown|VolumeMute)$/.test(key)) return key.slice(5); - - switch (key) { - case ' ': - return 'Space'; - case '+': - return 'Plus'; - case 'MediaTrackNext': - return 'MediaNextTrack'; - case 'MediaTrackPrevious': - return 'MediaPreviousTrack'; - } - - console.warn(`Ignoring ${key}`); - return null; -}; diff --git a/ReactNativeClient/lib/services/CommandService.ts b/ReactNativeClient/lib/services/CommandService.ts index e5de9adda03..e16b32c20ab 100644 --- a/ReactNativeClient/lib/services/CommandService.ts +++ b/ReactNativeClient/lib/services/CommandService.ts @@ -246,7 +246,7 @@ export default class CommandService extends BaseService { label(commandName:string):string { const command = this.commandByName(commandName); - if (!command) return null; + if (!command) throw new Error(`Command: ${commandName} is not declared`); return command.declaration.label(); } From 66c53752fb6a5cd3d2e0e0642158774d361137b1 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Sat, 25 Jul 2020 17:10:03 +0530 Subject: [PATCH 05/48] Refactor into subfolders --- .eslintignore | 4 +- .gitignore | 4 +- ElectronClient/gui/ConfigScreen.jsx | 2 +- .../gui/KeymapConfig/KeymapConfigScreen.tsx | 127 ++++++++ .../gui/KeymapConfig/ShortcutRecorder.tsx | 116 ++++++++ .../gui/KeymapConfig/styles/index.ts | 40 +++ ElectronClient/gui/KeymapConfigScreen.tsx | 276 ------------------ 7 files changed, 290 insertions(+), 279 deletions(-) create mode 100644 ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx create mode 100644 ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx create mode 100644 ElectronClient/gui/KeymapConfig/styles/index.ts delete mode 100644 ElectronClient/gui/KeymapConfigScreen.tsx diff --git a/.eslintignore b/.eslintignore index da3dbcbe7b1..292b6164055 100644 --- a/.eslintignore +++ b/.eslintignore @@ -67,7 +67,9 @@ ElectronClient/commands/stopExternalEditing.js ElectronClient/global.d.js ElectronClient/gui/ErrorBoundary.js ElectronClient/gui/Header/commands/focusSearch.js -ElectronClient/gui/KeymapConfigScreen.js +ElectronClient/gui/KeymapConfig/KeymapConfigScreen.js +ElectronClient/gui/KeymapConfig/ShortcutRecorder.js +ElectronClient/gui/KeymapConfig/styles/index.js ElectronClient/gui/MainScreen/commands/editAlarm.js ElectronClient/gui/MainScreen/commands/exportPdf.js ElectronClient/gui/MainScreen/commands/hideModalMessage.js diff --git a/.gitignore b/.gitignore index 9157a482b9e..f4f023d5360 100644 --- a/.gitignore +++ b/.gitignore @@ -58,7 +58,9 @@ ElectronClient/commands/stopExternalEditing.js ElectronClient/global.d.js ElectronClient/gui/ErrorBoundary.js ElectronClient/gui/Header/commands/focusSearch.js -ElectronClient/gui/KeymapConfigScreen.js +ElectronClient/gui/KeymapConfig/KeymapConfigScreen.js +ElectronClient/gui/KeymapConfig/ShortcutRecorder.js +ElectronClient/gui/KeymapConfig/styles/index.js ElectronClient/gui/MainScreen/commands/editAlarm.js ElectronClient/gui/MainScreen/commands/exportPdf.js ElectronClient/gui/MainScreen/commands/hideModalMessage.js diff --git a/ElectronClient/gui/ConfigScreen.jsx b/ElectronClient/gui/ConfigScreen.jsx index f819f890441..ad50505b847 100644 --- a/ElectronClient/gui/ConfigScreen.jsx +++ b/ElectronClient/gui/ConfigScreen.jsx @@ -10,7 +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('./KeymapConfigScreen.js'); +const { KeymapConfigScreen } = require('./KeymapConfig/KeymapConfigScreen'); class ConfigScreenComponent extends React.Component { constructor() { diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx new file mode 100644 index 00000000000..e53d012fdfd --- /dev/null +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -0,0 +1,127 @@ +import * as React from 'react'; +import { useState } from 'react'; + +import styles_ from './styles'; +import { ShortcutRecorder } from './ShortcutRecorder'; +const CommandService = require('lib/services/CommandService').default; +const { _ } = require('lib/locale.js'); + +export interface KeymapConfigScreenProps { + theme: number +} + +interface KeymapItem { + label: string, + accelerator: string, + command: string, + isEditing: boolean, +} + +export const KeymapConfigScreen = (props: KeymapConfigScreenProps) => { + const styles = styles_(props); + + const [filter, setFilter] = useState(''); + const [keymap, setKeymap] = useState( + () => KeymapService_instance_getCommands().map(commandName => { + return { + label: CommandService.instance().label(commandName), + accelerator: KeymapService_instance_getAccelerator(commandName), + command: commandName, + isEditing: false, + }; + }) + ); + + const setAccelerator = (commandName: string, accelerator: string) => { + setKeymap(prevKeymap => { + const newKeymap = [...prevKeymap]; + + newKeymap.find(item => item.command === commandName).accelerator = accelerator; + return newKeymap; + }); + }; + + const setIsEditing = (commandName: string, isEditing: boolean) => { + setKeymap(prevKeymap => { + const newKeymap = [...prevKeymap]; + + newKeymap.find(item => item.command === commandName).isEditing = isEditing; + return newKeymap; + }); + }; + + const renderKeymapRow = (item: KeymapItem) => { + const handleClick = () => setIsEditing(item.command, true); + const cellContent = item.isEditing ? + setAccelerator(item.command, accelerator)} + updateIsEditing={(isEditing: boolean) => setIsEditing(item.command, isEditing)} + theme={props.theme} + /> :
+ {item.accelerator.length ? item.accelerator : _('Disabled')} +
; + + return ( + + {item.label} + + {cellContent} + + + ); + }; + + return ( +
+
+ setFilter(event.target.value)} + placeholder={_('Search..')} + style={styles.filterInputStyle} + /> + + + + +
+ + + + + + + + + + {keymap.map(item => renderKeymapRow(item))} + +
{_('Command')}{_('Keyboard Shortcut')}
+
+ ); +}; + +// Placeholder +const KeymapService_instance_getAccelerator = (commandName: string) => { + switch (commandName) { + case 'textCopy': + return 'Ctrl+C'; + case 'textCut': + return 'Ctrl+X'; + case 'textPaste': + return 'Ctrl+V'; + default: + throw new Error('Invalid command'); + } +}; + +// Placeholder +const KeymapService_instance_getCommands = () => [ + 'textCopy', + 'textCut', + 'textPaste', +]; diff --git a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx new file mode 100644 index 00000000000..1651dea4a08 --- /dev/null +++ b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx @@ -0,0 +1,116 @@ +import * as React from 'react'; +import { useState, KeyboardEvent } from 'react'; + +import styles_ from './styles'; +import { KeymapConfigScreenProps } from './KeymapConfigScreen'; +const { shim } = require('lib/shim'); +const { _ } = require('lib/locale.js'); + +export interface ShortcutRecorderProps extends KeymapConfigScreenProps { + updateAccelerator: (accelerator: string) => void, + updateIsEditing: (isEditing: boolean) => void, +} + +export const ShortcutRecorder = (props: ShortcutRecorderProps) => { + const styles = styles_(props); + const [accelerator, setAccelerator] = useState(''); + + const handleKeydown = (event: KeyboardEvent) => { + event.preventDefault(); + const newAccelerator = KeymapService_domToElectronAccelerator(event); + + switch (newAccelerator) { + case 'Enter': + props.updateAccelerator(accelerator); + props.updateIsEditing(false); + break; + case 'Escape': + return props.updateIsEditing(false); + default: + setAccelerator(newAccelerator); + } + }; + + const handleRestore = () => { + console.warn('handleRestore'); + + props.updateIsEditing(false); + }; + + const handleBlur = () => { + // Use a reference element to ignore blur if still focused on current row + // Otherwise it wouldn't be possible to click on "Default" button + + // props.updateIsEditing(false); + }; + + return ( +
+ + + +
+ ); +}; + +// Placeholder +const KeymapService_keyCodes = /^([0-9A-Z)!@#$%^&*(:+<_>?~{|}";=,\-./`[\\\]']|F1*[1-9]|F10|F2[0-4]|Plus|Space|Tab|Backspace|Delete|Insert|Return|Enter|Up|Down|Left|Right|Home|End|PageUp|PageDown|Escape|Esc|VolumeUp|VolumeDown|VolumeMute|MediaNextTrack|MediaPreviousTrack|MediaStop|MediaPlayPause|PrintScreen)$/; + +// Will be moved to KeymapService +const KeymapService_domToElectronAccelerator = (event: KeyboardEvent) => { + const parts = []; + const { key, ctrlKey, metaKey, altKey, shiftKey } = event; + + // First, the modifiers + if (ctrlKey) parts.push('Ctrl'); + if (shim.isMac()) { + if (altKey) parts.push('Option'); + if (shiftKey) parts.push('Shift'); + if (metaKey) parts.push('Cmd'); + } else { + if (altKey) parts.push('Alt'); + if (shiftKey) parts.push('Shift'); + } + + // Finally, the key + const _key = KeymapService_domToElectronKey(key); + if (_key) parts.push(_key); + + return parts.join('+'); +}; + +// Will be moved to KeymapService +const KeymapService_domToElectronKey = (domKey: string) => { + let electronKey; + + if (/^([a-z])$/.test(domKey)) { electronKey = domKey.toUpperCase(); } else if (/^Arrow(Up|Down|Left|Right)|Audio(VolumeUp|VolumeDown|VolumeMute)$/.test(domKey)) { electronKey = domKey.slice(5); } else { + switch (domKey) { + case ' ': + electronKey = 'Space'; + break; + case '+': + electronKey = 'Plus'; + break; + case 'MediaTrackNext': + electronKey = 'MediaNextTrack'; + break; + case 'MediaTrackPrevious': + electronKey = 'MediaPreviousTrack'; + break; + default: + electronKey = domKey; + } + } + + if (KeymapService_keyCodes.test(electronKey)) return electronKey; + else return null; +}; diff --git a/ElectronClient/gui/KeymapConfig/styles/index.ts b/ElectronClient/gui/KeymapConfig/styles/index.ts new file mode 100644 index 00000000000..ff58e2096dc --- /dev/null +++ b/ElectronClient/gui/KeymapConfig/styles/index.ts @@ -0,0 +1,40 @@ +import { KeymapConfigScreenProps } from '../KeymapConfigScreen'; + +const { buildStyle } = require('lib/theme'); + +export default function styles(props: KeymapConfigScreenProps) { + return buildStyle('KeymapConfigScreen', props.theme, (theme: any) => { + return { + containerStyle: { + ...theme.containerStyle, + padding: 10, + overflow: 'auto', + }, + topActionsStyle: { + ...theme.containerStyle, + marginTop: 10, + display: 'flex', + flexDirection: 'row', + }, + inlineButtonStyle: { + ...theme.buttonStyle, + height: theme.inputStyle.height, + padding: 0, + marginLeft: 12, + }, + filterInputStyle: { + ...theme.inputStyle, + flexGrow: 1, + }, + tableStyle: { + ...theme.containerStyle, + marginTop: 10, + overflow: 'auto', + width: '100%', + }, + textStyle: { + ...theme.textStyle, + }, + }; + }); +} diff --git a/ElectronClient/gui/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfigScreen.tsx deleted file mode 100644 index c08abdb0668..00000000000 --- a/ElectronClient/gui/KeymapConfigScreen.tsx +++ /dev/null @@ -1,276 +0,0 @@ -import * as React from 'react'; -const CommandService = require('lib/services/CommandService').default; -const { themeStyle } = require('lib/theme'); -const { shim } = require('lib/shim'); -const { _ } = require('lib/locale.js'); -// const { bridge } = require('electron').remote.require('./bridge'); - -// Placeholders for current KeymapService members -const KeymapService_keyCodes = /^([0-9A-Z)!@#$%^&*(:+<_>?~{|}";=,\-./`[\\\]']|F1*[1-9]|F10|F2[0-4]|Plus|Space|Tab|Backspace|Delete|Insert|Return|Enter|Up|Down|Left|Right|Home|End|PageUp|PageDown|Escape|Esc|VolumeUp|VolumeDown|VolumeMute|MediaNextTrack|MediaPreviousTrack|MediaStop|MediaPlayPause|PrintScreen)$/; -const KeymapService_instance_getAccelerator = (commandName: string) => { - switch (commandName) { - case 'textCopy': - return 'Ctrl+C'; - case 'textCut': - return 'Ctrl+X'; - case 'textPaste': - return 'Ctrl+V'; - default: - throw new Error('Invalid command'); - } -}; -const KeymapService_instance_getCommands = () => [ - 'textCopy', - 'textCut', - 'textPaste', -]; - -// To be moved to KeymapService -const KeymapService_domToElectronAccelerator = (event: React.KeyboardEvent) => { - const parts = []; - const { key, ctrlKey, metaKey, altKey, shiftKey } = event; - - // First, the modifiers - if (ctrlKey) parts.push('Ctrl'); - if (shim.isMac()) { - if (altKey) parts.push('Option'); - if (shiftKey) parts.push('Shift'); - if (metaKey) parts.push('Cmd'); - } else { - if (altKey) parts.push('Alt'); - if (shiftKey) parts.push('Shift'); - } - - // Finally, the key - const _key = KeymapService_domToElectronKey(key); - if (_key) parts.push(_key); - - return parts.join('+'); -}; - -// To be moved to KeymapService -const KeymapService_domToElectronKey = (domKey: string) => { - let electronKey; - - if (/^([a-z])$/.test(domKey)) electronKey = domKey.toUpperCase(); - if (/^Arrow(Up|Down|Left|Right)|Audio(VolumeUp|VolumeDown|VolumeMute)$/.test(domKey)) electronKey = domKey.slice(5); - - switch (domKey) { - case ' ': - electronKey = 'Space'; - break; - case '+': - electronKey = 'Plus'; - break; - case 'MediaTrackNext': - electronKey = 'MediaNextTrack'; - break; - case 'MediaTrackPrevious': - electronKey = 'MediaPreviousTrack'; - break; - } - - if (KeymapService_keyCodes.test(electronKey)) return electronKey; - else return null; -}; - -interface KeymapItem { - label: string, - accelerator: string, - command: string - isEditing: boolean, -} - -const initialKeymap = KeymapService_instance_getCommands().map(commandName => { - return { - label: CommandService.instance().label(commandName), - accelerator: KeymapService_instance_getAccelerator(commandName), - command: commandName, - isEditing: false, - }; -}); - -const KeymapConfigScreen = (props: { theme: Object }) => { - const [keymap, setKeymap] = React.useState([...initialKeymap]); - const [filter, setFilter] = React.useState(''); - - const theme = themeStyle(props.theme); - const containerStyle = { - ...theme.containerStyle, - padding: 10, - overflow: 'auto', - }; - - const updateAccelerator = (commandName: string, accelerator: string) => { - const _keymap = [...keymap]; - - _keymap.find(item => item.command === commandName).accelerator = accelerator; - _keymap.find(item => item.command === commandName).isEditing = false; - setKeymap(_keymap); - }; - - const updateIsEditing = (commandName: string, isEditing: boolean) => { - const _keymap = [...keymap]; - - _keymap.forEach(item => item.isEditing = false); - _keymap.find(item => item.command === commandName).isEditing = isEditing; - setKeymap(_keymap); - }; - - return ( -
- - -
- ); -}; - -const TopActions = (props: { - theme: any, - filter: string, - setFilter: (filter: string) => void -}) => { - const theme = themeStyle(props.theme); - const topActionsStyle = { - ...theme.containerStyle, - marginTop: 10, - display: 'flex', - flexDirection: 'row', - }; - const inlineButtonStyle = { - ...theme.buttonStyle, - height: theme.inputStyle.height, - padding: 0, - marginLeft: 12, - }; - const filterInputStyle = { - ...theme.inputStyle, - flexGrow: 1, - }; - - return ( -
- props.setFilter(e.target.value)} - /> - - - -
- ); -}; - -const KeymapTable = (props: { - theme: any, - keymap: KeymapItem[], - updateAccelerator: (commandName: string, accelerator: string) => void - updateIsEditing: (commandName: string, isEditing: boolean) => void -}) => { - const theme = themeStyle(props.theme); - const tableStyle = { - ...theme.containerStyle, - marginTop: 10, - overflow: 'auto', - width: '100%', - }; - - const tableRows = props.keymap.map(item => - - {item.label} - - { - item.isEditing ? - props.updateAccelerator(item.command, accelerator)} - theme={props.theme} - /> - : -
props.updateIsEditing(item.command, true)}> - {item.accelerator.length ? item.accelerator : _('Disabled')} -
- } - - - ); - - return ( - - - - - - - - - {...tableRows} - -
{_('Command')}{_('Keyboard Shortcut')}
- ); -}; - -const ShortcutRecorder = (props: { - updateAccelerator: (accelerator: string) => void, - theme: any, -}) => { - const [newAccelerator, setNewAccelerator] = React.useState(''); - - const theme = themeStyle(props.theme); - const inlineButtonStyle = { - ...theme.buttonStyle, - height: theme.inputStyle.height, - padding: 0, - marginLeft: 12, - }; - - - const handleKeydown = ( - event: React.KeyboardEvent - ) => { - event.preventDefault(); - - const _newAccelerator = KeymapService_domToElectronAccelerator(event); - switch (_newAccelerator) { - case 'Enter': - return props.updateAccelerator(newAccelerator); - case 'Escape': - return alert('Toggle isEditing'); - default: - setNewAccelerator(_newAccelerator); - } - }; - - return ( -
- - - -
- ); -}; - -export = KeymapConfigScreen; From 5a46b5d9c76ce70cb197bcf4bdbf2ab679afc652 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Sun, 2 Aug 2020 09:35:35 +0530 Subject: [PATCH 06/48] Improve styling --- .../gui/KeymapConfig/KeymapConfigScreen.tsx | 12 +++++----- .../gui/KeymapConfig/ShortcutRecorder.tsx | 5 +++- .../gui/KeymapConfig/styles/index.ts | 23 ++++++++++++++----- 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index e53d012fdfd..bf63e7fbebe 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -62,9 +62,9 @@ export const KeymapConfigScreen = (props: KeymapConfigScreenProps) => {
; return ( - - {item.label} - + + {item.label} + {cellContent} @@ -92,9 +92,9 @@ export const KeymapConfigScreen = (props: KeymapConfigScreenProps) => { - - - + + + diff --git a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx index 1651dea4a08..687a43f45ae 100644 --- a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx +++ b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx @@ -44,15 +44,18 @@ export const ShortcutRecorder = (props: ShortcutRecorderProps) => { // props.updateIsEditing(false); }; + const placeholderText = _('Press the shortcut and hit Enter'); + return (
- - + + ); }; + const renderErrorMessage = (errorMessage: string) => { + if (errorMessage.length) { + return ( +
+

+ {errorMessage} +

+
+ ); + } else { return null; } + }; + return ( -
-
- setFilter(event.target.value)} - placeholder={_('Search..')} - style={styles.filterInputStyle} - /> - - - - + <> + {renderErrorMessage(errorMessage)} + +
+
+ setFilter(event.target.value)} + placeholder={_('Search...')} + style={styles.filterInput} + /> +
+
{_('Command')}{_('Keyboard Shortcut')}
{_('Command')}{_('Keyboard Shortcut')}
{item.label} +
{commandLabels[command]} {cellContent}
+ + + + + + + + {keymap.map(item => renderKeymapRow(item))} + +
{_('Command')}{_('Keyboard Shortcut')}
- - - - - - - - - - {keymap.map(item => renderKeymapRow(item))} - -
{_('Command')}{_('Keyboard Shortcut')}
- + ); }; -// Placeholder -const KeymapService_instance_getAccelerator = (commandName: string) => { - switch (commandName) { - case 'textCopy': - return 'Ctrl+C'; - case 'textCut': - return 'Ctrl+X'; - case 'textPaste': - return 'Ctrl+V'; - default: - throw new Error('Invalid command'); - } -}; - -// Placeholder -const KeymapService_instance_getCommands = () => [ - 'textCopy', - 'textCut', - 'textPaste', -]; diff --git a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx index 687a43f45ae..d6b767bc4af 100644 --- a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx +++ b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx @@ -2,118 +2,64 @@ import * as React from 'react'; import { useState, KeyboardEvent } from 'react'; import styles_ from './styles'; +import KeymapService from '../../lib/services/KeymapService'; import { KeymapConfigScreenProps } from './KeymapConfigScreen'; -const { shim } = require('lib/shim'); + const { _ } = require('lib/locale.js'); +const keymapService = KeymapService.instance(); export interface ShortcutRecorderProps extends KeymapConfigScreenProps { - updateAccelerator: (accelerator: string) => void, - updateIsEditing: (isEditing: boolean) => void, + setAccelerator: (accelerator: string) => void, + resetAccelerator: () => void, + toggleEditing: () => void, } -export const ShortcutRecorder = (props: ShortcutRecorderProps) => { - const styles = styles_(props); - const [accelerator, setAccelerator] = useState(''); +export const ShortcutRecorder = ({ setAccelerator, resetAccelerator, toggleEditing, theme }: ShortcutRecorderProps) => { + const styles = styles_(theme); + const [newAccelerator, setNewAccelerator] = useState(''); const handleKeydown = (event: KeyboardEvent) => { event.preventDefault(); - const newAccelerator = KeymapService_domToElectronAccelerator(event); + const newAccelerator = keymapService.domToElectronAccelerator(event); switch (newAccelerator) { case 'Enter': - props.updateAccelerator(accelerator); - props.updateIsEditing(false); - break; + return handleSave(); case 'Escape': - return props.updateIsEditing(false); + return handleReset(); default: - setAccelerator(newAccelerator); + setNewAccelerator(newAccelerator); } }; - const handleRestore = () => { - console.warn('handleRestore'); - - props.updateIsEditing(false); + const handleReset = () => { + resetAccelerator(); + toggleEditing(); }; - const handleBlur = () => { - // Use a reference element to ignore blur if still focused on current row - // Otherwise it wouldn't be possible to click on "Default" button - - // props.updateIsEditing(false); + const handleSave = () => { + setAccelerator(newAccelerator); + toggleEditing(); }; - const placeholderText = _('Press the shortcut and hit Enter'); - return ( -
+
- + +
); }; - -// Placeholder -const KeymapService_keyCodes = /^([0-9A-Z)!@#$%^&*(:+<_>?~{|}";=,\-./`[\\\]']|F1*[1-9]|F10|F2[0-4]|Plus|Space|Tab|Backspace|Delete|Insert|Return|Enter|Up|Down|Left|Right|Home|End|PageUp|PageDown|Escape|Esc|VolumeUp|VolumeDown|VolumeMute|MediaNextTrack|MediaPreviousTrack|MediaStop|MediaPlayPause|PrintScreen)$/; - -// Will be moved to KeymapService -const KeymapService_domToElectronAccelerator = (event: KeyboardEvent) => { - const parts = []; - const { key, ctrlKey, metaKey, altKey, shiftKey } = event; - - // First, the modifiers - if (ctrlKey) parts.push('Ctrl'); - if (shim.isMac()) { - if (altKey) parts.push('Option'); - if (shiftKey) parts.push('Shift'); - if (metaKey) parts.push('Cmd'); - } else { - if (altKey) parts.push('Alt'); - if (shiftKey) parts.push('Shift'); - } - - // Finally, the key - const _key = KeymapService_domToElectronKey(key); - if (_key) parts.push(_key); - - return parts.join('+'); -}; - -// Will be moved to KeymapService -const KeymapService_domToElectronKey = (domKey: string) => { - let electronKey; - - if (/^([a-z])$/.test(domKey)) { electronKey = domKey.toUpperCase(); } else if (/^Arrow(Up|Down|Left|Right)|Audio(VolumeUp|VolumeDown|VolumeMute)$/.test(domKey)) { electronKey = domKey.slice(5); } else { - switch (domKey) { - case ' ': - electronKey = 'Space'; - break; - case '+': - electronKey = 'Plus'; - break; - case 'MediaTrackNext': - electronKey = 'MediaNextTrack'; - break; - case 'MediaTrackPrevious': - electronKey = 'MediaPreviousTrack'; - break; - default: - electronKey = domKey; - } - } - - if (KeymapService_keyCodes.test(electronKey)) return electronKey; - else return null; -}; diff --git a/ElectronClient/gui/KeymapConfig/styles/index.ts b/ElectronClient/gui/KeymapConfig/styles/index.ts index b2f815c2179..c89743dd7e8 100644 --- a/ElectronClient/gui/KeymapConfig/styles/index.ts +++ b/ElectronClient/gui/KeymapConfig/styles/index.ts @@ -1,51 +1,55 @@ -import { KeymapConfigScreenProps } from '../KeymapConfigScreen'; - const { buildStyle } = require('lib/theme'); -export default function styles(props: KeymapConfigScreenProps) { - return buildStyle('KeymapConfigScreen', props.theme, (theme: any) => { +export default function styles(theme: number) { + return buildStyle('KeymapConfigScreen', theme, (theme: any) => { return { - containerStyle: { + container: { ...theme.containerStyle, - padding: 15, + padding: 16, }, - topActionsStyle: { + topActions: { display: 'flex', flexDirection: 'row', }, - inlineButtonStyle: { - ...theme.buttonStyle, - padding: 0, - marginLeft: 12, + text: { + ...theme.textStyle, }, - filterInputStyle: { + input: { + ...theme.inputStyle, + }, + filterInput: { ...theme.inputStyle, flexGrow: 1, minHeight: theme.buttonStyle.minHeight, }, - tableStyle: { + inlineButton: { + ...theme.buttonStyle, + marginLeft: 12, + }, + warning: { + backgroundColor: theme.warningBackgroundColor, + paddingLeft: 10, + paddingRight: 10, + paddingTop: 2, + paddingBottom: 2, + }, + table: { ...theme.containerStyle, marginTop: 15, overflow: 'auto', width: '100%', }, - tableShortcutColumnStyle: { + tableShortcutColumn: { ...theme.textStyle, width: '65%', }, - tableCommandColumnStyle: { + tableCommandColumn: { ...theme.textStyle, width: 'auto', }, - tableRowStyle: { + tableRow: { minHeight: 25, }, - textStyle: { - ...theme.textStyle, - }, - inputStyle: { - ...theme.inputStyle, - }, }; }); } diff --git a/ReactNativeClient/lib/services/KeymapService.ts b/ReactNativeClient/lib/services/KeymapService.ts index 8e145082812..c75e0373247 100644 --- a/ReactNativeClient/lib/services/KeymapService.ts +++ b/ReactNativeClient/lib/services/KeymapService.ts @@ -1,3 +1,5 @@ +import { KeyboardEvent } from 'react'; + const fs = require('fs-extra'); const BaseService = require('lib/services/BaseService'); const { shim } = require('lib/shim.js'); @@ -87,16 +89,20 @@ interface Keymap { export default class KeymapService extends BaseService { private keymap: Keymap; private defaultKeymap: KeymapItem[]; + private platform: string; + private keymapPath: string; + private refreshMenu: Function; constructor() { super(); - // Automatically initialized for the current platform + // Automatically initialize for the current platform this.initialize(); } initialize(platform: string = shim.platformName()) { - this.keysRegExp = keysRegExp; + this.platform = platform; + switch (platform) { case 'darwin': this.defaultKeymap = defaultKeymap.darwin; @@ -116,7 +122,7 @@ export default class KeymapService extends BaseService { } async loadKeymap(keymapPath: string) { - this.keymapPath = keymapPath; // Used for saving changes later.. + this.keymapPath = keymapPath; // For saving the changes later if (await fs.exists(keymapPath)) { this.logger().info(`Loading keymap: ${keymapPath}`); @@ -131,6 +137,17 @@ export default class KeymapService extends BaseService { } } + async saveKeymap() { + this.logger().info(`Saving keymap: ${this.keymapPath}`); + + try { + const customKeymap = this.generateCustomKeymap(); + await fs.writeFile(this.keymapPath, JSON.stringify(customKeymap, null, 2)); + } catch (err) { + throw new Error(`Failed to save keymap: ${this.keymapPath}\n${err}`); + } + } + hasAccelerator(command: string) { return !!this.keymap[command]; } @@ -153,6 +170,13 @@ export default class KeymapService extends BaseService { else this.setAccelerator(command, defaultItem.accelerator); } + getDefaultAccelerator(command: string) { + const defaultItem = this.defaultKeymap.find((item => item.command === command)); + + if (!defaultItem) throw new Error(`KeymapService: "${command}" command does not exist!`); + else return defaultItem.accelerator; + } + setKeymap(customKeymap: KeymapItem[]) { for (let i = 0; i < customKeymap.length; i++) { const item = customKeymap[i]; @@ -168,11 +192,39 @@ export default class KeymapService extends BaseService { try { this.validateKeymap(); // Throws whenever there are duplicate Accelerators used in the keymap } catch (err) { - this.initialize(); - throw new Error(`Keymap configuration contains duplicates\n${err.message}`); + this.initialize(); // Fallback to default keymap configuration + throw new Error(`Keymap configuration contains duplicates. \n${err.message}`); } } + getKeymap() { + return Object.values(this.keymap); + } + + getCommands() { + return Object.keys(this.keymap); + } + + setMenuRefreshCallback(callback: Function) { + this.refreshMenu = callback; + } + + triggerMenuRefresh() { + this.refreshMenu(); + } + + generateCustomKeymap() { + const customKeymap: KeymapItem[] = []; + this.defaultKeymap.forEach(({ command, accelerator }) => { + const currentAccelerator = this.getAccelerator(command); + if (this.getAccelerator(command) !== accelerator) { + customKeymap.push({ command, accelerator: currentAccelerator }); + } + }); + + return customKeymap; + } + private validateKeymapItem(item: KeymapItem) { if (!item.hasOwnProperty('command')) { throw new Error('"command" property is missing'); @@ -186,7 +238,7 @@ export default class KeymapService extends BaseService { try { this.validateAccelerator(item.accelerator); } catch (err) { - throw new Error(`"${item.accelerator}" is not a valid accelerator`); + throw new Error(`"${item.accelerator}" is not a valid accelerator.`); } } } @@ -200,8 +252,8 @@ export default class KeymapService extends BaseService { if (usedAccelerators.has(itemAccelerator)) { const originalItem = Object.values(this.keymap).find(_item => _item.accelerator == item.accelerator); throw new Error( - `Accelerator "${itemAccelerator}" can't be used for both "${item.command}" and "${originalItem.command}" commands\n` + - 'You have to change the accelerator for any of above commands' + `Accelerator "${itemAccelerator}" can't be used for both "${item.command}" and "${originalItem.command}" commands. \n` + + 'You have to change the accelerator for any of above commands.' ); } else if (itemAccelerator !== null) { usedAccelerators.add(itemAccelerator); @@ -214,7 +266,7 @@ export default class KeymapService extends BaseService { const parts = accelerator.split('+'); const isValid = parts.every((part, index) => { - const isKey = this.keysRegExp.test(part); + const isKey = keysRegExp.test(part); const isModifier = this.modifiersRegExp.test(part); if (isKey) { @@ -231,6 +283,60 @@ export default class KeymapService extends BaseService { if (!isValid) throw new Error(`Accelerator invalid: ${accelerator}`); } + domToElectronAccelerator(event: KeyboardEvent) { + const parts = []; + const { key, ctrlKey, metaKey, altKey, shiftKey } = event; + + // First, the modifiers + if (ctrlKey) parts.push('Ctrl'); + switch (this.platform) { + case 'darwin': + if (altKey) parts.push('Option'); + if (shiftKey) parts.push('Shift'); + if (metaKey) parts.push('Cmd'); + break; + default: + if (altKey) parts.push('Alt'); + if (shiftKey) parts.push('Shift'); + } + + // Finally, the key + const electronKey = KeymapService.domToElectronKey(key); + if (electronKey) parts.push(electronKey); + + return parts.join('+'); + } + + static domToElectronKey(domKey: string) { + let electronKey; + + if (/^([a-z])$/.test(domKey)) { + electronKey = domKey.toUpperCase(); + } else if (/^Arrow(Up|Down|Left|Right)|Audio(VolumeUp|VolumeDown|VolumeMute)$/.test(domKey)) { + electronKey = domKey.slice(5); + } else { + switch (domKey) { + case ' ': + electronKey = 'Space'; + break; + case '+': + electronKey = 'Plus'; + break; + case 'MediaTrackNext': + electronKey = 'MediaNextTrack'; + break; + case 'MediaTrackPrevious': + electronKey = 'MediaPreviousTrack'; + break; + default: + electronKey = domKey; + } + } + + if (keysRegExp.test(electronKey)) return electronKey; + else return null; + } + static instance() { if (this.instance_) return this.instance_; From fa08c285c08145e0be70aa506d8b49024cfe12e2 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Sat, 15 Aug 2020 14:35:07 +0530 Subject: [PATCH 08/48] Make compatible with macOS --- ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index 627013032f1..4cc9684f672 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -26,6 +26,8 @@ const commandLabels: CommandLabels = { config: _(shim.isMac() ? 'Preferences' : 'Options'), gotoAnything: _('Goto Anything...'), help: _('Website and documentation'), + hideApp: _('Hide Joplin'), + closeWindow: _('Close Window'), }; // Obtain the rest of the labels keymapService.getCommands().forEach((commandName: string) => { From 57a68b9f2de376adad5b2e581647fe58551cc923 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Sat, 15 Aug 2020 15:05:33 +0530 Subject: [PATCH 09/48] Fix Goto Anything shortcut not updating --- ReactNativeClient/lib/services/PluginManager.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ReactNativeClient/lib/services/PluginManager.js b/ReactNativeClient/lib/services/PluginManager.js index 846df6082a3..58d2d66b6d5 100644 --- a/ReactNativeClient/lib/services/PluginManager.js +++ b/ReactNativeClient/lib/services/PluginManager.js @@ -1,4 +1,5 @@ const { Logger } = require('lib/logger.js'); +const KeymapService = require('lib/services/KeymapService.js').default; class PluginManager { constructor() { @@ -91,10 +92,7 @@ class PluginManager { itemName: item.name, }); }; - - if (item.accelerator instanceof Function) { - item.accelerator = item.accelerator(); - } + item.accelerator = KeymapService.instance().getAccelerator(name); } output = output.concat(menuItems); From de493e6b5a37d26bf613cda4c0ef9bfc5ddaed57 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Sat, 15 Aug 2020 15:12:48 +0530 Subject: [PATCH 10/48] Simple filtering --- ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index 4cc9684f672..6799268d35a 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -155,7 +155,10 @@ export const KeymapConfigScreen = ({ theme }: KeymapConfigScreenProps) => { - {keymap.map(item => renderKeymapRow(item))} + {keymap.filter(({ command }) => { + const filterLowerCase = filter.toLowerCase(); + return (command.toLowerCase().includes(filterLowerCase) || commandLabels[command].toLowerCase().includes(filterLowerCase)); + }).map(item => renderKeymapRow(item))}
From 51bc13f8e26e9b22f0925418feab57fda1e14774 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Sat, 15 Aug 2020 18:44:14 +0530 Subject: [PATCH 11/48] Fix false warning when disabling a shortcut --- ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx | 2 +- ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index 6799268d35a..0cc352ecf7c 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -109,7 +109,7 @@ export const KeymapConfigScreen = ({ theme }: KeymapConfigScreenProps) => { theme={theme} /> :
- {accelerator.length ? accelerator : _('Disabled')} + {accelerator || _('Disabled')}
; return ( diff --git a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx index d6b767bc4af..d3c33a6c41a 100644 --- a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx +++ b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx @@ -38,7 +38,7 @@ export const ShortcutRecorder = ({ setAccelerator, resetAccelerator, toggleEditi }; const handleSave = () => { - setAccelerator(newAccelerator); + setAccelerator(newAccelerator.length ? newAccelerator : null); toggleEditing(); }; From 52dbc6e5d1e61ac61970a51c9027b688384da993 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Thu, 20 Aug 2020 15:52:15 +0530 Subject: [PATCH 12/48] Fix command labels not translating --- .eslintignore | 1 + .gitignore | 1 + .../gui/KeymapConfig/KeymapConfigScreen.tsx | 32 +++--------------- .../gui/KeymapConfig/utils/getLabel.ts | 33 +++++++++++++++++++ 4 files changed, 40 insertions(+), 27 deletions(-) create mode 100644 ElectronClient/gui/KeymapConfig/utils/getLabel.ts diff --git a/.eslintignore b/.eslintignore index 246a1383168..9fc7a7c674d 100644 --- a/.eslintignore +++ b/.eslintignore @@ -74,6 +74,7 @@ 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/MainScreen/commands/editAlarm.js ElectronClient/gui/MainScreen/commands/exportPdf.js ElectronClient/gui/MainScreen/commands/hideModalMessage.js diff --git a/.gitignore b/.gitignore index ea188484e57..f8c31512c75 100644 --- a/.gitignore +++ b/.gitignore @@ -65,6 +65,7 @@ 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/MainScreen/commands/editAlarm.js ElectronClient/gui/MainScreen/commands/exportPdf.js ElectronClient/gui/MainScreen/commands/hideModalMessage.js diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index 0cc352ecf7c..56503df5230 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -3,36 +3,12 @@ import { useState, useEffect } from 'react'; import styles_ from './styles'; import { ShortcutRecorder } from './ShortcutRecorder'; -import CommandService from '../../lib/services/CommandService'; import KeymapService, { KeymapItem } from '../../lib/services/KeymapService'; +import getLabel from './utils/getLabel'; const { _ } = require('lib/locale.js'); -const { shim } = require('lib/shim.js'); const keymapService = KeymapService.instance(); -const commandService = CommandService.instance(); - -interface CommandLabels { - [commandName: string]: string -} - -// Some commands aren't actually registered in CommandService -// These hardcoded labels are used for those commands instead -const commandLabels: CommandLabels = { - quit: _('Quit'), - insertTemplate: _('Insert template'), - zoomActualSize: _('Actual Size'), - // Conditionally set the label because it's different in macOS and other platforms - config: _(shim.isMac() ? 'Preferences' : 'Options'), - gotoAnything: _('Goto Anything...'), - help: _('Website and documentation'), - hideApp: _('Hide Joplin'), - closeWindow: _('Close Window'), -}; -// Obtain the rest of the labels -keymapService.getCommands().forEach((commandName: string) => { - commandLabels[commandName] = commandLabels[commandName] || commandService.label(commandName); -}); interface CommandEditing { [commandName: string]: boolean @@ -114,7 +90,9 @@ export const KeymapConfigScreen = ({ theme }: KeymapConfigScreenProps) => { return ( - {commandLabels[command]} + + {getLabel(command)} + {cellContent} @@ -157,7 +135,7 @@ export const KeymapConfigScreen = ({ theme }: KeymapConfigScreenProps) => { {keymap.filter(({ command }) => { const filterLowerCase = filter.toLowerCase(); - return (command.toLowerCase().includes(filterLowerCase) || commandLabels[command].toLowerCase().includes(filterLowerCase)); + return (command.toLowerCase().includes(filterLowerCase) || getLabel(command).toLowerCase().includes(filterLowerCase)); }).map(item => renderKeymapRow(item))} diff --git a/ElectronClient/gui/KeymapConfig/utils/getLabel.ts b/ElectronClient/gui/KeymapConfig/utils/getLabel.ts new file mode 100644 index 00000000000..29145380b2c --- /dev/null +++ b/ElectronClient/gui/KeymapConfig/utils/getLabel.ts @@ -0,0 +1,33 @@ +import CommandService from '../../../lib/services/CommandService'; + +const { _ } = require('lib/locale.js'); +const { shim } = require('lib/shim.js'); + +const getLabel = (commandName: string) => { + try { + return CommandService.instance().label(commandName); + } catch (err) { + switch (commandName) { + case 'quit': + return _('Quit'); + case 'insertTemplate': + return _('Insert template'); + case 'zoomActualSize': + return _('Actual Size'); + case 'gotoAnything': + return _('Goto Anything...'); + case 'help': + return _('Website and documentation'); + case 'hideApp': + return _('Hide Joplin'); + case 'closeWindow': + return _('Close Window'); + case 'config': + return shim.isMac() ? _('Preferences') : _('Options'); + default: + throw err; + } + } +}; + +export default getLabel; From 0b309d96bcdc209a5308653282a10119efa07a50 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Thu, 20 Aug 2020 16:04:48 +0530 Subject: [PATCH 13/48] Rename property theme to themeId --- ElectronClient/gui/ConfigScreen.jsx | 2 +- ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx | 8 ++++---- ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx | 5 +++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/ElectronClient/gui/ConfigScreen.jsx b/ElectronClient/gui/ConfigScreen.jsx index ad50505b847..6182bb91141 100644 --- a/ElectronClient/gui/ConfigScreen.jsx +++ b/ElectronClient/gui/ConfigScreen.jsx @@ -69,7 +69,7 @@ class ConfigScreenComponent extends React.Component { screenFromName(screenName) { if (screenName === 'encryption') return ; if (screenName === 'server') return ; - if (screenName === 'keymap') return ; + if (screenName === 'keymap') return ; throw new Error(`Invalid screen name: ${screenName}`); } diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index 56503df5230..a2d61a079d5 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -15,11 +15,11 @@ interface CommandEditing { } export interface KeymapConfigScreenProps { - theme: number + themeId: number } -export const KeymapConfigScreen = ({ theme }: KeymapConfigScreenProps) => { - const styles = styles_(theme); +export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { + const styles = styles_(themeId); const [filter, setFilter] = useState(''); const [errorMessage, setErrorMessage] = useState(''); @@ -82,7 +82,7 @@ export const KeymapConfigScreen = ({ theme }: KeymapConfigScreenProps) => { setAccelerator={(accelerator: string) => setCommandAccelerator(command, accelerator)} resetAccelerator={() => resetCommandAccelerator(command)} toggleEditing={() => toggleEditing(command)} - theme={theme} + themeId={themeId} /> :
{accelerator || _('Disabled')} diff --git a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx index d3c33a6c41a..8f82679923e 100644 --- a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx +++ b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx @@ -12,10 +12,11 @@ export interface ShortcutRecorderProps extends KeymapConfigScreenProps { setAccelerator: (accelerator: string) => void, resetAccelerator: () => void, toggleEditing: () => void, + themeId: number } -export const ShortcutRecorder = ({ setAccelerator, resetAccelerator, toggleEditing, theme }: ShortcutRecorderProps) => { - const styles = styles_(theme); +export const ShortcutRecorder = ({ setAccelerator, resetAccelerator, toggleEditing, themeId }: ShortcutRecorderProps) => { + const styles = styles_(themeId); const [newAccelerator, setNewAccelerator] = useState(''); const handleKeydown = (event: KeyboardEvent) => { From 8ca3aa04faaae245e903b7422b7c5f237cb99e00 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Fri, 21 Aug 2020 06:21:58 +0530 Subject: [PATCH 14/48] Use handlers --- .../gui/KeymapConfig/KeymapConfigScreen.tsx | 25 ++++++++----------- .../gui/KeymapConfig/ShortcutRecorder.tsx | 25 ++++++++----------- 2 files changed, 21 insertions(+), 29 deletions(-) diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index a2d61a079d5..ed16e9868ac 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -7,7 +7,6 @@ import KeymapService, { KeymapItem } from '../../lib/services/KeymapService'; import getLabel from './utils/getLabel'; const { _ } = require('lib/locale.js'); - const keymapService = KeymapService.instance(); interface CommandEditing { @@ -31,16 +30,17 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { }, {}) ); - const setCommandAccelerator = (commandName: string, accelerator: string) => { + const handleSave = (commandName: string, accelerator: string) => { setKeymap(prevKeymap => { const newKeymap = [...prevKeymap]; newKeymap.find(item => item.command === commandName).accelerator = accelerator; return newKeymap; }); + setEditing(prevEditing => ({ ...prevEditing, [commandName]: false })); }; - const resetCommandAccelerator = (commandName: string) => { + const hanldeReset = (commandName: string) => { setKeymap(prevKeymap => { const newKeymap = [...prevKeymap]; const defaultAccelerator = keymapService.getDefaultAccelerator(commandName); @@ -48,15 +48,11 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { newKeymap.find(item => item.command === commandName).accelerator = defaultAccelerator; return newKeymap; }); + setEditing(prevEditing => ({ ...prevEditing, [commandName]: false })); }; - const toggleEditing = (commandName: string) => { - setEditing(prevEditing => { - const newEditing = { ...prevEditing }; - - newEditing[commandName] = !newEditing[commandName]; - return newEditing; - }); + const handleCancel = (commandName: string) => { + setEditing(prevEditing => ({ ...prevEditing, [commandName]: false })); }; useEffect(() => { @@ -76,12 +72,13 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { }, [keymap]); const renderKeymapRow = ({ command, accelerator }: KeymapItem) => { - const handleClick = () => toggleEditing(command); + const handleClick = () => setEditing(prevEditing => ({ ...prevEditing, [command]: true })); const cellContent = editing[command] ? setCommandAccelerator(command, accelerator)} - resetAccelerator={() => resetCommandAccelerator(command)} - toggleEditing={() => toggleEditing(command)} + onSave={handleSave} + onReset={hanldeReset} + onCancel={handleCancel} + commandName={command} themeId={themeId} /> :
diff --git a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx index 8f82679923e..9200f189db7 100644 --- a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx +++ b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx @@ -9,16 +9,21 @@ const { _ } = require('lib/locale.js'); const keymapService = KeymapService.instance(); export interface ShortcutRecorderProps extends KeymapConfigScreenProps { - setAccelerator: (accelerator: string) => void, - resetAccelerator: () => void, - toggleEditing: () => void, + onSave: (commandName: string, accelerator: string) => void, + onReset: (commandName: string) => void, + onCancel: (commandName: string) => void, + commandName: string, themeId: number } -export const ShortcutRecorder = ({ setAccelerator, resetAccelerator, toggleEditing, themeId }: ShortcutRecorderProps) => { +export const ShortcutRecorder = ({ onSave, onReset, onCancel, commandName, themeId }: ShortcutRecorderProps) => { const styles = styles_(themeId); const [newAccelerator, setNewAccelerator] = useState(''); + const handleSave = () => onSave(commandName, newAccelerator); + const handleReset = () => onReset(commandName); + const handleCancel = () => onCancel(commandName); + const handleKeydown = (event: KeyboardEvent) => { event.preventDefault(); const newAccelerator = keymapService.domToElectronAccelerator(event); @@ -27,22 +32,12 @@ export const ShortcutRecorder = ({ setAccelerator, resetAccelerator, toggleEditi case 'Enter': return handleSave(); case 'Escape': - return handleReset(); + return handleCancel(); default: setNewAccelerator(newAccelerator); } }; - const handleReset = () => { - resetAccelerator(); - toggleEditing(); - }; - - const handleSave = () => { - setAccelerator(newAccelerator.length ? newAccelerator : null); - toggleEditing(); - }; - return (
Date: Fri, 21 Aug 2020 06:33:12 +0530 Subject: [PATCH 15/48] Refresh menu by emitting an event --- ElectronClient/app.js | 4 ++-- .../gui/KeymapConfig/KeymapConfigScreen.tsx | 3 --- .../lib/services/KeymapService.ts | 21 +++++++++++-------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/ElectronClient/app.js b/ElectronClient/app.js index b228a1dc0a0..cff87f1c8f5 100644 --- a/ElectronClient/app.js +++ b/ElectronClient/app.js @@ -108,10 +108,10 @@ class Application extends BaseApplication { this.bridge_nativeThemeUpdated = this.bridge_nativeThemeUpdated.bind(this); - KeymapService.instance().setMenuRefreshCallback(this.refreshMenu.bind(this)); - 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() { diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index ed16e9868ac..bb355188a57 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -61,9 +61,6 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { keymapService.setKeymap(keymap); keymapService.saveKeymap(); // Save changes to the disk - // TODO: Use events instead? - keymapService.triggerMenuRefresh(); - setErrorMessage(''); } catch (err) { const message = err.message || ''; diff --git a/ReactNativeClient/lib/services/KeymapService.ts b/ReactNativeClient/lib/services/KeymapService.ts index c75e0373247..91506a4ff10 100644 --- a/ReactNativeClient/lib/services/KeymapService.ts +++ b/ReactNativeClient/lib/services/KeymapService.ts @@ -2,6 +2,7 @@ import { KeyboardEvent } from 'react'; const fs = require('fs-extra'); const BaseService = require('lib/services/BaseService'); +const eventManager = require('lib/eventManager'); const { shim } = require('lib/shim.js'); const keysRegExp = /^([0-9A-Z)!@#$%^&*(:+<_>?~{|}";=,\-./`[\\\]']|F1*[1-9]|F10|F2[0-4]|Plus|Space|Tab|Backspace|Delete|Insert|Return|Enter|Up|Down|Left|Right|Home|End|PageUp|PageDown|Escape|Esc|VolumeUp|VolumeDown|VolumeMute|MediaNextTrack|MediaPreviousTrack|MediaStop|MediaPlayPause|PrintScreen)$/; @@ -91,7 +92,6 @@ export default class KeymapService extends BaseService { private defaultKeymap: KeymapItem[]; private platform: string; private keymapPath: string; - private refreshMenu: Function; constructor() { super(); @@ -121,6 +121,14 @@ export default class KeymapService extends BaseService { } } + public on(eventName: string, callback: Function) { + eventManager.on(eventName, callback); + } + + public off(eventName: string, callback: Function) { + eventManager.off(eventName, callback); + } + async loadKeymap(keymapPath: string) { this.keymapPath = keymapPath; // For saving the changes later @@ -143,6 +151,9 @@ export default class KeymapService extends BaseService { try { const customKeymap = this.generateCustomKeymap(); await fs.writeFile(this.keymapPath, JSON.stringify(customKeymap, null, 2)); + + // On successful save, refresh the menu items + eventManager.emit('keymapChange'); } catch (err) { throw new Error(`Failed to save keymap: ${this.keymapPath}\n${err}`); } @@ -205,14 +216,6 @@ export default class KeymapService extends BaseService { return Object.keys(this.keymap); } - setMenuRefreshCallback(callback: Function) { - this.refreshMenu = callback; - } - - triggerMenuRefresh() { - this.refreshMenu(); - } - generateCustomKeymap() { const customKeymap: KeymapItem[] = []; this.defaultKeymap.forEach(({ command, accelerator }) => { From 1e636363d205aead6ed00c174ba8131186736b95 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Fri, 21 Aug 2020 06:33:37 +0530 Subject: [PATCH 16/48] Wrap content in div --- ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index bb355188a57..1a9c1de7d1a 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -107,9 +107,8 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { }; return ( - <> +
{renderErrorMessage(errorMessage)} -
{
- +
); }; From 264dd21257ce55c31770539a62078621d8c72b5b Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Fri, 21 Aug 2020 06:34:37 +0530 Subject: [PATCH 17/48] Remove unneccesary parameter --- ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index 1a9c1de7d1a..67fbc27bdef 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -94,7 +94,7 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { ); }; - const renderErrorMessage = (errorMessage: string) => { + const renderErrorMessage = () => { if (errorMessage.length) { return (
@@ -108,7 +108,7 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { return (
- {renderErrorMessage(errorMessage)} + {renderErrorMessage()}
Date: Fri, 21 Aug 2020 06:36:31 +0530 Subject: [PATCH 18/48] Accessibility label --- ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index 67fbc27bdef..ded9611d468 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -116,6 +116,7 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { onChange={event => setFilter(event.target.value)} placeholder={_('Search...')} style={styles.filterInput} + aria-label="Search" />
From c36f80514dfd31adcbc89fa10ea289f3420d40d8 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Fri, 21 Aug 2020 06:37:28 +0530 Subject: [PATCH 19/48] Remove interface extend --- ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx index 9200f189db7..2ce2b2b9132 100644 --- a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx +++ b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx @@ -3,12 +3,11 @@ import { useState, KeyboardEvent } from 'react'; import styles_ from './styles'; import KeymapService from '../../lib/services/KeymapService'; -import { KeymapConfigScreenProps } from './KeymapConfigScreen'; const { _ } = require('lib/locale.js'); const keymapService = KeymapService.instance(); -export interface ShortcutRecorderProps extends KeymapConfigScreenProps { +export interface ShortcutRecorderProps { onSave: (commandName: string, accelerator: string) => void, onReset: (commandName: string) => void, onCancel: (commandName: string) => void, From 88122abadabfa9f98ab571d6abd0a53ac2af709f Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Fri, 21 Aug 2020 06:38:29 +0530 Subject: [PATCH 20/48] Change label "Reset to default" --- ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx index 2ce2b2b9132..5b129e6b13e 100644 --- a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx +++ b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx @@ -53,7 +53,7 @@ export const ShortcutRecorder = ({ onSave, onReset, onCancel, commandName, theme ); From d98878d262d24fb0faccd539800c2d3c5f2f9663 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Fri, 21 Aug 2020 06:42:52 +0530 Subject: [PATCH 21/48] Use shim.fsDriver() for file operations --- ReactNativeClient/lib/services/KeymapService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ReactNativeClient/lib/services/KeymapService.ts b/ReactNativeClient/lib/services/KeymapService.ts index 91506a4ff10..cb7cdfc532b 100644 --- a/ReactNativeClient/lib/services/KeymapService.ts +++ b/ReactNativeClient/lib/services/KeymapService.ts @@ -136,7 +136,7 @@ export default class KeymapService extends BaseService { this.logger().info(`Loading keymap: ${keymapPath}`); try { - const keymapFile = await fs.readFile(keymapPath, 'utf-8'); + const keymapFile = await shim.fsDriver().readFile(keymapPath, 'utf-8'); this.setKeymap(JSON.parse(keymapFile)); } catch (err) { const msg = err.message ? err.message : ''; @@ -150,7 +150,7 @@ export default class KeymapService extends BaseService { try { const customKeymap = this.generateCustomKeymap(); - await fs.writeFile(this.keymapPath, JSON.stringify(customKeymap, null, 2)); + await shim.fsDriver().writeFile(this.keymapPath, JSON.stringify(customKeymap, null, 2)); // On successful save, refresh the menu items eventManager.emit('keymapChange'); From 9758161f465c55634d071fdf2919b502a4bab479 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Fri, 21 Aug 2020 06:43:40 +0530 Subject: [PATCH 22/48] Rename getCommands() to getCommandNames() --- ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx | 2 +- ReactNativeClient/lib/services/KeymapService.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index ded9611d468..bdef47917d6 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -24,7 +24,7 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { const [errorMessage, setErrorMessage] = useState(''); const [keymap, setKeymap] = useState(() => keymapService.getKeymap()); const [editing, setEditing] = useState(() => - keymapService.getCommands().reduce((accumulator: CommandEditing, command: string) => { + keymapService.getCommandNames().reduce((accumulator: CommandEditing, command: string) => { accumulator[command] = false; return accumulator; }, {}) diff --git a/ReactNativeClient/lib/services/KeymapService.ts b/ReactNativeClient/lib/services/KeymapService.ts index cb7cdfc532b..2172ef2f889 100644 --- a/ReactNativeClient/lib/services/KeymapService.ts +++ b/ReactNativeClient/lib/services/KeymapService.ts @@ -212,7 +212,7 @@ export default class KeymapService extends BaseService { return Object.values(this.keymap); } - getCommands() { + getCommandNames() { return Object.keys(this.keymap); } From 1b6ac9fe650508fe7d7eacaedd0d5b64ff8b8ff7 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Fri, 21 Aug 2020 07:26:20 +0530 Subject: [PATCH 23/48] Custom hook useKeymap --- .eslintignore | 1 + .gitignore | 1 + .../gui/KeymapConfig/KeymapConfigScreen.tsx | 34 +++---------------- .../gui/KeymapConfig/utils/useKeymap.ts | 34 +++++++++++++++++++ .../lib/services/KeymapService.ts | 2 +- 5 files changed, 42 insertions(+), 30 deletions(-) create mode 100644 ElectronClient/gui/KeymapConfig/utils/useKeymap.ts diff --git a/.eslintignore b/.eslintignore index 9fc7a7c674d..22684f06a5f 100644 --- a/.eslintignore +++ b/.eslintignore @@ -75,6 +75,7 @@ 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/useKeymap.js ElectronClient/gui/MainScreen/commands/editAlarm.js ElectronClient/gui/MainScreen/commands/exportPdf.js ElectronClient/gui/MainScreen/commands/hideModalMessage.js diff --git a/.gitignore b/.gitignore index f8c31512c75..0ead332339f 100644 --- a/.gitignore +++ b/.gitignore @@ -66,6 +66,7 @@ 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/useKeymap.js ElectronClient/gui/MainScreen/commands/editAlarm.js ElectronClient/gui/MainScreen/commands/exportPdf.js ElectronClient/gui/MainScreen/commands/hideModalMessage.js diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index bdef47917d6..5b297777ab7 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -1,10 +1,11 @@ import * as React from 'react'; -import { useState, useEffect } from 'react'; +import { useState } from 'react'; import styles_ from './styles'; import { ShortcutRecorder } from './ShortcutRecorder'; import KeymapService, { KeymapItem } from '../../lib/services/KeymapService'; import getLabel from './utils/getLabel'; +import useKeymap from './utils/useKeymap'; const { _ } = require('lib/locale.js'); const keymapService = KeymapService.instance(); @@ -21,8 +22,7 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { const styles = styles_(themeId); const [filter, setFilter] = useState(''); - const [errorMessage, setErrorMessage] = useState(''); - const [keymap, setKeymap] = useState(() => keymapService.getKeymap()); + const [keymap, setCommandAccelerator, errorMessage] = useKeymap(keymapService); const [editing, setEditing] = useState(() => keymapService.getCommandNames().reduce((accumulator: CommandEditing, command: string) => { accumulator[command] = false; @@ -31,23 +31,12 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { ); const handleSave = (commandName: string, accelerator: string) => { - setKeymap(prevKeymap => { - const newKeymap = [...prevKeymap]; - - newKeymap.find(item => item.command === commandName).accelerator = accelerator; - return newKeymap; - }); + setCommandAccelerator(commandName, accelerator); setEditing(prevEditing => ({ ...prevEditing, [commandName]: false })); }; const hanldeReset = (commandName: string) => { - setKeymap(prevKeymap => { - const newKeymap = [...prevKeymap]; - const defaultAccelerator = keymapService.getDefaultAccelerator(commandName); - - newKeymap.find(item => item.command === commandName).accelerator = defaultAccelerator; - return newKeymap; - }); + setCommandAccelerator(commandName, keymapService.getDefaultAccelerator(commandName)); setEditing(prevEditing => ({ ...prevEditing, [commandName]: false })); }; @@ -55,19 +44,6 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { setEditing(prevEditing => ({ ...prevEditing, [commandName]: false })); }; - useEffect(() => { - try { - // Synchronize the keymap of KeymapService with the current keymap state - keymapService.setKeymap(keymap); - keymapService.saveKeymap(); // Save changes to the disk - - setErrorMessage(''); - } catch (err) { - const message = err.message || ''; - setErrorMessage(message); - } - }, [keymap]); - const renderKeymapRow = ({ command, accelerator }: KeymapItem) => { const handleClick = () => setEditing(prevEditing => ({ ...prevEditing, [command]: true })); const cellContent = editing[command] diff --git a/ElectronClient/gui/KeymapConfig/utils/useKeymap.ts b/ElectronClient/gui/KeymapConfig/utils/useKeymap.ts new file mode 100644 index 00000000000..06fa4b04785 --- /dev/null +++ b/ElectronClient/gui/KeymapConfig/utils/useKeymap.ts @@ -0,0 +1,34 @@ +import { useState, useEffect } from 'react'; +import KeymapService, { KeymapItem } from '../../../lib/services/KeymapService'; + +const useKeymap = (keymapService: KeymapService): [KeymapItem[], (commandName: string, accelerator: string) => void, string] => { + const [keymap, setKeymap] = useState(() => keymapService.getKeymap()); + const [errorMessage, setErrorMessage] = useState(''); + + const setCommandAccelerator = (commandName: string, accelerator: string) => { + setKeymap(prevKeymap => { + const newKeymap = [...prevKeymap]; + + newKeymap.find(item => item.command === commandName).accelerator = accelerator; + return newKeymap; + }); + }; + + useEffect(() => { + try { + // Synchronize the keymap of KeymapService with the current keymap state + keymapService.setKeymap(keymap); + // Save changes to the disk + keymapService.saveKeymap(); + + setErrorMessage(''); + } catch (err) { + const message = err.message || ''; + setErrorMessage(message); + } + }, [keymap]); + + return [keymap, setCommandAccelerator, errorMessage]; +}; + +export default useKeymap; diff --git a/ReactNativeClient/lib/services/KeymapService.ts b/ReactNativeClient/lib/services/KeymapService.ts index 2172ef2f889..ba55bea25cd 100644 --- a/ReactNativeClient/lib/services/KeymapService.ts +++ b/ReactNativeClient/lib/services/KeymapService.ts @@ -150,7 +150,7 @@ export default class KeymapService extends BaseService { try { const customKeymap = this.generateCustomKeymap(); - await shim.fsDriver().writeFile(this.keymapPath, JSON.stringify(customKeymap, null, 2)); + await shim.fsDriver().writeFile(this.keymapPath, JSON.stringify(customKeymap, null, 2), 'utf-8'); // On successful save, refresh the menu items eventManager.emit('keymapChange'); From a12f4a1b303a1f2fdf633b7b99bbed7b8a44899a Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Fri, 21 Aug 2020 07:41:29 +0530 Subject: [PATCH 24/48] Minor refactoring --- .../gui/KeymapConfig/KeymapConfigScreen.tsx | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index 5b297777ab7..9b0758200fe 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -1,11 +1,11 @@ import * as React from 'react'; import { useState } from 'react'; -import styles_ from './styles'; -import { ShortcutRecorder } from './ShortcutRecorder'; import KeymapService, { KeymapItem } from '../../lib/services/KeymapService'; +import { ShortcutRecorder } from './ShortcutRecorder'; import getLabel from './utils/getLabel'; import useKeymap from './utils/useKeymap'; +import styles_ from './styles'; const { _ } = require('lib/locale.js'); const keymapService = KeymapService.instance(); @@ -71,15 +71,12 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { }; const renderErrorMessage = () => { - if (errorMessage.length) { - return ( -
-

- {errorMessage} -

-
- ); - } else { return null; } + errorMessage.length && +
+

+ {errorMessage} +

+
; }; return ( From 3005f587b5aedb1aeca97aa938565af19324c3cf Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Sat, 22 Aug 2020 22:18:24 +0530 Subject: [PATCH 25/48] Remove indirect method calls --- .../gui/KeymapConfig/ShortcutRecorder.tsx | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx index 5b129e6b13e..09d53a6ab83 100644 --- a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx +++ b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx @@ -19,19 +19,15 @@ export const ShortcutRecorder = ({ onSave, onReset, onCancel, commandName, theme const styles = styles_(themeId); const [newAccelerator, setNewAccelerator] = useState(''); - const handleSave = () => onSave(commandName, newAccelerator); - const handleReset = () => onReset(commandName); - const handleCancel = () => onCancel(commandName); - const handleKeydown = (event: KeyboardEvent) => { event.preventDefault(); const newAccelerator = keymapService.domToElectronAccelerator(event); switch (newAccelerator) { case 'Enter': - return handleSave(); + return onSave(commandName, newAccelerator); case 'Escape': - return handleCancel(); + return onCancel(commandName); default: setNewAccelerator(newAccelerator); } @@ -47,12 +43,10 @@ export const ShortcutRecorder = ({ onSave, onReset, onCancel, commandName, theme readOnly autoFocus /> - - - - From 8a70a9718c0193b360f9dd40a24e014a54693b19 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Sat, 22 Aug 2020 22:21:27 +0530 Subject: [PATCH 26/48] Rename state variable to make the code clear --- ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx index 09d53a6ab83..a450aa479ef 100644 --- a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx +++ b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx @@ -17,7 +17,7 @@ export interface ShortcutRecorderProps { export const ShortcutRecorder = ({ onSave, onReset, onCancel, commandName, themeId }: ShortcutRecorderProps) => { const styles = styles_(themeId); - const [newAccelerator, setNewAccelerator] = useState(''); + const [accelerator, setAccelerator] = useState(''); const handleKeydown = (event: KeyboardEvent) => { event.preventDefault(); @@ -29,21 +29,21 @@ export const ShortcutRecorder = ({ onSave, onReset, onCancel, commandName, theme case 'Escape': return onCancel(commandName); default: - setNewAccelerator(newAccelerator); + setAccelerator(newAccelerator); } }; return (
- -
From d2aa3f99dae57c3ff91d49eaca4016dde4586098 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Sun, 23 Aug 2020 00:52:29 +0530 Subject: [PATCH 28/48] Enable options parameter overriding on CommandService.commandByName() method; Use CommandService.exists() instead of try/catch to check for existing commands --- ElectronClient/gui/KeymapConfig/utils/getLabel.ts | 9 +++++---- ReactNativeClient/lib/services/CommandService.ts | 6 ++++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/ElectronClient/gui/KeymapConfig/utils/getLabel.ts b/ElectronClient/gui/KeymapConfig/utils/getLabel.ts index 29145380b2c..b571180df76 100644 --- a/ElectronClient/gui/KeymapConfig/utils/getLabel.ts +++ b/ElectronClient/gui/KeymapConfig/utils/getLabel.ts @@ -2,11 +2,12 @@ import CommandService from '../../../lib/services/CommandService'; const { _ } = require('lib/locale.js'); const { shim } = require('lib/shim.js'); +const commandService = CommandService.instance(); const getLabel = (commandName: string) => { - try { - return CommandService.instance().label(commandName); - } catch (err) { + if (commandService.exists(commandName)) { + return commandService.label(commandName); + } else { switch (commandName) { case 'quit': return _('Quit'); @@ -25,7 +26,7 @@ const getLabel = (commandName: string) => { case 'config': return shim.isMac() ? _('Preferences') : _('Options'); default: - throw err; + throw new Error(`Command: ${commandName} is unknown`); } } }; diff --git a/ReactNativeClient/lib/services/CommandService.ts b/ReactNativeClient/lib/services/CommandService.ts index a501a054055..dcd0c25c04e 100644 --- a/ReactNativeClient/lib/services/CommandService.ts +++ b/ReactNativeClient/lib/services/CommandService.ts @@ -157,6 +157,7 @@ export default class CommandService extends BaseService { options = { mustExist: true, runtimeMustBeRegistered: false, + ...options, }; const command = this.commands_[name]; @@ -254,6 +255,11 @@ export default class CommandService extends BaseService { return command.declaration.label(); } + exists(commandName:string):boolean { + const command = this.commandByName(commandName, { mustExist: false }); + return !!command; + } + private extractExecuteArgs(command:Command, executeArgs:any) { if (executeArgs) return executeArgs; if (!command.runtime) throw new Error(`Command: ${command.declaration.name}: Runtime is not defined - make sure it has been registered.`); From c86d97d413452fc3cc664068aac793007a45fac8 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Sun, 23 Aug 2020 04:59:30 +0530 Subject: [PATCH 29/48] Implement error class KeymapError (For highlighting invalid keymap entries) --- .../gui/KeymapConfig/KeymapConfigScreen.tsx | 30 ++++--- .../gui/KeymapConfig/utils/useKeymap.ts | 21 +++-- .../lib/services/KeymapService.ts | 85 +++++++++++++------ 3 files changed, 87 insertions(+), 49 deletions(-) diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index 4c034f21eae..d8d5fbfaf0f 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -22,7 +22,7 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { const styles = styles_(themeId); const [filter, setFilter] = useState(''); - const [keymap, setCommandAccelerator, errorMessage] = useKeymap(keymapService); + const [keymap, setAccelerator, error] = useKeymap(); const [editing, setEditing] = useState(() => keymapService.getCommandNames().reduce((accumulator: CommandEditing, command: string) => { accumulator[command] = false; @@ -33,14 +33,14 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { const handleSave = (event: { commandName: string, accelerator: string }) => { const { commandName, accelerator } = event; - setCommandAccelerator(commandName, accelerator); + setAccelerator(commandName, accelerator); setEditing(prevEditing => ({ ...prevEditing, [commandName]: false })); }; - const hanldeReset = (event : { commandName: string }) => { + const hanldeReset = (event: { commandName: string }) => { const { commandName } = event; - setCommandAccelerator(commandName, keymapService.getDefaultAccelerator(commandName)); + setAccelerator(commandName, keymapService.getDefaultAccelerator(commandName)); setEditing(prevEditing => ({ ...prevEditing, [commandName]: false })); }; @@ -76,18 +76,20 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { ); }; - const renderErrorMessage = () => { - errorMessage.length && -
-

- {errorMessage} -

-
; - }; - return (
- {renderErrorMessage()} + {error && +
+

+ + {error.details.duplicateAccelerators + ? _('Keymap configuration contains duplicates. Change or disable one of the shortcuts to continue.') + : error.message // Highly unlikely that any other error will occur at this point + } + +

+
+ }
void, KeymapError] => { + const keymapService = KeymapService.instance(); -const useKeymap = (keymapService: KeymapService): [KeymapItem[], (commandName: string, accelerator: string) => void, string] => { const [keymap, setKeymap] = useState(() => keymapService.getKeymap()); - const [errorMessage, setErrorMessage] = useState(''); + const [error, setError] = useState(null); - const setCommandAccelerator = (commandName: string, accelerator: string) => { + const setAccelerator = (commandName: string, accelerator: string) => { setKeymap(prevKeymap => { const newKeymap = [...prevKeymap]; - newKeymap.find(item => item.command === commandName).accelerator = accelerator; + newKeymap.find(item => item.command === commandName).accelerator = accelerator || null; return newKeymap; }); }; useEffect(() => { try { - // Synchronize the keymap of KeymapService with the current keymap state keymapService.setKeymap(keymap); // Save changes to the disk keymapService.saveKeymap(); - - setErrorMessage(''); + setError(null); } catch (err) { - const message = err.message || ''; - setErrorMessage(message); + setError(err); } }, [keymap]); - return [keymap, setCommandAccelerator, errorMessage]; + return [keymap, setAccelerator, error]; }; export default useKeymap; diff --git a/ReactNativeClient/lib/services/KeymapService.ts b/ReactNativeClient/lib/services/KeymapService.ts index ba55bea25cd..a12a6a64888 100644 --- a/ReactNativeClient/lib/services/KeymapService.ts +++ b/ReactNativeClient/lib/services/KeymapService.ts @@ -78,6 +78,36 @@ const defaultKeymap = { ], }; +export interface KeymapErrorDetails { + missingAccelerator?: boolean, + missingCommand?: boolean + invalidAccelerator?: boolean, + invalidCommand?: boolean, + invalidKeymapItem?: KeymapItem, + duplicateAccelerators?: boolean, + duplicateKeymapItems?: [KeymapItem, KeymapItem] +} + +export class KeymapError extends Error { + private details_: KeymapErrorDetails; + + constructor(message: string, details: KeymapErrorDetails = {}) { + super(message); + this.details_ = { + missingAccelerator: false, + missingCommand: false, + invalidAccelerator: false, + invalidCommand: false, + duplicateAccelerators: false, + ...details, + }; + } + + get details() { + return this.details_; + } +} + export interface KeymapItem { accelerator: string; command: string; @@ -133,20 +163,20 @@ export default class KeymapService extends BaseService { this.keymapPath = keymapPath; // For saving the changes later if (await fs.exists(keymapPath)) { - this.logger().info(`Loading keymap: ${keymapPath}`); + this.logger().info(`KeymapService: Loading keymap: ${keymapPath}`); try { const keymapFile = await shim.fsDriver().readFile(keymapPath, 'utf-8'); this.setKeymap(JSON.parse(keymapFile)); } catch (err) { const msg = err.message ? err.message : ''; - throw new Error(`Failed to load keymap: ${keymapPath}\n${msg}`); + throw new Error(`KeymapService: Failed to load keymap: ${keymapPath}\n${msg}`); } } } async saveKeymap() { - this.logger().info(`Saving keymap: ${this.keymapPath}`); + this.logger().info(`KeymapService: Saving keymap: ${this.keymapPath}`); try { const customKeymap = this.generateCustomKeymap(); @@ -155,7 +185,7 @@ export default class KeymapService extends BaseService { // On successful save, refresh the menu items eventManager.emit('keymapChange'); } catch (err) { - throw new Error(`Failed to save keymap: ${this.keymapPath}\n${err}`); + throw new Error(`KeymapServiceL Failed to save keymap: ${this.keymapPath}\n${err}`); } } @@ -192,20 +222,13 @@ export default class KeymapService extends BaseService { for (let i = 0; i < customKeymap.length; i++) { const item = customKeymap[i]; - try { - this.validateKeymapItem(item); // Throws if there are any issues in the keymap item - this.setAccelerator(item.command, item.accelerator); - } catch (err) { - throw new Error(`Keymap item ${JSON.stringify(item)} is invalid: ${err.message}`); - } + // Throws if there are any issues in the keymap item + this.validateKeymapItem(item); + this.setAccelerator(item.command, item.accelerator); } - try { - this.validateKeymap(); // Throws whenever there are duplicate Accelerators used in the keymap - } catch (err) { - this.initialize(); // Fallback to default keymap configuration - throw new Error(`Keymap configuration contains duplicates. \n${err.message}`); - } + // Throws whenever there are duplicate Accelerators used in the keymap + this.validateKeymap(); } getKeymap() { @@ -230,18 +253,30 @@ export default class KeymapService extends BaseService { private validateKeymapItem(item: KeymapItem) { if (!item.hasOwnProperty('command')) { - throw new Error('"command" property is missing'); + throw new KeymapError( + `Keymap item ${JSON.stringify(item)} is invalid because "command" property is missing.`, + { invalidKeymapItem: item, missingCommand: true } + ); } else if (!this.keymap.hasOwnProperty(item.command)) { - throw new Error(`"${item.command}" is not a valid command`); + throw new KeymapError( + `Keymap item ${JSON.stringify(item)} is invalid because "${item.command}" is not a valid command.`, + { invalidKeymapItem: item, invalidCommand: true } + ); } if (!item.hasOwnProperty('accelerator')) { - throw new Error('"accelerator" property is missing'); + throw new KeymapError( + `Keymap item ${JSON.stringify(item)} is invalid because "accelerator" property is missing.`, + { invalidKeymapItem: item, missingAccelerator: true } + ); } else if (item.accelerator !== null) { try { this.validateAccelerator(item.accelerator); - } catch (err) { - throw new Error(`"${item.accelerator}" is not a valid accelerator.`); + } catch { + throw new KeymapError( + `Keymap item ${JSON.stringify(item)} is invalid because "${item.accelerator}" is not a valid accelerator.`, + { invalidKeymapItem: item, invalidAccelerator: true } + ); } } } @@ -254,9 +289,11 @@ export default class KeymapService extends BaseService { if (usedAccelerators.has(itemAccelerator)) { const originalItem = Object.values(this.keymap).find(_item => _item.accelerator == item.accelerator); - throw new Error( - `Accelerator "${itemAccelerator}" can't be used for both "${item.command}" and "${originalItem.command}" commands. \n` + - 'You have to change the accelerator for any of above commands.' + throw new KeymapError( + 'Keymap configuration contains duplicates.\n' + + `Accelerator "${itemAccelerator}" can't be used for both "${item.command}" and "${originalItem.command}" commands.\n` + + 'You have to change the accelerator for any of above commands.', + { duplicateAccelerators: true, duplicateKeymapItems: [item, originalItem] } ); } else if (itemAccelerator !== null) { usedAccelerators.add(itemAccelerator); From 4e3b67568efcb83cbfe3599c803981e9569b6850 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Sun, 23 Aug 2020 05:17:53 +0530 Subject: [PATCH 30/48] Cancel button --- ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx index a65478327fa..a1c021c6f62 100644 --- a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx +++ b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx @@ -49,6 +49,9 @@ export const ShortcutRecorder = ({ onSave, onReset, onCancel, commandName, theme +
); }; From 670075f37c0ce75879ca28a6d2365f2f754856ac Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Sun, 23 Aug 2020 16:27:38 +0530 Subject: [PATCH 31/48] Better errors; Improve styling --- .eslintignore | 2 + .gitignore | 2 + CliClient/tests/services_KeymapService.js | 6 --- .../gui/KeymapConfig/KeymapConfigScreen.tsx | 46 ++++++++++++++---- .../gui/KeymapConfig/ShortcutRecorder.tsx | 6 +-- .../gui/KeymapConfig/styles/index.ts | 47 ++++++++++++++----- .../lib/services/KeymapService.ts | 19 ++++---- 7 files changed, 89 insertions(+), 39 deletions(-) diff --git a/.eslintignore b/.eslintignore index 22684f06a5f..855d7ad5ddb 100644 --- a/.eslintignore +++ b/.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 diff --git a/.gitignore b/.gitignore index 0ead332339f..c6807e9149b 100644 --- a/.gitignore +++ b/.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 diff --git a/CliClient/tests/services_KeymapService.js b/CliClient/tests/services_KeymapService.js index 18a6e8f8045..9aa91542210 100644 --- a/CliClient/tests/services_KeymapService.js +++ b/CliClient/tests/services_KeymapService.js @@ -306,13 +306,7 @@ 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]); - } } }); }); diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index d8d5fbfaf0f..ff5b913cb4d 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -32,24 +32,44 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { const handleSave = (event: { commandName: string, accelerator: string }) => { const { commandName, accelerator } = event; - setAccelerator(commandName, accelerator); setEditing(prevEditing => ({ ...prevEditing, [commandName]: false })); }; const hanldeReset = (event: { commandName: string }) => { const { commandName } = event; - setAccelerator(commandName, keymapService.getDefaultAccelerator(commandName)); setEditing(prevEditing => ({ ...prevEditing, [commandName]: false })); }; const handleCancel = (event: { commandName: string }) => { const { commandName } = event; - setEditing(prevEditing => ({ ...prevEditing, [commandName]: false })); }; + const renderAccelerator = (accelerator: string) => { + return ( +
+ {accelerator.split('+').map(part => {part}).reduce( + (accumulator, part) => (accumulator.length ? [...accumulator, ' + ', part] : [part]), + [] + )} +
+ ); + }; + + const renderStatus = (command: string) => { + if (!error) return null; + const { invalidAccelerator, invalidKeymapItem, duplicateAccelerators, duplicateKeymapItems } = error.details; + + if ((invalidAccelerator && invalidKeymapItem.command === command) || + (duplicateAccelerators && (duplicateKeymapItems[0].command === command || duplicateKeymapItems[1].command === command))) { + return '❌'; + } else { + return null; + } + }; + const renderKeymapRow = ({ command, accelerator }: KeymapItem) => { const handleClick = () => setEditing(prevEditing => ({ ...prevEditing, [command]: true })); const cellContent = editing[command] @@ -60,12 +80,20 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { commandName={command} themeId={themeId} /> - :
- {accelerator || _('Disabled')} + :
+
+ {accelerator + ? renderAccelerator(accelerator) + :
{_('Disabled')}
+ } +
+
+ {renderStatus(command)} +
; return ( -
+ @@ -84,7 +112,9 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { {error.details.duplicateAccelerators ? _('Keymap configuration contains duplicates. Change or disable one of the shortcuts to continue.') - : error.message // Highly unlikely that any other error will occur at this point + : error.details.invalidAccelerator + ? _('Keymap configuration an invalid shortcut. Change or disable it to continue.') + : error.message // Highly unlikely that any other error will occur at this point }

@@ -102,7 +132,7 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => {
{getLabel(command)}
- + diff --git a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx index a1c021c6f62..81361c4861f 100644 --- a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx +++ b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx @@ -1,8 +1,8 @@ import * as React from 'react'; import { useState, KeyboardEvent } from 'react'; -import styles_ from './styles'; import KeymapService from '../../lib/services/KeymapService'; +import styles_ from './styles'; const { _ } = require('lib/locale.js'); const keymapService = KeymapService.instance(); @@ -34,12 +34,12 @@ export const ShortcutRecorder = ({ onSave, onReset, onCancel, commandName, theme }; return ( -
+
diff --git a/ElectronClient/gui/KeymapConfig/styles/index.ts b/ElectronClient/gui/KeymapConfig/styles/index.ts index c89743dd7e8..afee3122b4f 100644 --- a/ElectronClient/gui/KeymapConfig/styles/index.ts +++ b/ElectronClient/gui/KeymapConfig/styles/index.ts @@ -1,30 +1,32 @@ const { buildStyle } = require('lib/theme'); -export default function styles(theme: number) { - return buildStyle('KeymapConfigScreen', theme, (theme: any) => { +export default function styles(themeId: number) { + return buildStyle('KeymapConfigScreen', themeId, (theme: any) => { return { container: { ...theme.containerStyle, padding: 16, }, - topActions: { - display: 'flex', - flexDirection: 'row', - }, text: { ...theme.textStyle, }, input: { ...theme.inputStyle, }, + topActions: { + display: 'flex', + flexDirection: 'row', + }, filterInput: { ...theme.inputStyle, flexGrow: 1, - minHeight: theme.buttonStyle.minHeight, + }, + shortcutInput: { + ...theme.inputStyle, }, inlineButton: { ...theme.buttonStyle, - marginLeft: 12, + marginLeft: 8, }, warning: { backgroundColor: theme.warningBackgroundColor, @@ -35,20 +37,41 @@ export default function styles(theme: number) { }, table: { ...theme.containerStyle, - marginTop: 15, + marginTop: 16, overflow: 'auto', width: '100%', }, tableShortcutColumn: { ...theme.textStyle, - width: '65%', + width: '60%', }, tableCommandColumn: { ...theme.textStyle, width: 'auto', }, - tableRow: { - minHeight: 25, + tableCell: { + display: 'flex', + flexDirection: 'row', + cursor: 'pointer', + }, + tableCellShortcut: { + flexGrow: 1, + }, + tableCellStatus: { + // No specific styling at the moment + }, + kbd: { + fontFamily: 'sans-serif', + border: '1px solid', + borderRadius: 4, + backgroundColor: theme.raisedBackgroundColor, + padding: 2, + paddingLeft: 6, + paddingRight: 6, + }, + disabled: { + color: theme.colorFaded, + fontStyle: 'italic', }, }; }); diff --git a/ReactNativeClient/lib/services/KeymapService.ts b/ReactNativeClient/lib/services/KeymapService.ts index a12a6a64888..e592c973c48 100644 --- a/ReactNativeClient/lib/services/KeymapService.ts +++ b/ReactNativeClient/lib/services/KeymapService.ts @@ -79,10 +79,10 @@ const defaultKeymap = { }; export interface KeymapErrorDetails { - missingAccelerator?: boolean, - missingCommand?: boolean - invalidAccelerator?: boolean, invalidCommand?: boolean, + invalidAccelerator?: boolean, + missingCommand?: boolean + missingAccelerator?: boolean, invalidKeymapItem?: KeymapItem, duplicateAccelerators?: boolean, duplicateKeymapItems?: [KeymapItem, KeymapItem] @@ -164,13 +164,12 @@ export default class KeymapService extends BaseService { if (await fs.exists(keymapPath)) { this.logger().info(`KeymapService: Loading keymap: ${keymapPath}`); - try { const keymapFile = await shim.fsDriver().readFile(keymapPath, 'utf-8'); this.setKeymap(JSON.parse(keymapFile)); } catch (err) { - const msg = err.message ? err.message : ''; - throw new Error(`KeymapService: Failed to load keymap: ${keymapPath}\n${msg}`); + const message = err.message ? err.message : ''; + throw new Error(`Failed to load keymap: ${keymapPath}\n${message}`); } } } @@ -181,11 +180,11 @@ export default class KeymapService extends BaseService { try { const customKeymap = this.generateCustomKeymap(); await shim.fsDriver().writeFile(this.keymapPath, JSON.stringify(customKeymap, null, 2), 'utf-8'); - - // On successful save, refresh the menu items + // Refresh the menu items eventManager.emit('keymapChange'); } catch (err) { - throw new Error(`KeymapServiceL Failed to save keymap: ${this.keymapPath}\n${err}`); + const message = err.message ? err.message : ''; + throw new Error(`Failed to save keymap: ${this.keymapPath}\n${message}`); } } @@ -266,7 +265,7 @@ export default class KeymapService extends BaseService { if (!item.hasOwnProperty('accelerator')) { throw new KeymapError( - `Keymap item ${JSON.stringify(item)} is invalid because "accelerator" property is missing.`, + `Keymap item ${JSON.stringify(item)} is invalid because "accelerator" property is missing.`, { invalidKeymapItem: item, missingAccelerator: true } ); } else if (item.accelerator !== null) { From 06723da291ca70085089f532644c748d533084a7 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Sun, 23 Aug 2020 16:49:23 +0530 Subject: [PATCH 32/48] Remove obsolete tests --- CliClient/tests/services_KeymapService.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/CliClient/tests/services_KeymapService.js b/CliClient/tests/services_KeymapService.js index 9aa91542210..69cf71543c3 100644 --- a/CliClient/tests/services_KeymapService.js +++ b/CliClient/tests/services_KeymapService.js @@ -282,13 +282,7 @@ 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 customKeymaps_Linux = [ From 374ddc3d60ef7d2c72795f4637b0e4b838108521 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Sun, 23 Aug 2020 17:04:59 +0530 Subject: [PATCH 33/48] Fix typo in error message --- ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index ff5b913cb4d..28d008be1e4 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -113,7 +113,7 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { {error.details.duplicateAccelerators ? _('Keymap configuration contains duplicates. Change or disable one of the shortcuts to continue.') : error.details.invalidAccelerator - ? _('Keymap configuration an invalid shortcut. Change or disable it to continue.') + ? _('Keymap configuration contains an invalid shortcut. Change or disable it to continue.') : error.message // Highly unlikely that any other error will occur at this point } From 3252dde718c24873c8416289b8f65a3e4a5e4029 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Sun, 23 Aug 2020 17:34:47 +0530 Subject: [PATCH 34/48] Tweak styling --- .../gui/KeymapConfig/KeymapConfigScreen.tsx | 3 +- .../gui/KeymapConfig/ShortcutRecorder.tsx | 11 +++--- .../gui/KeymapConfig/styles/index.ts | 37 +++++++++---------- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index 28d008be1e4..b1c82705e7d 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -77,6 +77,7 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { onSave={handleSave} onReset={hanldeReset} onCancel={handleCancel} + initialAccelerator={accelerator} commandName={command} themeId={themeId} /> @@ -121,7 +122,7 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => {
}
-
+
setFilter(event.target.value)} diff --git a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx index 81361c4861f..d59d6a201d2 100644 --- a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx +++ b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx @@ -11,13 +11,14 @@ export interface ShortcutRecorderProps { onSave: (event: { commandName: string, accelerator: string }) => void, onReset: (event: { commandName: string }) => void, onCancel: (event: { commandName: string }) => void, + initialAccelerator: string commandName: string, themeId: number } -export const ShortcutRecorder = ({ onSave, onReset, onCancel, commandName, themeId }: ShortcutRecorderProps) => { +export const ShortcutRecorder = ({ onSave, onReset, onCancel, initialAccelerator, commandName, themeId }: ShortcutRecorderProps) => { const styles = styles_(themeId); - const [accelerator, setAccelerator] = useState(''); + const [accelerator, setAccelerator] = useState(initialAccelerator); const handleKeydown = (event: KeyboardEvent) => { event.preventDefault(); @@ -34,12 +35,12 @@ export const ShortcutRecorder = ({ onSave, onReset, onCancel, commandName, theme }; return ( -
+
@@ -47,7 +48,7 @@ export const ShortcutRecorder = ({ onSave, onReset, onCancel, commandName, theme {_('Save')} +
{_('Command')} {_('Keyboard Shortcut')}
diff --git a/ElectronClient/gui/KeymapConfig/utils/useKeymap.ts b/ElectronClient/gui/KeymapConfig/utils/useKeymap.ts index 72d5a1778f1..6651bc3b20e 100644 --- a/ElectronClient/gui/KeymapConfig/utils/useKeymap.ts +++ b/ElectronClient/gui/KeymapConfig/utils/useKeymap.ts @@ -3,7 +3,7 @@ import KeymapService, { KeymapItem, KeymapError } from '../../../lib/services/Ke const keymapService = KeymapService.instance(); -const useKeymap = (): [KeymapItem[], KeymapError, (commandName: string, accelerator: string) => void] => { +const useKeymap = (): [KeymapItem[], KeymapError, (keymapItems: KeymapItem[]) => void, (commandName: string, accelerator: string) => void] => { const [keymap, setKeymap] = useState(() => keymapService.getKeymap()); const [keymapError, setKeymapError] = useState(null); @@ -16,18 +16,36 @@ const useKeymap = (): [KeymapItem[], KeymapError, (commandName: string, accelera }); }; + const setKeymapWrapper = (keymapItems: KeymapItem[]) => { + const oldKeymap = [...keymap]; + + // Avoid new changes being merged with the old changes + keymapService.initialize(); + try { + // Update the in-memory keymap of KeymapService + keymapService.overrideKeymap(keymapItems); + // Synchronize the state with KeymapService + // Side-effect: Changes will also be saved to the disk + setKeymap(keymapService.getKeymap()); + } catch (err) { + // Avoid partially-loading keymap files + keymapService.overrideKeymap(oldKeymap); + throw err; + } + }; + useEffect(() => { try { - // Save changes to the drive + keymapService.overrideKeymap(keymap); + // Save changes to the disk keymapService.saveKeymap(); - keymapService.setKeymap(keymap); setKeymapError(null); } catch (err) { setKeymapError(err); } }, [keymap]); - return [keymap, keymapError, setAccelerator]; + return [keymap, keymapError, setKeymapWrapper, setAccelerator]; }; export default useKeymap; diff --git a/ReactNativeClient/lib/services/KeymapService.ts b/ReactNativeClient/lib/services/KeymapService.ts index f7fe664a523..caa6855ef6c 100644 --- a/ReactNativeClient/lib/services/KeymapService.ts +++ b/ReactNativeClient/lib/services/KeymapService.ts @@ -151,7 +151,7 @@ export default class KeymapService extends BaseService { this.logger().info(`KeymapService: Loading keymap: ${keymapPath}`); try { const keymapFile = await shim.fsDriver().readFile(keymapPath, 'utf-8'); - this.setKeymap(JSON.parse(keymapFile)); + this.overrideKeymap(JSON.parse(keymapFile)); } catch (err) { const message = err.message ? err.message : ''; throw new Error(`Failed to load keymap: ${keymapPath}\n${message}`); @@ -159,17 +159,17 @@ export default class KeymapService extends BaseService { } } - async saveKeymap() { - this.logger().info(`KeymapService: Saving keymap: ${this.keymapPath}`); + async saveKeymap(keymapPath: string = this.keymapPath) { + this.logger().info(`KeymapService: Saving keymap: ${keymapPath}`); try { const customKeymap = this.generateCustomKeymap(); - await shim.fsDriver().writeFile(this.keymapPath, JSON.stringify(customKeymap, null, 2), 'utf-8'); + await shim.fsDriver().writeFile(keymapPath, JSON.stringify(customKeymap, null, 2), 'utf-8'); // Refresh the menu items eventManager.emit('keymapChange'); } catch (err) { const message = err.message ? err.message : ''; - throw new Error(`Failed to save keymap: ${this.keymapPath}\n${message}`); + throw new Error(`Failed to save keymap: ${keymapPath}\n${message}`); } } @@ -202,7 +202,11 @@ export default class KeymapService extends BaseService { else return defaultItem.accelerator; } - setKeymap(customKeymap: KeymapItem[]) { + getDefaultKeymap() { + return [...this.defaultKeymap]; + } + + overrideKeymap(customKeymap: KeymapItem[]) { for (let i = 0; i < customKeymap.length; i++) { const item = customKeymap[i]; From 471210f27811dc564572b9f37f6e3ea604a82cd3 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Sun, 30 Aug 2020 22:47:35 +0530 Subject: [PATCH 42/48] Refactor and code clean-up --- CliClient/tests/services_KeymapService.js | 52 +++-- ElectronClient/app.js | 10 +- .../gui/KeymapConfig/KeymapConfigScreen.tsx | 34 +-- .../gui/KeymapConfig/ShortcutRecorder.tsx | 7 +- .../gui/KeymapConfig/utils/getLabel.ts | 47 ++--- .../KeymapConfig/utils/useCommandStatus.ts | 4 +- .../gui/KeymapConfig/utils/useKeymap.ts | 67 ++++-- .../lib/services/CommandService.ts | 2 +- .../lib/services/KeymapService.ts | 193 +++++++++--------- 9 files changed, 220 insertions(+), 196 deletions(-) diff --git a/CliClient/tests/services_KeymapService.js b/CliClient/tests/services_KeymapService.js index 69cf71543c3..9eb04fa284e 100644 --- a/CliClient/tests/services_KeymapService.js +++ b/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,6 +132,7 @@ 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); @@ -138,10 +140,10 @@ describe('services_KeymapService', () => { }); }); - 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,8 +279,8 @@ describe('services_KeymapService', () => { ]; for (let i = 0; i < customKeymaps_Darwin.length; i++) { - const customKeymap = customKeymaps_Darwin[i]; - expect(() => keymapService.setKeymap(customKeymap)).toThrow(); + const customKeymapItems = customKeymaps_Darwin[i]; + expect(() => keymapService.overrideKeymap(customKeymapItems)).toThrow(); } const customKeymaps_Linux = [ @@ -299,8 +297,8 @@ describe('services_KeymapService', () => { ]; for (let i = 0; i < customKeymaps_Linux.length; i++) { - const customKeymap = customKeymaps_Linux[i]; - expect(() => keymapService.setKeymap(customKeymap)).toThrow(); + const customKeymapItems = customKeymaps_Linux[i]; + expect(() => keymapService.overrideKeymap(customKeymapItems)).toThrow(); } }); }); diff --git a/ElectronClient/app.js b/ElectronClient/app.js index 4387e83b844..291edd4e200 100644 --- a/ElectronClient/app.js +++ b/ElectronClient/app.js @@ -571,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', @@ -633,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', @@ -682,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', @@ -702,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', @@ -1089,7 +1089,7 @@ class Application extends BaseApplication { const keymapService = KeymapService.instance(); try { - await keymapService.loadKeymap(`${dir}/keymap-desktop.json`); + await keymapService.loadCustomKeymap(`${dir}/keymap-desktop.json`); } catch (err) { bridge().showErrorMessageBox(err.message); } diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index b83afe6aebe..542b1311c2f 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -1,7 +1,7 @@ import * as React from 'react'; import { useState } from 'react'; -import KeymapService, { KeymapItem, KeymapError } from '../../lib/services/KeymapService'; +import { KeymapItem } from '../../lib/services/KeymapService'; import { ShortcutRecorder } from './ShortcutRecorder'; import getLabel from './utils/getLabel'; import useKeymap from './utils/useKeymap'; @@ -11,7 +11,6 @@ import styles_ from './styles'; const { bridge } = require('electron').remote.require('./bridge'); const { shim } = require('lib/shim'); const { _ } = require('lib/locale'); -const keymapService = KeymapService.instance(); export interface KeymapConfigScreenProps { themeId: number @@ -20,9 +19,9 @@ export interface KeymapConfigScreenProps { export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { const styles = styles_(themeId); - const [keymap, keymapError, setKeymap, setAccelerator] = useKeymap(); - const [recorderError, setRecorderError] = useState(null); const [filter, setFilter] = useState(''); + const [keymapItems, keymapError, overrideKeymapItems, exportCustomKeymap, setAccelerator, resetAccelerator] = useKeymap(); + const [recorderError, setRecorderError] = useState(null); const [editing, enableEditing, disableEditing] = useCommandStatus(); const [hovering, enableHovering, disableHovering] = useCommandStatus(); @@ -34,7 +33,7 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { const handleReset = (event: { commandName: string }) => { const { commandName } = event; - setAccelerator(commandName, keymapService.getDefaultAccelerator(commandName)); + resetAccelerator(commandName); disableEditing(commandName); }; @@ -43,7 +42,7 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { disableEditing(commandName); }; - const handleError = (event: { recorderError: KeymapError }) => { + const handleError = (event: { recorderError: Error }) => { const { recorderError } = event; setRecorderError(recorderError); }; @@ -51,31 +50,33 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { const handleImport = async () => { const filePath = bridge().showOpenDialog({ properties: ['openFile'], - filters: [{ name: 'Joplin Keymap', extensions: ['json'] }], + defaultPath: 'keymap-desktop', + filters: [{ name: 'Joplin Keymaps (keymap-desktop.json)', extensions: ['json'] }], }); if (filePath) { const actualFilePath = filePath[0]; try { const keymapFile = await shim.fsDriver().readFile(actualFilePath, 'utf-8'); - setKeymap(JSON.parse(keymapFile)); + overrideKeymapItems(JSON.parse(keymapFile)); } catch (err) { - bridge().showErrorMessageBox(_('Error importing keymap: %s', err.message)); + bridge().showErrorMessageBox(`${_('An unexpected error occured while importing the keymap!')}\n${err.message}`); } } }; const handleExport = async () => { const filePath = bridge().showSaveDialog({ - filters: [{ name: 'Joplin Keymap', extensions: ['json'] }], + defaultPath: 'keymap-desktop', + filters: [{ name: 'Joplin Keymaps (keymap-desktop.json)', extensions: ['json'] }], }); if (filePath) { try { // KeymapService is already synchronized with the in-state keymap - await keymapService.saveKeymap(filePath); + await exportCustomKeymap(filePath); } catch (err) { - bridge().showErrorMessageBox(_('Error exporting keymap: %s', err.message)); + bridge().showErrorMessageBox(`${_('An unexpected error occured while exporting the keymap!')}\n${err.message}`); } } }; @@ -101,12 +102,12 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { } }; - const renderError = (error: KeymapError) => { + const renderError = (error: Error) => { return (

- {error.altMessage} + {error.message}

@@ -125,7 +126,7 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { onReset={handleReset} onCancel={handleCancel} onError={handleError} - initialAccelerator={accelerator || ''} + initialAccelerator={accelerator || '' /* Because accelerator is null if disabled */} commandName={command} themeId={themeId} /> : @@ -168,6 +169,7 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { +
@@ -176,7 +178,7 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { - {keymap.filter(({ command }) => { + {keymapItems.filter(({ command }) => { const filterLowerCase = filter.toLowerCase(); return (command.toLowerCase().includes(filterLowerCase) || getLabel(command).toLowerCase().includes(filterLowerCase)); }).map(item => renderKeymapRow(item))} diff --git a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx index f2c4308b85f..f6754bc33ff 100644 --- a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx +++ b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx @@ -1,7 +1,7 @@ import * as React from 'react'; import { useState, useEffect, KeyboardEvent } from 'react'; -import KeymapService, { KeymapError } from '../../lib/services/KeymapService'; +import KeymapService from '../../lib/services/KeymapService'; import styles_ from './styles'; const { _ } = require('lib/locale'); @@ -11,7 +11,7 @@ export interface ShortcutRecorderProps { onSave: (event: { commandName: string, accelerator: string }) => void, onReset: (event: { commandName: string }) => void, onCancel: (event: { commandName: string }) => void, - onError: (event: { recorderError: KeymapError }) => void, + onError: (event: { recorderError: Error }) => void, initialAccelerator: string commandName: string, themeId: number @@ -25,6 +25,8 @@ export const ShortcutRecorder = ({ onSave, onReset, onCancel, onError, initialAc useEffect(() => { try { + // Only perform validations if there's an accelerator provided + // Otherwise performing a save means that it's going to be disabled if (accelerator) { keymapService.validateAccelerator(accelerator); keymapService.validateKeymap({ accelerator, command: commandName }); @@ -67,6 +69,7 @@ export const ShortcutRecorder = ({ onSave, onReset, onCancel, onError, initialAc readOnly autoFocus /> + diff --git a/ElectronClient/gui/KeymapConfig/utils/getLabel.ts b/ElectronClient/gui/KeymapConfig/utils/getLabel.ts index 81efdd9f687..099aeaee07b 100644 --- a/ElectronClient/gui/KeymapConfig/utils/getLabel.ts +++ b/ElectronClient/gui/KeymapConfig/utils/getLabel.ts @@ -6,29 +6,30 @@ const { shim } = require('lib/shim'); const commandService = CommandService.instance(); const getLabel = (commandName: string) => { - if (commandService.exists(commandName)) { - return commandService.label(commandName); - } else { - switch (commandName) { - case 'quit': - return _('Quit'); - case 'insertTemplate': - return _('Insert template'); - case 'zoomActualSize': - return _('Actual Size'); - case 'gotoAnything': - return _('Goto Anything...'); - case 'help': - return _('Website and documentation'); - case 'hideApp': - return _('Hide Joplin'); - case 'closeWindow': - return _('Close Window'); - case 'config': - return shim.isMac() ? _('Preferences') : _('Options'); - default: - throw new Error(`Command: ${commandName} is unknown`); - } + if (commandService.exists(commandName)) return commandService.label(commandName); + + // Some commands are not registered in CommandService at the moment + // Following hard-coded labels are used as a workaround + + switch (commandName) { + case 'quit': + return _('Quit'); + case 'insertTemplate': + return _('Insert template'); + case 'zoomActualSize': + return _('Actual Size'); + case 'gotoAnything': + return _('Goto Anything...'); + case 'help': + return _('Website and documentation'); + case 'hideApp': + return _('Hide Joplin'); + case 'closeWindow': + return _('Close Window'); + case 'config': + return shim.isMac() ? _('Preferences') : _('Options'); + default: + throw new Error(`Command: ${commandName} is unknown`); } }; diff --git a/ElectronClient/gui/KeymapConfig/utils/useCommandStatus.ts b/ElectronClient/gui/KeymapConfig/utils/useCommandStatus.ts index 76058811762..f646513770c 100644 --- a/ElectronClient/gui/KeymapConfig/utils/useCommandStatus.ts +++ b/ElectronClient/gui/KeymapConfig/utils/useCommandStatus.ts @@ -17,13 +17,13 @@ const useCommandStatus = (): [CommandStatus, (commandName: string) => void, (com const disableStatus = (commandName: string) => setStatus(prevStatus => ({ ...prevStatus, [commandName]: false })); const enableStatus = (commandName: string) => setStatus(prevStatus => { - // Disable the state of all the commands + // Falsify all the commands; Only one command should be truthy at any given time const newStatus = Object.keys(prevStatus).reduce((accumulator: CommandStatus, command: string) => { accumulator[command] = false; return accumulator; }, {}); - // Enable the state of the appropriate command + // Make the appropriate command truthful newStatus[commandName] = true; return newStatus; }); diff --git a/ElectronClient/gui/KeymapConfig/utils/useKeymap.ts b/ElectronClient/gui/KeymapConfig/utils/useKeymap.ts index 6651bc3b20e..c62e5052be3 100644 --- a/ElectronClient/gui/KeymapConfig/utils/useKeymap.ts +++ b/ElectronClient/gui/KeymapConfig/utils/useKeymap.ts @@ -1,51 +1,76 @@ import { useState, useEffect } from 'react'; -import KeymapService, { KeymapItem, KeymapError } from '../../../lib/services/KeymapService'; +import KeymapService, { KeymapItem } from '../../../lib/services/KeymapService'; const keymapService = KeymapService.instance(); -const useKeymap = (): [KeymapItem[], KeymapError, (keymapItems: KeymapItem[]) => void, (commandName: string, accelerator: string) => void] => { - const [keymap, setKeymap] = useState(() => keymapService.getKeymap()); - const [keymapError, setKeymapError] = useState(null); +// This custom hook provides a synchronized snapshot of the keymap residing at KeymapService +// All the logic regarding altering and interacting with the keymap is isolated from the components + +const useKeymap = (): [ + KeymapItem[], + Error, + (keymapItems: KeymapItem[]) => void, + (customKeymapPath: string) => Promise, + (commandName: string, accelerator: string) => void, + (commandName: string) => void +] => { + const [keymapItems, setKeymapItems] = useState(() => keymapService.getKeymapItems()); + const [keymapError, setKeymapError] = useState(null); const setAccelerator = (commandName: string, accelerator: string) => { - setKeymap(prevKeymap => { + setKeymapItems(prevKeymap => { const newKeymap = [...prevKeymap]; - newKeymap.find(item => item.command === commandName).accelerator = accelerator || null; + newKeymap.find(item => item.command === commandName).accelerator = accelerator || null /* Disabled */; return newKeymap; }); }; - const setKeymapWrapper = (keymapItems: KeymapItem[]) => { - const oldKeymap = [...keymap]; + const resetAccelerator = (commandName: string) => { + const defaultAccelerator = keymapService.getDefaultAccelerator(commandName); + setKeymapItems(prevKeymap => { + const newKeymap = [...prevKeymap]; + + newKeymap.find(item => item.command === commandName).accelerator = defaultAccelerator; + return newKeymap; + }); + }; + + const overrideKeymapItems = (customKeymapItems: KeymapItem[]) => { + const oldKeymapItems = [...customKeymapItems]; + keymapService.initialize(); // Start with a fresh keymap - // Avoid new changes being merged with the old changes - keymapService.initialize(); try { - // Update the in-memory keymap of KeymapService - keymapService.overrideKeymap(keymapItems); - // Synchronize the state with KeymapService + // First, try to update the in-memory keymap of KeymapService + // This function will throw if there are any issues with the new custom keymap + keymapService.overrideKeymap(customKeymapItems); + // Then, update the state with the data from KeymapService // Side-effect: Changes will also be saved to the disk - setKeymap(keymapService.getKeymap()); + setKeymapItems(keymapService.getKeymapItems()); } catch (err) { - // Avoid partially-loading keymap files - keymapService.overrideKeymap(oldKeymap); + // oldKeymapItems includes even the unchanged keymap items + // However, it is not an issue because the logic accounts for such scenarios + keymapService.overrideKeymap(oldKeymapItems); throw err; } }; + const exportCustomKeymap = async (customKeymapPath: string) => { + // KeymapService is already synchronized automatically with the in-state keymap + await keymapService.saveCustomKeymap(customKeymapPath); + }; + useEffect(() => { try { - keymapService.overrideKeymap(keymap); - // Save changes to the disk - keymapService.saveKeymap(); + keymapService.overrideKeymap(keymapItems); + keymapService.saveCustomKeymap(); setKeymapError(null); } catch (err) { setKeymapError(err); } - }, [keymap]); + }, [keymapItems]); - return [keymap, keymapError, setKeymapWrapper, setAccelerator]; + return [keymapItems, keymapError, overrideKeymapItems, exportCustomKeymap, setAccelerator, resetAccelerator]; }; export default useKeymap; diff --git a/ReactNativeClient/lib/services/CommandService.ts b/ReactNativeClient/lib/services/CommandService.ts index dcd0c25c04e..94b8c1b41bd 100644 --- a/ReactNativeClient/lib/services/CommandService.ts +++ b/ReactNativeClient/lib/services/CommandService.ts @@ -293,7 +293,7 @@ export default class CommandService extends BaseService { }; if (command.declaration.role) item.role = command.declaration.role; - if (this.keymapService.hasAccelerator(commandName)) { + if (this.keymapService.acceleratorExists(commandName)) { item.accelerator = this.keymapService.getAccelerator(commandName); } diff --git a/ReactNativeClient/lib/services/KeymapService.ts b/ReactNativeClient/lib/services/KeymapService.ts index caa6855ef6c..e3c86053560 100644 --- a/ReactNativeClient/lib/services/KeymapService.ts +++ b/ReactNativeClient/lib/services/KeymapService.ts @@ -1,6 +1,5 @@ import { KeyboardEvent } from 'react'; -const fs = require('fs-extra'); const BaseService = require('lib/services/BaseService'); const eventManager = require('lib/eventManager'); const { shim } = require('lib/shim'); @@ -12,7 +11,7 @@ const modifiersRegExp = { default: /^(Ctrl|Alt|AltGr|Shift|Super)$/, }; -const defaultKeymap = { +const defaultKeymapItems = { darwin: [ { accelerator: 'Cmd+N', command: 'newNote' }, { accelerator: 'Cmd+T', command: 'newTodo' }, @@ -79,20 +78,6 @@ const defaultKeymap = { ], }; -export class KeymapError extends Error { - // Translated error message for the GUI - private altMessage_: string; - - constructor(message: string, altMessage: string) { - super(message); - this.altMessage_ = altMessage; - } - - get altMessage() { - return this.altMessage_; - } -} - export interface KeymapItem { accelerator: string; command: string; @@ -104,14 +89,15 @@ interface Keymap { export default class KeymapService extends BaseService { private keymap: Keymap; - private defaultKeymap: KeymapItem[]; private platform: string; - private keymapPath: string; + private customKeymapPath: string; + private defaultKeymapItems: KeymapItem[]; constructor() { super(); - // Automatically initialize for the current platform + // By default, initialize for the current platform + // Manual initialization allows testing for other platforms this.initialize(); } @@ -120,159 +106,160 @@ export default class KeymapService extends BaseService { switch (platform) { case 'darwin': - this.defaultKeymap = defaultKeymap.darwin; + this.defaultKeymapItems = defaultKeymapItems.darwin; this.modifiersRegExp = modifiersRegExp.darwin; break; default: - this.defaultKeymap = defaultKeymap.default; + this.defaultKeymapItems = defaultKeymapItems.default; this.modifiersRegExp = modifiersRegExp.default; } this.keymap = {}; - for (let i = 0; i < this.defaultKeymap.length; i++) { - // Make a copy of the KeymapItem before assigning it - // Otherwise we're going to mess up the defaultKeymap array - this.keymap[this.defaultKeymap[i].command] = { ...this.defaultKeymap[i] }; + for (let i = 0; i < this.defaultKeymapItems.length; i++) { + // Keep the original defaultKeymapItems array untouched + // Makes it possible to retrieve the original accelerator later, if needed + this.keymap[this.defaultKeymapItems[i].command] = { ...this.defaultKeymapItems[i] }; } } - public on(eventName: string, callback: Function) { - eventManager.on(eventName, callback); - } - - public off(eventName: string, callback: Function) { - eventManager.off(eventName, callback); - } + async loadCustomKeymap(customKeymapPath: string) { + this.customKeymapPath = customKeymapPath; // Useful for saving the changes later - async loadKeymap(keymapPath: string) { - this.keymapPath = keymapPath; // For saving the changes later + if (await shim.fsDriver().exists(customKeymapPath)) { + this.logger().info(`KeymapService: Loading keymap from file: ${customKeymapPath}`); - if (await fs.exists(keymapPath)) { - this.logger().info(`KeymapService: Loading keymap: ${keymapPath}`); try { - const keymapFile = await shim.fsDriver().readFile(keymapPath, 'utf-8'); - this.overrideKeymap(JSON.parse(keymapFile)); + const customKeymapFile = await shim.fsDriver().readFile(customKeymapPath, 'utf-8'); + // Custom keymaps are supposed to contain an array of keymap items + this.overrideKeymap(JSON.parse(customKeymapFile)); } catch (err) { - const message = err.message ? err.message : ''; - throw new Error(`Failed to load keymap: ${keymapPath}\n${message}`); + const message = err.message || ''; + throw new Error(`${_('Error loading the keymap from file: %s', customKeymapPath)}\n${message}`); } } } - async saveKeymap(keymapPath: string = this.keymapPath) { - this.logger().info(`KeymapService: Saving keymap: ${keymapPath}`); + async saveCustomKeymap(customKeymapPath: string = this.customKeymapPath) { + this.logger().info(`KeymapService: Saving keymap to file: ${customKeymapPath}`); try { - const customKeymap = this.generateCustomKeymap(); - await shim.fsDriver().writeFile(keymapPath, JSON.stringify(customKeymap, null, 2), 'utf-8'); - // Refresh the menu items + // Only the customized keymap items should be saved to the disk + const customKeymapItems = this.getCustomKeymapItems(); + await shim.fsDriver().writeFile(customKeymapPath, JSON.stringify(customKeymapItems, null, 2), 'utf-8'); + + // Refresh the menu items so that the changes are reflected eventManager.emit('keymapChange'); } catch (err) { - const message = err.message ? err.message : ''; - throw new Error(`Failed to save keymap: ${keymapPath}\n${message}`); + const message = err.message || ''; + throw new Error(`${_('Error saving the keymap to file: %s', customKeymapPath)}\n${message}`); } } - hasAccelerator(command: string) { + acceleratorExists(command: string) { return !!this.keymap[command]; } - getAccelerator(command: string) { - const item = this.keymap[command]; - - if (!item) throw new Error(`KeymapService: "${command}" command does not exist!`); - else return item.accelerator; - } - setAccelerator(command: string, accelerator: string) { this.keymap[command].accelerator = accelerator; } - resetAccelerator(command: string) { - const defaultItem = this.defaultKeymap.find((item => item.command === command)); + getAccelerator(command: string) { + const item = this.keymap[command]; + if (!item) throw new Error(`KeymapService: "${command}" command does not exist!`); - if (!defaultItem) throw new Error(`KeymapService: "${command}" command does not exist!`); - else this.setAccelerator(command, defaultItem.accelerator); + return item.accelerator; } getDefaultAccelerator(command: string) { - const defaultItem = this.defaultKeymap.find((item => item.command === command)); - + const defaultItem = this.defaultKeymapItems.find((item => item.command === command)); if (!defaultItem) throw new Error(`KeymapService: "${command}" command does not exist!`); - else return defaultItem.accelerator; - } - getDefaultKeymap() { - return [...this.defaultKeymap]; + return defaultItem.accelerator; } - overrideKeymap(customKeymap: KeymapItem[]) { - for (let i = 0; i < customKeymap.length; i++) { - const item = customKeymap[i]; - - // Throws if there are any issues in the keymap item - this.validateKeymapItem(item); - this.setAccelerator(item.command, item.accelerator); - } - - // Throws whenever there are duplicate Accelerators used in the keymap - this.validateKeymap(); + getCommandNames() { + return Object.keys(this.keymap); } - getKeymap() { + getKeymapItems() { return Object.values(this.keymap); } - getCommandNames() { - return Object.keys(this.keymap); - } - - generateCustomKeymap() { - const customKeymap: KeymapItem[] = []; - this.defaultKeymap.forEach(({ command, accelerator }) => { + getCustomKeymapItems() { + const customkeymapItems: KeymapItem[] = []; + this.defaultKeymapItems.forEach(({ command, accelerator }) => { const currentAccelerator = this.getAccelerator(command); + + // Only the customized/changed keymap items are neccessary for the custom keymap + // Customizations can be merged with the original keymap at the runtime if (this.getAccelerator(command) !== accelerator) { - customKeymap.push({ command, accelerator: currentAccelerator }); + customkeymapItems.push({ command, accelerator: currentAccelerator }); } }); - return customKeymap; + return customkeymapItems; + } + + getDefaultKeymapItems() { + return [...this.defaultKeymapItems]; + } + + overrideKeymap(customKeymapItems: KeymapItem[]) { + try { + for (let i = 0; i < customKeymapItems.length; i++) { + const item = customKeymapItems[i]; + // Validate individual custom keymap items + // Throws if there are any issues in the keymap item + this.validateKeymapItem(item); + this.setAccelerator(item.command, item.accelerator); + } + + // Validate the entire keymap for duplicates + // Throws whenever there are duplicate Accelerators used in the keymap + this.validateKeymap(); + } catch (err) { + this.initialize(); // Discard all the changes if there are any issues + throw err; + } } private validateKeymapItem(item: KeymapItem) { if (!item.hasOwnProperty('command')) { - throw new Error(`Keymap item ${JSON.stringify(item)} is invalid because "command" property is missing.`); + throw new Error(_('Keymap item %s is missing the required "command" property.', JSON.stringify(item))); } else if (!this.keymap.hasOwnProperty(item.command)) { - throw new Error(`Keymap item ${JSON.stringify(item)} is invalid because "${item.command}" is not a valid command.`); + throw new Error(_('Keymap item %s is invalid because %s is not a valid command.', JSON.stringify(item), item.command)); } if (!item.hasOwnProperty('accelerator')) { - throw new Error(`Keymap item ${JSON.stringify(item)} is invalid because "accelerator" property is missing.`); + throw new Error(_('Keymap item %s is missing the required "accelerator" property.', JSON.stringify(item))); } else if (item.accelerator !== null) { try { this.validateAccelerator(item.accelerator); } catch { - throw new Error(`Keymap item ${JSON.stringify(item)} is invalid because "${item.accelerator}" is not a valid accelerator.`); + throw new Error(_('Keymap item %s is invalid because %s is not a valid accelerator.', JSON.stringify(item), item.accelerator)); } } } - validateKeymap(newItem: KeymapItem = null) { + validateKeymap(proposedKeymapItem: KeymapItem = null) { const usedAccelerators = new Set(); - if (newItem) usedAccelerators.add(newItem.accelerator); + + // Validate as if the proposed change is already present in the current keymap + // Helpful for detecting any errors that'll occur, when the proposed change is performed on the keymap + if (proposedKeymapItem) usedAccelerators.add(proposedKeymapItem.accelerator); for (const item of Object.values(this.keymap)) { const [itemAccelerator, itemCommand] = [item.accelerator, item.command]; - if (newItem && itemCommand === newItem.command) continue; + if (proposedKeymapItem && itemCommand === proposedKeymapItem.command) continue; // Ignore the original accelerator if (usedAccelerators.has(item.accelerator)) { const originalItem = Object.values(this.keymap).find(_item => _item.accelerator == item.accelerator); - throw new KeymapError( - 'Keymap configuration contains one or more duplicates.\n' + - `Accelerator "${item.accelerator}" can't be used for both "${item.command}" and "${originalItem.command}" commands.\n` + - 'You have to change the accelerator for any of above commands.', - _('Keymap configuration contains one or more duplicate keyboard shortcuts. This may lead to unexpected behaviour.') - ); + throw new Error(_( + 'Accelerator "%s" is used for "%s" and "%s" commands. This may lead to unexpected behaviour.', + item.accelerator, + item.command, + originalItem.command + )); } else if (itemAccelerator) { usedAccelerators.add(itemAccelerator); } @@ -355,6 +342,14 @@ export default class KeymapService extends BaseService { else return null; } + public on(eventName: string, callback: Function) { + eventManager.on(eventName, callback); + } + + public off(eventName: string, callback: Function) { + eventManager.off(eventName, callback); + } + static instance() { if (this.instance_) return this.instance_; From d5732ff4788e8780ce0affce86df33eae3c0bd10 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Sun, 30 Aug 2020 23:49:38 +0530 Subject: [PATCH 43/48] Add label for Search --- ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx | 2 +- ElectronClient/gui/KeymapConfig/styles/index.ts | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index 542b1311c2f..bce44d0f8f2 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -159,12 +159,12 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { {keymapError && renderError(keymapError)}
+ setFilter(event.target.value)} placeholder={_('Search...')} style={styles.filterInput} - aria-label="Search" /> diff --git a/ElectronClient/gui/KeymapConfig/styles/index.ts b/ElectronClient/gui/KeymapConfig/styles/index.ts index fac6abac826..facd6269774 100644 --- a/ElectronClient/gui/KeymapConfig/styles/index.ts +++ b/ElectronClient/gui/KeymapConfig/styles/index.ts @@ -18,11 +18,18 @@ export default function styles(themeId: number) { filterInput: { ...theme.inputStyle, flexGrow: 1, + minHeight: 29, + alignSelf: 'center', }, recorderInput: { ...theme.inputStyle, minHeight: 29, }, + label: { + ...theme.textStyle, + alignSelf: 'center', + marginRight: 10, + }, table: { ...theme.containerStyle, marginTop: 16, @@ -64,7 +71,7 @@ export default function styles(themeId: number) { }, inlineButton: { ...theme.buttonStyle, - marginLeft: 8, + marginLeft: 12, }, warning: { ...theme.textStyle, From 8ca20ad085c45005ff49100ee4bb39eb08538763 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Mon, 31 Aug 2020 02:43:00 +0530 Subject: [PATCH 44/48] Remove unneccessary translated string --- ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index bce44d0f8f2..f95b2b43d64 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -76,7 +76,7 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { // KeymapService is already synchronized with the in-state keymap await exportCustomKeymap(filePath); } catch (err) { - bridge().showErrorMessageBox(`${_('An unexpected error occured while exporting the keymap!')}\n${err.message}`); + bridge().showErrorMessageBox(err.message); } } }; From e6643b9710fd636b46ca3b9963f9b42ebd3dccb6 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Mon, 31 Aug 2020 02:48:46 +0530 Subject: [PATCH 45/48] Remove unnecessary method from useKeymap hook in favor of directly calling the KeymapService method --- ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx | 8 +++++--- ElectronClient/gui/KeymapConfig/utils/useKeymap.ts | 8 +------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index f95b2b43d64..4415c01407c 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -1,7 +1,7 @@ import * as React from 'react'; import { useState } from 'react'; -import { KeymapItem } from '../../lib/services/KeymapService'; +import KeymapService, { KeymapItem } from '../../lib/services/KeymapService'; import { ShortcutRecorder } from './ShortcutRecorder'; import getLabel from './utils/getLabel'; import useKeymap from './utils/useKeymap'; @@ -12,6 +12,8 @@ const { bridge } = require('electron').remote.require('./bridge'); const { shim } = require('lib/shim'); const { _ } = require('lib/locale'); +const keymapService = KeymapService.instance(); + export interface KeymapConfigScreenProps { themeId: number } @@ -20,7 +22,7 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { const styles = styles_(themeId); const [filter, setFilter] = useState(''); - const [keymapItems, keymapError, overrideKeymapItems, exportCustomKeymap, setAccelerator, resetAccelerator] = useKeymap(); + const [keymapItems, keymapError, overrideKeymapItems, setAccelerator, resetAccelerator] = useKeymap(); const [recorderError, setRecorderError] = useState(null); const [editing, enableEditing, disableEditing] = useCommandStatus(); const [hovering, enableHovering, disableHovering] = useCommandStatus(); @@ -74,7 +76,7 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { if (filePath) { try { // KeymapService is already synchronized with the in-state keymap - await exportCustomKeymap(filePath); + await keymapService.saveCustomKeymap(filePath); } catch (err) { bridge().showErrorMessageBox(err.message); } diff --git a/ElectronClient/gui/KeymapConfig/utils/useKeymap.ts b/ElectronClient/gui/KeymapConfig/utils/useKeymap.ts index c62e5052be3..2f47e21089b 100644 --- a/ElectronClient/gui/KeymapConfig/utils/useKeymap.ts +++ b/ElectronClient/gui/KeymapConfig/utils/useKeymap.ts @@ -10,7 +10,6 @@ const useKeymap = (): [ KeymapItem[], Error, (keymapItems: KeymapItem[]) => void, - (customKeymapPath: string) => Promise, (commandName: string, accelerator: string) => void, (commandName: string) => void ] => { @@ -55,11 +54,6 @@ const useKeymap = (): [ } }; - const exportCustomKeymap = async (customKeymapPath: string) => { - // KeymapService is already synchronized automatically with the in-state keymap - await keymapService.saveCustomKeymap(customKeymapPath); - }; - useEffect(() => { try { keymapService.overrideKeymap(keymapItems); @@ -70,7 +64,7 @@ const useKeymap = (): [ } }, [keymapItems]); - return [keymapItems, keymapError, overrideKeymapItems, exportCustomKeymap, setAccelerator, resetAccelerator]; + return [keymapItems, keymapError, overrideKeymapItems, setAccelerator, resetAccelerator]; }; export default useKeymap; From a6d793bd39ed346baef8cd6ea8ea8e4b23cc0c37 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Tue, 1 Sep 2020 11:56:00 +0530 Subject: [PATCH 46/48] Add tooltips --- ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx | 2 +- ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx | 1 + ReactNativeClient/lib/services/KeymapService.ts | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index 4415c01407c..4787bf501f5 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -96,7 +96,7 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { const renderStatus = (commandName: string) => { if (editing[commandName]) { - return (recorderError && ); + return (recorderError && ); } else if (hovering[commandName]) { return (); } else { diff --git a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx index f6754bc33ff..dbc0e32ac41 100644 --- a/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx +++ b/ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx @@ -66,6 +66,7 @@ export const ShortcutRecorder = ({ onSave, onReset, onCancel, onError, initialAc placeholder={_('Press the shortcut')} onKeyDown={handleKeydown} style={styles.recorderInput} + title={_('Press the shortcut and then press ENTER. Or, press BACKSPACE to clear the shortcut.')} readOnly autoFocus /> diff --git a/ReactNativeClient/lib/services/KeymapService.ts b/ReactNativeClient/lib/services/KeymapService.ts index e3c86053560..935923dbb5c 100644 --- a/ReactNativeClient/lib/services/KeymapService.ts +++ b/ReactNativeClient/lib/services/KeymapService.ts @@ -285,7 +285,7 @@ export default class KeymapService extends BaseService { return isKey || isModifier; }); - if (!isValid) throw new Error(`Accelerator invalid: ${accelerator}`); + if (!isValid) throw new Error(_('Accelerator "%s" is not valid.', accelerator)); } domToElectronAccelerator(event: KeyboardEvent) { From 80b831dcc9410165676c40576b9584ef22d46148 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Tue, 1 Sep 2020 12:19:04 +0530 Subject: [PATCH 47/48] Fix a bug where it failed to find both of duplicate keymap items --- ReactNativeClient/lib/services/KeymapService.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/ReactNativeClient/lib/services/KeymapService.ts b/ReactNativeClient/lib/services/KeymapService.ts index 935923dbb5c..e61e42564fc 100644 --- a/ReactNativeClient/lib/services/KeymapService.ts +++ b/ReactNativeClient/lib/services/KeymapService.ts @@ -252,13 +252,16 @@ export default class KeymapService extends BaseService { const [itemAccelerator, itemCommand] = [item.accelerator, item.command]; if (proposedKeymapItem && itemCommand === proposedKeymapItem.command) continue; // Ignore the original accelerator - if (usedAccelerators.has(item.accelerator)) { - const originalItem = Object.values(this.keymap).find(_item => _item.accelerator == item.accelerator); + if (usedAccelerators.has(itemAccelerator)) { + const originalItem = (proposedKeymapItem && proposedKeymapItem.accelerator === itemAccelerator) + ? proposedKeymapItem + : Object.values(this.keymap).find(_item => _item.accelerator == itemAccelerator); + throw new Error(_( 'Accelerator "%s" is used for "%s" and "%s" commands. This may lead to unexpected behaviour.', - item.accelerator, - item.command, - originalItem.command + itemAccelerator, + originalItem.command, + itemCommand )); } else if (itemAccelerator) { usedAccelerators.add(itemAccelerator); From 88f530054bd65d55c6f04c2614c1ccdfb8f4aa12 Mon Sep 17 00:00:00 2001 From: Anjula Karunarathne Date: Sun, 6 Sep 2020 00:43:14 +0530 Subject: [PATCH 48/48] Remove search label --- ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx index 4787bf501f5..4b5e18116d9 100644 --- a/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx +++ b/ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx @@ -161,7 +161,6 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => { {keymapError && renderError(keymapError)}
- setFilter(event.target.value)}