Improve UI/UX

.eslintignore

@@ -77,7 +77,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/useEditing.js
+ElectronClient/gui/KeymapConfig/utils/useCommandStatus.js
 ElectronClient/gui/KeymapConfig/utils/useKeymap.js
 ElectronClient/gui/MainScreen/commands/editAlarm.js
 ElectronClient/gui/MainScreen/commands/exportPdf.js

.gitignore

@@ -68,7 +68,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/useEditing.js
+ElectronClient/gui/KeymapConfig/utils/useCommandStatus.js
 ElectronClient/gui/KeymapConfig/utils/useKeymap.js
 ElectronClient/gui/MainScreen/commands/editAlarm.js
 ElectronClient/gui/MainScreen/commands/exportPdf.js

ElectronClient/gui/KeymapConfig/KeymapConfigScreen.tsx

@@ -1,11 +1,11 @@
 import * as React from 'react';
 import { useState } from 'react';
 
-import KeymapService, { KeymapItem } from '../../lib/services/KeymapService';
+import KeymapService, { KeymapItem, KeymapError } from '../../lib/services/KeymapService';
 import { ShortcutRecorder } from './ShortcutRecorder';
 import getLabel from './utils/getLabel';
 import useKeymap from './utils/useKeymap';
-import useEditing from './utils/useEditing';
+import useCommandStatus from './utils/useCommandStatus';
 import styles_ from './styles';
 
 const { _ } = require('lib/locale.js');
