Desktop: Accessiblity: Make keyboard shortcuts settings screen keyboard-navigable (#11232)

packages/app-desktop/gui/KeymapConfig/KeymapConfigScreen.tsx

@@ -9,8 +9,8 @@ import useCommandStatus from './utils/useCommandStatus';
 import styles_ from './styles';
 import { _ } from '@joplin/lib/locale';
 
-const bridge = require('@electron/remote').require('./bridge').default;
 import shim from '@joplin/lib/shim';
+import bridge from '../../services/bridge';
 
 const keymapService = KeymapService.instance();
 
@@ -25,7 +25,6 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => {
 	const [keymapItems, keymapError, overrideKeymapItems, setAccelerator, resetAccelerator] = useKeymap();
 	const [recorderError, setRecorderError] = useState<Error>(null);
 	const [editing, enableEditing, disableEditing] = useCommandStatus();
-	const [hovering, enableHovering, disableHovering] = useCommandStatus();
 
 	const handleSave = (event: { commandName: string; accelerator: string }) => {
 		const { commandName, accelerator } = event;
@@ -95,13 +94,14 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => {
 	};
 
 	const renderStatus = (commandName: string) => {
-		if (editing[commandName]) {
+		if (!editing[commandName]) {
-			return (recorderError && <i className="fa fa-exclamation-triangle" title={recorderError.message} />);
+			const editLabel = _('Change shortcut for "%s"', getLabel(commandName));
-		} else if (hovering[commandName]) {
+			return <i className="fa fa-pen" role='img' aria-label={editLabel} title={editLabel}/>;
-			return (<i className="fa fa-pen" />);
+		} else if (recorderError) {
-		} else {
+			return <i className="fa fa-exclamation-triangle" role='img' aria-label={recorderError.message} title={recorderError.message} />;
-			return null;
 		}
+
+		return null;
 	};
 
 	const renderError = (error: Error) => {
@@ -117,11 +117,16 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => {
 	};
 
 	const renderKeymapRow = ({ command, accelerator }: KeymapItem) => {
-		const handleClick = () => enableEditing(command);
+		const handleClick = () => {
-		const handleMouseEnter = () => enableHovering(command);
+			if (!editing[command]) {
-		const handleMouseLeave = () => disableHovering(command);
+				enableEditing(command);
+			} else if (recorderError) {
+				void bridge().showErrorMessageBox(recorderError.message);
+			}
+		};
+		const statusContent = renderStatus(command);
 		const cellContent =
-			<div style={styles.tableCell} onMouseEnter={handleMouseEnter} onMouseLeave={handleMouseLeave}>
+			<div className='keymap-shortcut-row-content'>
 				{editing[command] ?
 					<ShortcutRecorder
 						onSave={handleSave}
@@ -139,9 +144,15 @@ export const KeymapConfigScreen = ({ themeId }: KeymapConfigScreenProps) => {
 						}
 					</div>
 				}
-				<div style={styles.tableCellStatus} onClick={handleClick}>
+				<button
-					{renderStatus(command)}
+					className={`flat-button edit ${editing[command] ? '-editing' : ''}`}
-				</div>
+					style={styles.tableCellStatus}
+					aria-live={recorderError ? 'polite' : null}
+					tabIndex={statusContent ? 0 : -1}
+					onClick={handleClick}
+				>
+					{statusContent}
+				</button>
 			</div>;
 
 		return (

packages/app-desktop/gui/KeymapConfig/ShortcutRecorder.tsx

@@ -43,6 +43,12 @@ export const ShortcutRecorder = ({ onSave, onReset, onCancel, onError, initialAc
 	}, [accelerator]);
 
 	const handleKeyDown = (event: KeyboardEvent<HTMLDivElement>) => {
+		// Shift-tab and tab are needed for navigating the shortcuts screen with the keyboard. Do not
+		// .preventDefault.
+		if (event.code === 'Tab' && !event.metaKey && !event.altKey && !event.ctrlKey) {
+			return;
+		}
+
 		event.preventDefault();
 		const newAccelerator = keymapService.domToElectronAccelerator(event);
 
@@ -60,14 +66,25 @@ export const ShortcutRecorder = ({ onSave, onReset, onCancel, onError, initialAc
 		}
 	};
 
+	const hintText = _('Press the shortcut and then press ENTER. Or, press BACKSPACE to clear the shortcut.');
+	const placeholderText = _('Press the shortcut');
+
 	return (
-		<div style={styles.recorderContainer}>
+		<div className='shortcut-recorder' style={styles.recorderContainer}>
 			<input
+				className='shortcut text-input'
+
 				value={accelerator}
-				placeholder={_('Press the shortcut')}
+				aria-label={accelerator ? accelerator : placeholderText}
+				placeholder={placeholderText}
+				title={hintText}
+				aria-description={hintText}
+				aria-invalid={accelerator && !saveAllowed}
+				// With readOnly, aria-live polite seems necessary for screen readers to read
+				// the shortcut as it updates.
+				aria-live='polite'
+
 				onKeyDown={handleKeyDown}
-				style={styles.recorderInput}
-				title={_('Press the shortcut and then press ENTER. Or, press BACKSPACE to clear the shortcut.')}
 				readOnly
 				autoFocus
 			/>

packages/app-desktop/gui/KeymapConfig/style.scss

@@ -0,0 +1,2 @@
+@use "./styles/keymap-shortcut-row-content.scss";
+@use "./styles/shortcut-recorder.scss";

packages/app-desktop/gui/KeymapConfig/styles/index.ts

@@ -14,21 +14,12 @@ export default function styles(themeId: number) {
 				display: 'flex',
 				flexDirection: 'row',
 			},
-			recorderContainer: {
-				padding: 2,
-				flexGrow: 1,
-			},
 			filterInput: {
 				...theme.inputStyle,
 				flexGrow: 1,
 				minHeight: 29,
 				alignSelf: 'center',
 			},
-			recorderInput: {
-				...theme.inputStyle,
-				minHeight: 29,
-				width: '200px',
-			},
 			label: {
 				...theme.textStyle,
 				alignSelf: 'center',
@@ -48,10 +39,6 @@ export default function styles(themeId: number) {
 				...theme.textStyle,
 				width: 'auto',
 			},
-			tableCell: {
-				display: 'flex',
-				flexDirection: 'row',
-			},
 			tableCellContent: {
 				flexGrow: 1,
 				alignSelf: 'center',
@@ -59,6 +46,8 @@ export default function styles(themeId: number) {
 			tableCellStatus: {
 				height: '100%',
 				alignSelf: 'center',
+				border: 'none',
+				background: 'transparent',
 			},
 			kbd: {
 				fontFamily: 'sans-serif',

packages/app-desktop/gui/KeymapConfig/styles/keymap-shortcut-row-content.scss

@@ -0,0 +1,17 @@
+
+.keymap-shortcut-row-content {
+	display: flex;
+	flex-direction: row;
+
+	> .edit {
+		opacity: 0;
+
+		&:focus-visible, &.-editing {
+			opacity: 1;
+		}
+	}
+
+	&:hover > .edit {
+		opacity: 1;
+	}
+}

packages/app-desktop/gui/KeymapConfig/styles/shortcut-recorder.scss

@@ -0,0 +1,13 @@
+.shortcut-recorder {
+	padding: 2px;
+	flex-grow: 1;
+	
+	> .shortcut {
+		min-height: 29px;
+		width: 200px;
+	}
+
+	> .shortcut:focus-visible {
+		border-color: var(--joplin-focus-outline-color);
+	}
+}

packages/app-desktop/gui/styles/index.scss

@@ -9,3 +9,4 @@
 @use './editor-toolbar.scss';
 @use './user-webview-dialog-container.scss';
 @use './dialog-anchor-node.scss';
+@use './text-input.scss';

packages/app-desktop/gui/styles/text-input.scss

@@ -0,0 +1,10 @@
+.text-input {
+	height: 24px;
+	max-height: 24px;
+	border: 1px solid var(--joplin-divider-color);
+	padding-left: 5px;
+	padding-right: 5px;
+	box-sizing: border-box;
+	color: var(--joplin-color);
+	background-color: var(--joplin-background-color);
+}

packages/app-desktop/style.scss

@@ -12,6 +12,7 @@
 @use 'gui/TrashNotification/style.scss' as trash-notification;
 @use 'gui/Sidebar/style.scss' as sidebar-styles;
 @use 'gui/NoteEditor/style.scss' as note-editor-styles;
+@use 'gui/KeymapConfig/style.scss' as keymap-styles;
 @use 'services/plugins/styles/index.scss' as plugins-styles;
 @use 'gui/styles/index.scss' as gui-styles;
 @use 'main.scss' as main;