Desktop: Accessibility: Improve keyboard navigation in the Markdown and note toolbar (#10819)

packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/Toolbar.tsx

@@ -6,6 +6,7 @@ import { connect } from 'react-redux';
 import { AppState } from '../../../../app.reducer';
 import ToolbarButtonUtils, { ToolbarButtonInfo } from '@joplin/lib/services/commands/ToolbarButtonUtils';
 import stateToWhenClauseContext from '../../../../services/commands/stateToWhenClauseContext';
+import { _ } from '@joplin/lib/locale';
 const { buildStyle } = require('@joplin/lib/theme');
 
 interface ToolbarProps {
@@ -29,7 +30,14 @@ const toolbarButtonUtils = new ToolbarButtonUtils(CommandService.instance());
 
 function Toolbar(props: ToolbarProps) {
 	const styles = styles_(props);
-	return <ToolbarBase style={styles.root} items={props.toolbarButtonInfos} disabled={!!props.disabled} />;
+	return (
+		<ToolbarBase
+			style={styles.root}
+			items={props.toolbarButtonInfos}
+			disabled={!!props.disabled}
+			aria-label={_('Editor actions')}
+		/>
+	);
 }
 
 const mapStateToProps = (state: AppState) => {

packages/app-desktop/gui/NoteToolbar/NoteToolbar.tsx

@@ -6,6 +6,7 @@ import ToolbarButtonUtils, { ToolbarButtonInfo } from '@joplin/lib/services/comm
 import stateToWhenClauseContext from '../../services/commands/stateToWhenClauseContext';
 const { connect } = require('react-redux');
 import { buildStyle } from '@joplin/lib/theme';
+import { _ } from '@joplin/lib/locale';
 
 interface NoteToolbarProps {
 	themeId: number;
@@ -29,7 +30,14 @@ function styles_(props: NoteToolbarProps) {
 
 function NoteToolbar(props: NoteToolbarProps) {
 	const styles = styles_(props);
-	return <ToolbarBase style={styles.root} items={props.toolbarButtonInfos} disabled={props.disabled}/>;
+	return (
+		<ToolbarBase
+			style={styles.root}
+			items={props.toolbarButtonInfos}
+			disabled={props.disabled}
+			aria-label={_('Note')}
+		/>
+	);
 }
 
 const toolbarButtonUtils = new ToolbarButtonUtils(CommandService.instance());

packages/app-desktop/gui/ToggleEditorsButton/ToggleEditorsButton.tsx

@@ -1,6 +1,7 @@
 import * as React from 'react';
 import styles_ from './styles';
 import { ToolbarButtonInfo } from '@joplin/lib/services/commands/ToolbarButtonUtils';
+import { _ } from '@joplin/lib/locale';
 
 export enum Value {
 	Markdown = 'markdown',
@@ -11,6 +12,8 @@ export interface Props {
 	themeId: number;
 	value: Value;
 	toolbarButtonInfo: ToolbarButtonInfo;
+	tabIndex?: number;
+	buttonRef?: React.Ref<HTMLButtonElement>;
 }
 
 export default function ToggleEditorsButton(props: Props) {
@@ -18,14 +21,17 @@ export default function ToggleEditorsButton(props: Props) {
 
 	return (
 		<button
+			ref={props.buttonRef}
 			style={style.button}
 			disabled={!props.toolbarButtonInfo.enabled}
 			aria-label={props.toolbarButtonInfo.tooltip}
+			aria-description={_('Switch to the %s Editor', props.value !== Value.Markdown ? _('Markdown') : _('Rich Text'))}
 			title={props.toolbarButtonInfo.tooltip}
 			type="button"
 			className={`tox-tbtn ${props.value}-active`}
 			aria-pressed="false"
 			onClick={props.toolbarButtonInfo.onClick}
+			tabIndex={props.tabIndex}
 		>
 			<div style={style.leftInnerButton}>
 				<i style={style.leftIcon} className="fab fa-markdown"></i>

packages/app-desktop/gui/ToolbarBase.tsx

@@ -2,98 +2,207 @@ import * as React from 'react';
 import ToolbarButton from './ToolbarButton/ToolbarButton';
 import ToggleEditorsButton, { Value } from './ToggleEditorsButton/ToggleEditorsButton';
 import ToolbarSpace from './ToolbarSpace';
-const { connect } = require('react-redux');
-const { themeStyle } = require('@joplin/lib/theme');
+import { ToolbarButtonInfo } from '@joplin/lib/services/commands/ToolbarButtonUtils';
+import { AppState } from '../app.reducer';
+import { connect } from 'react-redux';
+import { useCallback, useMemo, useRef, useState } from 'react';
+import { focus } from '@joplin/lib/utils/focusHandler';
+
+interface ToolbarItemInfo extends ToolbarButtonInfo {
+	type?: string;
+}
 
 interface Props {
 	themeId: number;
-	// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
-	style: any;
-	// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
-	items: any[];
+	style: React.CSSProperties;
+	items: ToolbarItemInfo[];
 	disabled: boolean;
+	'aria-label': string;
 }
 
-// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
-class ToolbarBaseComponent extends React.Component<Props, any> {
+const getItemType = (item: ToolbarItemInfo) => {
+	return item.type ?? 'button';
+};
 
-	public render() {
-		const theme = themeStyle(this.props.themeId);
+const isFocusable = (item: ToolbarItemInfo) => {
+	if (!item.enabled) {
+		return false;
+	}
 
-		// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
-		const style: any = { display: 'flex',
-			flexDirection: 'row',
-			boxSizing: 'border-box',
-			backgroundColor: theme.backgroundColor3,
-			padding: theme.toolbarPadding,
-			paddingRight: theme.mainPadding, ...this.props.style };
+	return getItemType(item) === 'button';
+};
 
-		// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
-		const groupStyle: any = {
-			display: 'flex',
-			flexDirection: 'row',
-			boxSizing: 'border-box',
-			minWidth: 0,
-		};
+const useCategorizedItems = (items: ToolbarItemInfo[]) => {
+	return useMemo(() => {
+		const itemsLeft: ToolbarItemInfo[] = [];
+		const itemsCenter: ToolbarItemInfo[] = [];
+		const itemsRight: ToolbarItemInfo[] = [];
 
-		// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
-		const leftItemComps: any[] = [];
-		// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
-		const centerItemComps: any[] = [];
-		// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
-		const rightItemComps: any[] = [];
-
-		if (this.props.items) {
-			for (let i = 0; i < this.props.items.length; i++) {
-				const o = this.props.items[i];
-				let key = o.iconName ? o.iconName : '';
-				key += o.title ? o.title : '';
-				key += o.name ? o.name : '';
-				const itemType = !('type' in o) ? 'button' : o.type;
-
-				if (!key) key = `${o.type}_${i}`;
-
-				const props = {
-					key: key,
-					themeId: this.props.themeId,
-					disabled: this.props.disabled,
-					...o,
-				};
-
-				if (o.name === 'toggleEditors') {
-					rightItemComps.push(<ToggleEditorsButton
-						key={o.name}
-						value={Value.Markdown}
-						themeId={this.props.themeId}
-						toolbarButtonInfo={o}
-					/>);
-				} else if (itemType === 'button') {
-					const target = ['historyForward', 'historyBackward', 'toggleExternalEditing'].includes(o.name) ? leftItemComps : centerItemComps;
-					target.push(<ToolbarButton {...props} />);
-				} else if (itemType === 'separator') {
-					centerItemComps.push(<ToolbarSpace {...props} />);
+		if (items) {
+			for (const item of items) {
+				const type = getItemType(item);
+				if (item.name === 'toggleEditors') {
+					itemsRight.push(item);
+				} else if (type === 'button') {
+					const target = ['historyForward', 'historyBackward', 'toggleExternalEditing'].includes(item.name) ? itemsLeft : itemsCenter;
+					target.push(item);
+				} else if (type === 'separator') {
+					itemsCenter.push(item);
 				}
 			}
 		}
 
-		return (
-			<div className="editor-toolbar" style={style}>
-				<div style={groupStyle}>
-					{leftItemComps}
-				</div>
-				<div style={groupStyle}>
-					{centerItemComps}
-				</div>
-				<div style={{ ...groupStyle, flex: 1, justifyContent: 'flex-end' }}>
-					{rightItemComps}
-				</div>
-			</div>
-		);
-	}
-}
+		return {
+			itemsLeft,
+			itemsCenter,
+			itemsRight,
+			allItems: itemsLeft.concat(itemsCenter, itemsRight),
+		};
+	}, [items]);
+};
 
-// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
-const mapStateToProps = (state: any) => {
+const useKeyboardHandler = (
+	setSelectedIndex: React.Dispatch<React.SetStateAction<number>>,
+	focusableItems: ToolbarItemInfo[],
+) => {
+	const onKeyDown: React.KeyboardEventHandler<HTMLElement> = useCallback(event => {
+		let direction = 0;
+		if (event.code === 'ArrowRight') {
+			direction = 1;
+		} else if (event.code === 'ArrowLeft') {
+			direction = -1;
+		}
+
+		let handled = true;
+		if (direction !== 0) {
+			setSelectedIndex(index => {
+				let newIndex = (index + direction) % focusableItems.length;
+				if (newIndex < 0) {
+					newIndex += focusableItems.length;
+				}
+				return newIndex;
+			});
+		} else if (event.code === 'End') {
+			setSelectedIndex(focusableItems.length - 1);
+		} else if (event.code === 'Home') {
+			setSelectedIndex(0);
+		} else {
+			handled = false;
+		}
+
+		if (handled) {
+			event.preventDefault();
+		}
+	}, [focusableItems, setSelectedIndex]);
+
+	return onKeyDown;
+};
+
+const ToolbarBaseComponent: React.FC<Props> = props => {
+	const { itemsLeft, itemsCenter, itemsRight, allItems } = useCategorizedItems(props.items);
+
+	const [selectedIndex, setSelectedIndex] = useState(0);
+	const focusableItems = useMemo(() => {
+		return allItems.filter(isFocusable);
+	}, [allItems]);
+	const containerRef = useRef<HTMLDivElement|null>(null);
+	const containerHasFocus = !!containerRef.current?.contains(document.activeElement);
+
+	let keyCounter = 0;
+	const renderItem = (o: ToolbarItemInfo, indexInFocusable: number) => {
+		let key = o.iconName ? o.iconName : '';
+		key += o.title ? o.title : '';
+		key += o.name ? o.name : '';
+		const itemType = !('type' in o) ? 'button' : o.type;
+
+		if (!key) key = `${o.type}_${keyCounter++}`;
+
+		const buttonProps = {
+			key,
+			themeId: props.themeId,
+			disabled: props.disabled || !o.enabled,
+			...o,
+		};
+
+		const tabIndex = indexInFocusable === (selectedIndex % focusableItems.length) ? 0 : -1;
+		const setButtonRefCallback = (button: HTMLButtonElement) => {
+			if (tabIndex === 0 && containerHasFocus) {
+				focus('ToolbarBase', button);
+			}
+		};
+
+		if (o.name === 'toggleEditors') {
+			return <ToggleEditorsButton
+				key={o.name}
+				buttonRef={setButtonRefCallback}
+				value={Value.Markdown}
+				themeId={props.themeId}
+				toolbarButtonInfo={o}
+				tabIndex={tabIndex}
+			/>;
+		} else if (itemType === 'button') {
+			return (
+				<ToolbarButton
+					tabIndex={tabIndex}
+					buttonRef={setButtonRefCallback}
+					{...buttonProps}
+				/>
+			);
+		} else if (itemType === 'separator') {
+			return <ToolbarSpace {...buttonProps} />;
+		}
+
+		return null;
+	};
+
+	let focusableIndex = 0;
+	const renderList = (items: ToolbarItemInfo[]) => {
+		const result: React.ReactNode[] = [];
+
+		for (const item of items) {
+			result.push(renderItem(item, focusableIndex));
+			if (isFocusable(item)) {
+				focusableIndex ++;
+			}
+		}
+
+		return result;
+	};
+
+	const leftItemComps = renderList(itemsLeft);
+	const centerItemComps = renderList(itemsCenter);
+	const rightItemComps = renderList(itemsRight);
+
+	const onKeyDown = useKeyboardHandler(
+		setSelectedIndex,
+		focusableItems,
+	);
+
+	return (
+		<div
+			ref={containerRef}
+			className='editor-toolbar'
+			style={props.style}
+
+			role='toolbar'
+			aria-label={props['aria-label']}
+
+			onKeyDown={onKeyDown}
+		>
+			<div className='group'>
+				{leftItemComps}
+			</div>
+			<div className='group'>
+				{centerItemComps}
+			</div>
+			<div className='group -right'>
+				{rightItemComps}
+			</div>
+		</div>
+	);
+};
+
+const mapStateToProps = (state: AppState) => {
 	return { themeId: state.settings.theme };
 };
 

packages/app-desktop/gui/ToolbarButton/ToolbarButton.tsx

@@ -1,6 +1,6 @@
 import * as React from 'react';
 import { ToolbarButtonInfo } from '@joplin/lib/services/commands/ToolbarButtonUtils';
-import { StyledRoot, StyledIconSpan, StyledIconI } from './styles';
+import { StyledIconSpan, StyledIconI } from './styles';
 
 interface Props {
 	readonly themeId: number;
@@ -10,6 +10,9 @@ interface Props {
 	readonly iconName?: string;
 	readonly disabled?: boolean;
 	readonly backgroundHover?: boolean;
+	readonly tabIndex?: number;
+
+	buttonRef?: React.Ref<HTMLButtonElement>;
 }
 
 function isFontAwesomeIcon(iconName: string) {
@@ -34,7 +37,7 @@ export default function ToolbarButton(props: Props) {
 	const iconName = getProp(props, 'iconName');
 	if (iconName) {
 		const IconClass = isFontAwesomeIcon(iconName) ? StyledIconI : StyledIconSpan;
-		icon = <IconClass className={iconName} title={title}/>;
+		icon = <IconClass className={iconName} aria-label='' hasTitle={!!title} role='img'/>;
 	}
 
 	// Keep this for legacy compatibility but for consistency we should use "disabled" prop
@@ -42,8 +45,9 @@ export default function ToolbarButton(props: Props) {
 	if (isEnabled === null) isEnabled = true;
 	if (props.disabled) isEnabled = false;
 
-	const classes = ['button'];
+	const classes = ['button', 'toolbar-button'];
 	if (!isEnabled) classes.push('disabled');
+	if (title) classes.push('-has-title');
 
 	const onClick = getProp(props, 'onClick');
 	const style: React.CSSProperties = {
@@ -51,25 +55,27 @@ export default function ToolbarButton(props: Props) {
 		overflow: 'hidden',
 		textOverflow: 'ellipsis' };
 	const disabled = !isEnabled;
+
 	return (
-		<StyledRoot
+		<button
 			className={classes.join(' ')}
 			title={tooltip}
-			href="#"
-			hasTitle={!!title}
 			onClick={() => {
 				if (isEnabled && onClick) onClick();
 			}}
+			ref={props.buttonRef}
 
 			// At least on MacOS, the disabled HTML prop isn't sufficient for the screen reader
 			// to read the element as disable. For this, aria-disabled is necessary.
 			disabled={disabled}
+			aria-label={!title ? tooltip : undefined}
+			aria-description={title ? tooltip : undefined}
 			aria-disabled={!isEnabled}
-			role='button'
+			tabIndex={props.tabIndex}
 		>
 			{icon}
 			<span style={style}>{title}</span>
-		</StyledRoot>
+		</button>
 	);
 }
 

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

@@ -3,46 +3,15 @@ import { ThemeStyle } from '@joplin/lib/theme';
 const styled = require('styled-components').default;
 const { css } = require('styled-components');
 
-interface RootProps {
-	readonly theme: ThemeStyle;
-	readonly disabled: boolean;
-	readonly hasTitle: boolean;
-}
-
-export const StyledRoot = styled.a<RootProps>`
-	opacity: ${(props: RootProps) => props.disabled ? 0.3 : 1};
-	height: ${(props: RootProps) => props.theme.toolbarHeight}px;
-	min-height: ${(props: RootProps) => props.theme.toolbarHeight}px;
-	width: ${(props: RootProps) => props.hasTitle ? 'auto' : `${props.theme.toolbarHeight}px`};
-	max-width: ${(props: RootProps) => props.hasTitle ? 'auto' : `${props.theme.toolbarHeight}px`};
-	display: flex;
-	align-items: center;
-	justify-content: center;
-	cursor: default;
-	border-radius: 3px;
-	box-sizing: border-box;
-	color: ${(props: RootProps) => props.theme.color3};
-	font-size: ${(props: RootProps) => props.theme.toolbarIconSize * 0.8}px;
-	padding-left: 5px;
-	padding-right: 5px;
-	white-space: nowrap;
-	overflow: hidden;
-	text-overflow: ellipsis;
-
-	&:hover {
-		background-color: ${(props: RootProps) => props.disabled ? 'none' : props.theme.backgroundColorHover3};
-	}
-`;
-
 interface IconProps {
 	readonly theme: ThemeStyle;
-	readonly title: string;
+	readonly hasTitle: boolean;
 }
 
 const iconStyle = css<IconProps>`
 	font-size: ${(props: IconProps) => props.theme.toolbarIconSize}px;
 	color: ${(props: IconProps) => props.theme.color3};
-	margin-right: ${(props: IconProps) => props.title ? 5 : 0}px;
+	margin-right: ${(props: IconProps) => props.hasTitle ? 5 : 0}px;
 	pointer-events: none; /* Need this to get button tooltip to work */
 `;
 

packages/app-desktop/gui/styles/editor-toolbar.scss

@@ -0,0 +1,21 @@
+
+.editor-toolbar {
+	display: flex;
+	flex-direction: row;
+	box-sizing: border-box;
+	background-color: var(--joplin-background-color3);
+	padding: var(--joplin-toolbar-padding);
+	padding-right: var(--joplin-main-padding);
+
+	> .group {
+		display: flex;
+		flex-direction: row;
+		box-sizing: border-box;
+		min-width: 0;
+
+		&.-right {
+			flex: 1;
+			justify-content: flex-end;
+		}
+	}
+}

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

@@ -1,4 +1,6 @@
 
 @use './dialog-modal-layer.scss';
 @use './user-webview-dialog.scss';
-@use './prompt-dialog.scss';
+@use './prompt-dialog.scss';
+@use './toolbar-button.scss';
+@use './editor-toolbar.scss';

packages/app-desktop/gui/styles/toolbar-button.scss

@@ -0,0 +1,42 @@
+
+.toolbar-button {
+	border: none;
+	background: transparent;
+	color: inherit;
+	padding: 0;
+	opacity: 1;
+	height: var(--joplin-toolbar-height);
+	min-height: var(--joplin-toolbar-height);
+	width: var(--joplin-toolbar-height);
+	max-width: var(--joplin-toolbar-height);
+
+	&.-has-title {
+		width: auto;
+		max-width: unset;
+	}
+
+	&:disabled {
+		opacity: 0.3;
+	}
+
+	display: flex;
+	align-items: center;
+	justify-content: center;
+	cursor: default;
+	border-radius: 3px;
+	box-sizing: border-box;
+	color: var(--joplin-color3);
+	font-size: calc(var(--joplin-toolbar-icon-size) * 0.8);
+	padding-left: 5px;
+	padding-right: 5px;
+	white-space: nowrap;
+	overflow: hidden;
+	text-overflow: ellipsis;
+
+
+	&:not(:disabled) {
+		&:hover, &:focus-visible {
+			background-color: var(--joplin-background-color-hover3);
+		}
+	}
+}

packages/app-desktop/integration-tests/markdownEditor.spec.ts

@@ -26,5 +26,42 @@ test.describe('markdownEditor', () => {
 		await expect(image).toBeAttached();
 		await expect(await getImageSourceSize(image)).toMatchObject([117, 30]);
 	});
+
+	test('arrow keys should navigate the toolbar', async ({ mainWindow }) => {
+		const mainScreen = new MainScreen(mainWindow);
+		await mainScreen.waitFor();
+
+		await mainScreen.createNewNote('Note 1');
+		await mainScreen.createNewNote('Note 2');
+		const noteEditor = mainScreen.noteEditor;
+		await noteEditor.focusCodeMirrorEditor();
+
+		// Escape, then Shift+Tab should focus the toolbar
+		await mainWindow.keyboard.press('Escape');
+		await mainWindow.keyboard.press('Shift+Tab');
+
+		// Should focus the first item by default, the "back" arrow (back to "Note 1")
+		const firstItemLocator = noteEditor.toolbarButtonLocator('Back');
+		await expect(firstItemLocator).toBeFocused();
+
+		// Left arrow should wrap to the end
+		await mainWindow.keyboard.press('ArrowLeft');
+		const lastItemLocator = noteEditor.toolbarButtonLocator('Toggle editors');
+		await expect(lastItemLocator).toBeFocused();
+
+		await mainWindow.keyboard.press('ArrowRight');
+		await expect(firstItemLocator).toBeFocused();
+
+		// ArrowRight should skip disabled items (Forward).
+		await mainWindow.keyboard.press('ArrowRight');
+		await expect(noteEditor.toolbarButtonLocator('Toggle external editing')).toBeFocused();
+
+		// Home/end should navigate to the first/last items
+		await mainWindow.keyboard.press('End');
+		await expect(lastItemLocator).toBeFocused();
+
+		await mainWindow.keyboard.press('Home');
+		await expect(firstItemLocator).toBeFocused();
+	});
 });
 

packages/app-desktop/integration-tests/models/NoteEditorScreen.ts

@@ -14,8 +14,12 @@ export default class NoteEditorPage {
 		this.codeMirrorEditor = this.containerLocator.locator('.cm-editor');
 		this.richTextEditor = this.containerLocator.locator('iframe[title="Rich Text Area"]');
 		this.noteTitleInput = this.containerLocator.locator('.title-input');
-		this.attachFileButton = this.containerLocator.locator('[title^="Attach file"]');
-		this.toggleEditorsButton = this.containerLocator.locator('[title^="Toggle editors"]');
+		this.attachFileButton = this.containerLocator.getByRole('button', { name: 'Attach file' });
+		this.toggleEditorsButton = this.containerLocator.getByRole('button', { name: 'Toggle editors' });
+	}
+
+	public toolbarButtonLocator(title: string) {
+		return this.containerLocator.getByRole('button', { name: title });
 	}
 
 	public getNoteViewerIframe() {

packages/app-desktop/integration-tests/settings.spec.ts

@@ -8,7 +8,7 @@ test.describe('settings', () => {
 		await mainScreen.waitFor();
 
 		// Sort order buttons should be visible by default
-		const sortOrderLocator = mainScreen.noteListContainer.locator('[title^="Toggle sort order"]');
+		const sortOrderLocator = mainScreen.noteListContainer.getByRole('button', { name: 'Toggle sort order' });
 		await expect(sortOrderLocator).toBeVisible();
 
 		await mainScreen.openSettings(electronApp);