@@ -18,17 +18,19 @@ export interface KeymapConfigScreenProps {
 export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => {
 	const styles = styles_(themeId);
 
+	const [keymap, keymapError, setAccelerator] = useKeymap();
+	const [recorderError, setRecorderError] = useState<KeymapError>(null);
 	const [filter, setFilter] = useState('');
-	const [keymap, setAccelerator, error] = useKeymap();
-	const [editing, enableEditing, disableEditing] = useEditing();
+	const [editing, enableEditing, disableEditing] = useCommandStatus();
+	const [hovering, enableHovering, disableHovering] = useCommandStatus();
 
 	const handleSave = (event: { commandName: string, accelerator: string }) => {
 		const { commandName, accelerator } = event;
 		setAccelerator(commandName, accelerator);
 		disableEditing(commandName);
 	};
 
-	const hanldeReset = (event: { commandName: string }) => {
+	const handleReset = (event: { commandName: string }) => {
 		const { commandName } = event;
 		setAccelerator(commandName, keymapService.getDefaultAccelerator(commandName));
 		disableEditing(commandName);
@@ -39,6 +41,11 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => {
 		disableEditing(commandName);
 	};
 
+	const handleError = (event: { recorderError: KeymapError }) => {
+		const { recorderError } = event;
+		setRecorderError(recorderError);
+	};
+
 	const renderAccelerator = (accelerator: string) => {
 		return (
 			<div>
@@ -50,37 +57,50 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => {
 		);
 	};
 
-	const renderStatus = (command: string) => {
-		if (!error) return null;
-		const { invalidAccelerator, invalidKeymapItem, duplicateAccelerators, duplicateKeymapItems } = error.details;
+	const renderStatus = (commandName: string) => {
+		if (editing[commandName]) {
+			return (recorderError && <i className="fa fa-exclamation-triangle" />);
+		} else if (hovering[commandName]) {
+			return (<i className="fa fa-pen" />);
+		} else { return null; }
+	};
 
-		if ((invalidAccelerator && invalidKeymapItem.command === command) ||
-			(duplicateAccelerators && (duplicateKeymapItems[0].command === command || duplicateKeymapItems[1].command === command))) {
-			return '❌';
-		} else {
-			return null;
-		}
+	const renderError = (error: KeymapError) => {
+		return (
+			<div style={styles.warning}>
+				<p style={styles.text}>
+					<span>
+						{error.altMessage}
+					</span>
+				</p>
+			</div>
+		);
 	};
 
 	const renderKeymapRow = ({ command, accelerator }: KeymapItem) => {
 		const handleClick = () => enableEditing(command);
-		const cellContent = editing[command]
-			? <ShortcutRecorder
-				onSave={handleSave}
-				onReset={hanldeReset}
-				onCancel={handleCancel}
-				initialAccelerator={accelerator}
-				commandName={command}
-				themeId={themeId}
-			/>
-			: <div onClick={handleClick} style={styles.tableCell}>
-				<div style={styles.tableCellShortcut}>
-					{accelerator
-						? renderAccelerator(accelerator)
-						: <div style={styles.disabled}>{_('Disabled')}</div>
-					}
-				</div>
-				<div style={styles.tableCellStatus}>
+		const handleMouseEnter = () => enableHovering(command);
+		const handleMouseLeave = () => disableHovering(command);
+		const cellContent =
+			<div style={styles.tableCell} onMouseEnter={handleMouseEnter} onMouseLeave={handleMouseLeave}>
+				{editing[command] ?
+					<ShortcutRecorder
+						onSave={handleSave}
+						onReset={handleReset}
+						onCancel={handleCancel}
+						onError={handleError}
+						initialAccelerator={accelerator}
+						commandName={command}
+						themeId={themeId}
+					/> :
+					<div style={styles.tableCellContent} onClick={handleClick}>
+						{accelerator
+							? renderAccelerator(accelerator)
+							: <div style={styles.disabled}>{_('Disabled')}</div>
+						}
+					</div>
+				}
+				<div style={styles.tableCellStatus} onClick={handleClick}>
 					{renderStatus(command)}
 				</div>
 			</div>;
@@ -99,20 +119,7 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => {
 
 	return (
 		<div>
-			{error &&
-				<div style={styles.warning}>
-					<p style={styles.text}>
-						<span>
-							{error.details.duplicateAccelerators
-								? _('Keymap configuration contains duplicates. Change or disable one of the shortcuts to continue.')
-								: error.details.invalidAccelerator
-									? _('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
-							}
-						</span>
-					</p>
-				</div>
-			}
+			{keymapError && renderError(keymapError)}
 			<div style={styles.container}>
 				<div style={styles.actionsContainer}>
 					<input

ElectronClient/gui/KeymapConfig/ShortcutRecorder.tsx

@@ -1,7 +1,7 @@
 import * as React from 'react';
-import { useState, KeyboardEvent } from 'react';
+import { useState, useEffect, KeyboardEvent } from 'react';
 
-import KeymapService from '../../lib/services/KeymapService';
+import KeymapService, { KeymapError } from '../../lib/services/KeymapService';
 import styles_ from './styles';
 
 const { _ } = require('lib/locale.js');
@@ -11,23 +11,39 @@ 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,
 	initialAccelerator: string
 	commandName: string,
 	themeId: number
 }
 
-export const ShortcutRecorder = ({ onSave, onReset, onCancel, initialAccelerator, commandName, themeId }: ShortcutRecorderProps) => {
+export const ShortcutRecorder = ({ onSave, onReset, onCancel, onError, initialAccelerator, commandName, themeId }: ShortcutRecorderProps) => {
 	const styles = styles_(themeId);
 
 	const [accelerator, setAccelerator] = useState(initialAccelerator);
+	const [saveAllowed, setSaveAllowed] = useState(true);
+
+	useEffect(() => {
+		try {
+			keymapService.validateAccelerator(accelerator);
+			keymapService.validateKeymap({ accelerator, command: commandName });
+			// Discard previous errors
+			onError({ recorderError: null });
+			setSaveAllowed(true);
+		} catch (recorderError) {
+			onError({ recorderError });
+			setSaveAllowed(false);
+		}
+	}, [accelerator]);
 
 	const handleKeydown = (event: KeyboardEvent<HTMLDivElement>) => {
 		event.preventDefault();
 		const newAccelerator = keymapService.domToElectronAccelerator(event);
 
 		switch (newAccelerator) {
 		case 'Enter':
-			return onSave({ commandName, accelerator });
+			if (saveAllowed) return onSave({ commandName, accelerator });
+			break;
 		case 'Escape':
 			return onCancel({ commandName });
 		case 'Backspace':
@@ -48,7 +64,7 @@ export const ShortcutRecorder = ({ onSave, onReset, onCancel, initialAccelerator
 				readOnly
 				autoFocus
 			/>
-			<button style={styles.inlineButton} onClick={() => onSave({ commandName, accelerator })}>
+			<button style={styles.inlineButton} disabled={!saveAllowed} onClick={() => onSave({ commandName, accelerator })}>
 				{_('Save')}
 			</button>
 			<button style={styles.inlineButton} onClick={() => onReset({ commandName })}>

ElectronClient/gui/KeymapConfig/styles/index.ts

@@ -13,6 +13,7 @@ export default function styles(themeId: number) {
 			},
 			recorderContainer: {
 				padding: 2,
+				flexGrow: 1,
 			},
 			filterInput: {
 				...theme.inputStyle,
@@ -39,13 +40,14 @@ export default function styles(themeId: number) {
 			tableCell: {
 				display: 'flex',
 				flexDirection: 'row',
-				cursor: 'pointer',
 			},
-			tableCellShortcut: {
+			tableCellContent: {
 				flexGrow: 1,
+				alignSelf: 'center',
 			},
 			tableCellStatus: {
-				// No specific styling at the moment
+				height: '100%',
+				alignSelf: 'center',
 			},
 			kbd: {
 				fontFamily: 'sans-serif',

ElectronClient/gui/KeymapConfig/utils/useCommandStatus.ts

@@ -0,0 +1,34 @@
+import { useState } from 'react';
+import KeymapService from '../../../lib/services/KeymapService';
+
+const keymapService = KeymapService.instance();
+
+interface CommandStatus {
+	[commandName: string]: boolean
+}
+
+const useCommandStatus = (): [CommandStatus, (commandName: string) => void, (commandName: string) => void] => {
+	const [status, setStatus] = useState<CommandStatus>(() =>
+		keymapService.getCommandNames().reduce((accumulator: CommandStatus, command: string) => {
+			accumulator[command] = false;
+			return accumulator;
+		}, {})
+	);
+
+	const disableStatus = (commandName: string) => setStatus(prevStatus => ({ ...prevStatus, [commandName]: false }));
+	const enableStatus = (commandName: string) => setStatus(prevStatus => {
+		// Disable the state of all the commands
+		const newStatus = Object.keys(prevStatus).reduce((accumulator: CommandStatus, command: string) => {
+			accumulator[command] = false;
+			return accumulator;
+		}, {});
+
+		// Enable the state of the appropriate command
+		newStatus[commandName] = true;
+		return newStatus;
+	});
+
+	return [status, enableStatus, disableStatus];
+};
+
+export default useCommandStatus;

ElectronClient/gui/KeymapConfig/utils/useKeymap.ts

@@ -3,9 +3,9 @@ import KeymapService, { KeymapItem, KeymapError } from '../../../lib/services/Ke
 
 const keymapService = KeymapService.instance();
 
-const useKeymap = (): [KeymapItem[], (commandName: string, accelerator: string) => void, KeymapError] => {
+const useKeymap = (): [KeymapItem[], KeymapError, (commandName: string, accelerator: string) => void] => {
 	const [keymap, setKeymap] = useState<KeymapItem[]>(() => keymapService.getKeymap());
-	const [error, setError] = useState<KeymapError>(null);
+	const [keymapError, setKeymapError] = useState<KeymapError>(null);
 
 	const setAccelerator = (commandName: string, accelerator: string) => {
 		setKeymap(prevKeymap => {
@@ -18,16 +18,16 @@ const useKeymap = (): [KeymapItem[], (commandName: string, accelerator: string)
 
 	useEffect(() => {
 		try {
-			keymapService.setKeymap(keymap);
-			// Save changes to the disk
+			// Save changes to the drive
 			keymapService.saveKeymap();
-			setError(null);
+			keymapService.setKeymap(keymap);
+			setKeymapError(null);
 		} catch (err) {
-			setError(err);
+			setKeymapError(err);
 		}
 	}, [keymap]);
 
-	return [keymap, setAccelerator, error];
+	return [keymap, keymapError, setAccelerator];
 };
 
 export default useKeymap;