Desktop: Fix errors found by automated accessibility testing (#11246)

Co-authored-by: Laurent Cozic <laurent22@users.noreply.github.com>

.eslintignore

@@ -511,10 +511,12 @@ packages/app-desktop/integration-tests/util/createStartupArgs.js
 packages/app-desktop/integration-tests/util/extendedExpect.js
 packages/app-desktop/integration-tests/util/firstNonDevToolsWindow.js
 packages/app-desktop/integration-tests/util/getImageSourceSize.js
+packages/app-desktop/integration-tests/util/setDarkMode.js
 packages/app-desktop/integration-tests/util/setFilePickerResponse.js
 packages/app-desktop/integration-tests/util/setMessageBoxResponse.js
 packages/app-desktop/integration-tests/util/test.js
 packages/app-desktop/integration-tests/util/waitForNextOpenPath.js
+packages/app-desktop/integration-tests/wcag.spec.js
 packages/app-desktop/playwright.config.js
 packages/app-desktop/plugins/GotoAnything.js
 packages/app-desktop/services/autoUpdater/AutoUpdaterService.test.js

.gitignore

@@ -487,10 +487,12 @@ packages/app-desktop/integration-tests/util/createStartupArgs.js
 packages/app-desktop/integration-tests/util/extendedExpect.js
 packages/app-desktop/integration-tests/util/firstNonDevToolsWindow.js
 packages/app-desktop/integration-tests/util/getImageSourceSize.js
+packages/app-desktop/integration-tests/util/setDarkMode.js
 packages/app-desktop/integration-tests/util/setFilePickerResponse.js
 packages/app-desktop/integration-tests/util/setMessageBoxResponse.js
 packages/app-desktop/integration-tests/util/test.js
 packages/app-desktop/integration-tests/util/waitForNextOpenPath.js
+packages/app-desktop/integration-tests/wcag.spec.js
 packages/app-desktop/playwright.config.js
 packages/app-desktop/plugins/GotoAnything.js
 packages/app-desktop/services/autoUpdater/AutoUpdaterService.test.js

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

@@ -224,7 +224,8 @@ const Button = React.forwardRef((props: Props, ref: any) => {
 	function renderIcon() {
 		if (!props.iconName) return null;
 		return <StyledIcon
-			aria-label={props.iconLabel ?? ''}
+			aria-label={props.iconLabel ?? undefined}
+			aria-hidden={!props.iconLabel}
 			animation={props.iconAnimation}
 			mr={iconOnly ? '0' : '6px'}
 			color={props.color}

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

@@ -438,7 +438,7 @@ class ConfigScreenComponent extends React.Component<any, any> {
 		}
 
 		return (
-			<div className="config-screen" style={{ display: 'flex', flexDirection: 'row', height: this.props.style.height }}>
+			<div className="config-screen" role="main" style={{ display: 'flex', flexDirection: 'row', height: this.props.style.height }}>
 				<Sidebar
 					selection={this.state.selectedSectionName}
 					onSelectionChange={this.sidebar_selectionChange}

packages/app-desktop/gui/ConfigScreen/Sidebar.tsx

@@ -56,7 +56,7 @@ export const StyledDivider = styled.div`
 	border-bottom: 1px solid ${(props: StyleProps) => props.theme.dividerColor};
 	background-color: ${(props: StyleProps) => props.theme.selectedColor2};
 	font-size: ${(props: StyleProps) => Math.round(props.theme.fontSize)}px;
-	opacity: 0.5;
+	opacity: 0.58;
 `;
 
 export const StyledListItemLabel = styled.span`
@@ -131,9 +131,9 @@ export default function Sidebar(props: Props) {
 				onKeyDown={onKeyDown}
 			>
 				<StyledListItemIcon
-					aria-label=''
 					className={Setting.sectionNameToIcon(section.name, AppType.Desktop)}
 					role='img'
+					aria-hidden='true'
 				/>
 				<StyledListItemLabel>
 					{Setting.sectionNameToLabel(section.name)}

packages/app-desktop/gui/ConfigScreen/controls/SettingDescription.tsx

@@ -6,7 +6,7 @@ interface Props {
 }
 
 const SettingDescription: React.FC<Props> = props => {
-	return props.text ? <div className='setting-description' id={props.id}>{props.text}</div> : null;
+	return <div className={`setting-description ${!props.text ? '-empty' : ''}`} id={props.id}>{props.text}</div>;
 };
 
 export default SettingDescription;

packages/app-desktop/gui/ConfigScreen/styles/setting-description.scss

@@ -6,4 +6,9 @@
 	font-style: italic;
 	max-width: 70em;
 	margin-top: 5px;
+
+	&.-empty {
+		margin: 0;
+		padding: 0;
+	}
 }

packages/app-desktop/gui/FolderIconBox.tsx

@@ -17,7 +17,7 @@ export default function(props: Props) {
 	} else if (folderIcon.type === FolderIconType.DataUrl) {
 		return <img style={{ width, height, opacity }} src={folderIcon.dataUrl} />;
 	} else if (folderIcon.type === FolderIconType.FontAwesome) {
-		return <i style={{ fontSize: 18, width, opacity }} className={folderIcon.name} role='img'></i>;
+		return <i style={{ fontSize: 18, width, opacity }} className={folderIcon.name} role='img' aria-hidden={true}></i>;
 	} else {
 		throw new Error(`Unsupported folder icon type: ${folderIcon.type}`);
 	}

packages/app-desktop/gui/ItemList.tsx

@@ -211,7 +211,6 @@ class ItemList<ItemType> extends React.Component<Props<ItemType>, State> {
 				id={this.props.id}
 				role={this.props.role}
 				aria-label={this.props['aria-label']}
-				aria-setsize={items.length}
 
 				onScroll={this.onScroll}
 				onKeyDown={this.onKeyDown}

packages/app-desktop/gui/MainScreen.tsx

@@ -633,10 +633,12 @@ class MainScreenComponent extends React.Component<Props, State> {
 			},
 
 			editor: () => {
-				return <NoteEditor
+				return <div className='note-editor-wrapper' role='main' aria-label={_('Note')}>
-					windowId={defaultWindowId}
+					<NoteEditor
-					key={key}
+						windowId={defaultWindowId}
-				/>;
+						key={key}
+					/>
+				</div>;
 			},
 		};
 

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

@@ -391,6 +391,7 @@ const CodeMirror = (props: NoteBodyEditorProps, ref: ForwardedRef<NoteBodyEditor
 			spellcheckEnabled: Setting.value('editor.spellcheckBeta'),
 			keymap: keyboardMode,
 			indentWithTabs: true,
+			editorLabel: _('Markdown editor'),
 		};
 	}, [
 		props.contentMarkupLanguage, props.disabled, props.keyboardMode, styles.globalTheme,

packages/app-desktop/gui/NoteEditor/NoteTitle/NoteTitleBar.tsx

@@ -116,6 +116,7 @@ export default function NoteTitleBar(props: Props) {
 				type="text"
 				ref={props.titleInputRef}
 				placeholder={props.isProvisional ? (props.noteIsTodo ? _('Creating new to-do...') : _('Creating new note...')) : ''}
+				aria-label={props.isProvisional ? undefined : _('Note title')}
 				style={styles.titleInput}
 				readOnly={props.disabled}
 				onChange={props.onTitleChange}

packages/app-desktop/gui/NoteList/NoteList2.tsx

@@ -282,12 +282,13 @@ const NoteList = (props: Props) => {
 		onItemContextMenu({ itemId: activeNoteId });
 	}, [onItemContextMenu, activeNoteId]);
 
+	const hasNotes = !!props.notes.length;
 	return (
 		<div
-			role='listbox'
+			role={hasNotes ? 'listbox' : 'group'}
-			aria-label={_('Notes')}
+			aria-label={hasNotes ? _('Notes') : null}
-			aria-activedescendant={getNoteElementIdFromJoplinId(activeNoteId)}
+			aria-activedescendant={activeNoteId ? getNoteElementIdFromJoplinId(activeNoteId) : undefined}
-			aria-multiselectable={true}
+			aria-multiselectable={hasNotes ? true : undefined}
 			tabIndex={0}
 
 			onFocus={onFocus}

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

@@ -178,7 +178,7 @@ export default function NoteListWrapper(props: Props) {
 	};
 
 	return (
-		<StyledRoot>
+		<StyledRoot role='navigation' aria-label={_('Note list')}>
 			<NoteListControls
 				height={controlHeight}
 				width={noteListSize.width}

packages/app-desktop/gui/NoteTextViewer.tsx

@@ -6,6 +6,7 @@ import { focus } from '@joplin/lib/utils/focusHandler';
 import { ForwardedRef, forwardRef, RefObject, useContext, useEffect, useImperativeHandle, useMemo, useRef, useState } from 'react';
 import { WindowIdContext } from './NewWindowOrIFrame';
 import useDocument from './hooks/useDocument';
+import { _ } from '@joplin/lib/locale';
 
 interface Props {
 	// eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied
@@ -224,6 +225,7 @@ const NoteTextViewer = forwardRef((props: Props, ref: ForwardedRef<NoteViewerCon
 			style={viewerStyle}
 			allow='clipboard-write=(self) fullscreen=(self) autoplay=(self) local-fonts=(self) encrypted-media=(self)'
 			allowFullScreen={true}
+			aria-label={_('Note editor')}
 			src={`joplin-content://note-viewer/${__dirname}/note-viewer/index.html`}
 		></iframe>
 	);

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

@@ -74,7 +74,7 @@ const SidebarComponent = (props: Props) => {
 	);
 
 	return (
-		<StyledRoot className="sidebar">
+		<StyledRoot className="sidebar" role='navigation' aria-label={_('Sidebar')}>
 			<div style={{ flex: 1 }}><FolderAndTagList/></div>
 			<div style={{ flex: 0, padding: theme.mainPadding }}>
 				{syncReportComp}

packages/app-desktop/gui/Sidebar/hooks/useOnRenderListWrapper.tsx

@@ -33,7 +33,6 @@ const useOnRenderListWrapper = ({ selectedIndex, onKeyDown }: Props) => {
 			<div
 				role='tree'
 				className='sidebar-list-items-wrapper'
-				aria-setsize={listItems.length}
 				tabIndex={allowContainerFocus ? 0 : undefined}
 				onKeyDown={onKeyDown}
 			>

packages/app-desktop/gui/Sidebar/listItemComponents/AllNotesItem.tsx

@@ -60,7 +60,7 @@ const AllNotesItem: React.FC<Props> = props => {
 			itemCount={props.itemCount}
 		>
 			<EmptyExpandLink/>
-			<StyledAllNotesIcon aria-label='' role='img' className='icon-notes'/>
+			<StyledAllNotesIcon aria-hidden='true' role='img' className='icon-notes'/>
 			<StyledListItemAnchor
 				className="list-item"
 				isSpecialItem={true}

packages/app-desktop/gui/Sidebar/listItemComponents/HeaderItem.tsx

@@ -59,7 +59,7 @@ const HeaderItem: React.FC<Props> = props => {
 			onDrop={props.onDrop}
 		>
 			<StyledHeader onClick={onClick}>
-				<StyledHeaderIcon aria-label='' role='img' className={item.iconName}/>
+				<StyledHeaderIcon aria-hidden='true' role='img' className={item.iconName}/>
 				<StyledHeaderLabel>{item.label}</StyledHeaderLabel>
 			</StyledHeader>
 		</ListItemWrapper>

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

@@ -36,7 +36,7 @@ export default function ToolbarButton(props: Props) {
 	const iconName = getProp(props, 'iconName');
 	if (iconName) {
 		const iconProps: React.HTMLProps<HTMLDivElement> = {
-			'aria-label': '',
+			'aria-hidden': true,
 			role: 'img',
 			className: `toolbar-icon ${title ? '-has-title' : ''} ${iconName}`,
 		};

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

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

packages/app-desktop/gui/styles/note-editor-wrapper.scss

@@ -0,0 +1,6 @@
+
+.note-editor-wrapper {
+	flex-grow: 1;
+	height: 100%;
+	width: 100%;
+}

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

@@ -4,9 +4,11 @@ import { ElectronApplication, Locator, Page } from '@playwright/test';
 
 export default class Sidebar {
 	public readonly container: Locator;
+	public readonly allNotes: Locator;
 
 	public constructor(page: Page, private mainScreen: MainScreen) {
 		this.container = page.locator('.rli-sideBar');
+		this.allNotes = this.container.getByText('All notes');
 	}
 
 	public async createNewFolder(title: string) {

packages/app-desktop/integration-tests/util/setDarkMode.ts

@@ -0,0 +1,9 @@
+import { ElectronApplication } from '@playwright/test';
+
+const setDarkMode = (app: ElectronApplication, darkMode: boolean) => {
+	return app.evaluate(({ nativeTheme }, darkMode) => {
+		nativeTheme.themeSource = darkMode ? 'dark' : 'light';
+	}, darkMode);
+};
+
+export default setDarkMode;

packages/app-desktop/integration-tests/util/test.ts

@@ -4,6 +4,7 @@ import { _electron as electron, Page, ElectronApplication, test as base } from '
 import uuid from '@joplin/lib/uuid';
 import createStartupArgs from './createStartupArgs';
 import firstNonDevToolsWindow from './firstNonDevToolsWindow';
+import setDarkMode from './setDarkMode';
 
 
 type StartWithPluginsResult = { app: ElectronApplication; mainWindow: Page };
@@ -61,6 +62,7 @@ export const test = base.extend<JoplinFixtures>({
 	electronApp: async ({ profileDirectory }, use) => {
 		const startupArgs = createStartupArgs(profileDirectory);
 		const electronApp = await electron.launch({ args: startupArgs });
+		await setDarkMode(electronApp, false);
 
 		await use(electronApp);
 

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

@@ -0,0 +1,68 @@
+import { test, expect } from './util/test';
+import MainScreen from './models/MainScreen';
+import SettingsScreen from './models/SettingsScreen';
+import AxeBuilder from '@axe-core/playwright';
+import { Page } from '@playwright/test';
+
+const createScanner = (page: Page) => {
+	return new AxeBuilder({ page })
+		.disableRules(['page-has-heading-one'])
+		.setLegacyMode(true);
+};
+
+// Fade-in transitions can cause color contrast issues if still running
+// during a scan.
+// See https://github.com/dequelabs/axe-core-npm/issues/952
+const waitForAnimationsToEnd = (page: Page) => {
+	return page.locator('body').evaluate(element => {
+		const animationPromises = element
+			.getAnimations({ subtree: true })
+			.map(animation => animation.finished);
+		return Promise.all(animationPromises);
+	});
+};
+
+const expectNoViolations = async (page: Page) => {
+	await waitForAnimationsToEnd(page);
+	const results = await createScanner(page).analyze();
+	expect(results.violations).toEqual([]);
+};
+
+
+test.describe('wcag', () => {
+	for (const tabName of ['General', 'Plugins']) {
+		test(`should not detect significant issues in the settings screen ${tabName} tab`, async ({ electronApp, mainWindow }) => {
+			const mainScreen = new MainScreen(mainWindow);
+			await mainScreen.waitFor();
+
+			await mainScreen.openSettings(electronApp);
+
+			// Should be on the settings screen
+			const settingsScreen = new SettingsScreen(mainWindow);
+			await settingsScreen.waitFor();
+
+			const tabLocator = settingsScreen.getTabLocator(tabName);
+			await tabLocator.click();
+			await expect(tabLocator).toBeFocused();
+
+			await expectNoViolations(mainWindow);
+		});
+	}
+
+	test('should not detect significant issues in the main screen with an open note', async ({ mainWindow }) => {
+		const mainScreen = new MainScreen(mainWindow);
+		await mainScreen.waitFor();
+
+		await mainScreen.createNewNote('Test');
+
+		// For now, activate all notes to make it active. When inactive, it causes a contrast warning.
+		// This seems to be allowed under WCAG 2.2 SC 1.4.3 under the "Incidental" exception.
+		await mainScreen.sidebar.allNotes.click();
+
+		// Ensure that `:hover` styling is consistent between tests:
+		await mainScreen.noteEditor.noteTitleInput.hover();
+
+		await expectNoViolations(mainWindow);
+	});
+});
+

packages/app-desktop/package.json

@@ -124,6 +124,7 @@
   "homepage": "https://github.com/laurent22/joplin#readme",
   "devDependencies": {
     "7zip-bin": "5.2.0",
+    "@axe-core/playwright": "4.10.0",
     "@electron/rebuild": "3.6.0",
     "@joplin/default-plugins": "~3.2",
     "@joplin/tools": "~3.2",

packages/app-mobile/components/NoteEditor/NoteEditor.tsx

@@ -348,6 +348,8 @@ function NoteEditor(props: Props, ref: any) {
 		autocompleteMarkup: Setting.value('editor.autocompleteMarkup'),
 
 		indentWithTabs: true,
+
+		editorLabel: _('Markdown editor'),
 	}), [props.themeId, props.readOnly]);
 
 	const injectedJavaScript = `

packages/editor/CodeMirror/configFromSettings.ts

@@ -56,6 +56,7 @@ const configFromSettings = (settings: EditorSettings) => {
 			autocapitalize: 'sentence',
 			autocorrect: settings.spellcheckEnabled ? 'true' : 'false',
 			spellcheck: settings.spellcheckEnabled ? 'true' : 'false',
+			'aria-label': settings.editorLabel,
 		}),
 		EditorState.readOnly.of(settings.readOnly),
 		indentUnit.of(settings.indentWithTabs ? '\t' : '    '),

packages/editor/CodeMirror/testUtil/createEditorSettings.ts

@@ -17,6 +17,7 @@ const createEditorSettings = (themeId: number) => {
 		themeData,
 
 		indentWithTabs: true,
+		editorLabel: 'Markdown editor',
 	};
 
 	return editorSettings;

packages/editor/types.ts

@@ -168,6 +168,8 @@ export interface EditorSettings {
 	readOnly: boolean;
 
 	indentWithTabs: boolean;
+
+	editorLabel: string;
 }
 
 export type LogMessageCallback = (message: string)=> void;

packages/lib/themes/light.ts

@@ -14,7 +14,7 @@ const theme: Theme = {
 	colorCorrect: 'green', // Opposite of colorError
 	colorWarn: 'rgb(228,86,0)',
 	colorWarnUrl: '#155BDA',
-	colorFaded: '#7C8B9E', // For less important text
+	colorFaded: '#627184', // For less important text
 	dividerColor: '#dddddd',
 	selectedColor: '#e5e5e5',
 	urlColor: '#155BDA',
@@ -32,7 +32,7 @@ const theme: Theme = {
 	// It's dark text over gray background.
 	backgroundColor3: '#F4F5F6',
 	backgroundColorHover3: '#CBDAF1',
-	color3: '#738598',
+	color3: '#627284',
 
 	// Color scheme "4" is used for secondary-style buttons. It makes a white
 	// button with blue text.

packages/tools/cspell/dictionary4.txt

@@ -137,6 +137,7 @@ runtimes
 onnx
 onnxruntime
 treeitem
+WCAG
 qrcode
 Rocketbook
 datamatrix

yarn.lock

@@ -1434,6 +1434,17 @@ __metadata:
   languageName: node
   linkType: hard
 
+"@axe-core/playwright@npm:4.10.0":
+  version: 4.10.0
+  resolution: "@axe-core/playwright@npm:4.10.0"
+  dependencies:
+    axe-core: ~4.10.0
+  peerDependencies:
+    playwright-core: ">= 1.0.0"
+  checksum: cba9b7f625c8ed510e661d8d014f7e924fb85175b918b818cb808eec848c3e8a2cbbe2960ccf8dd4b9f84a4a4ea33eeeace48de2560a4a5777dd0ee6281d0c81
+  languageName: node
+  linkType: hard
+
 "@babel/code-frame@npm:7.10.4, @babel/code-frame@npm:~7.10.4":
   version: 7.10.4
   resolution: "@babel/code-frame@npm:7.10.4"
@@ -8168,6 +8179,7 @@ __metadata:
   resolution: "@joplin/app-desktop@workspace:packages/app-desktop"
   dependencies:
     7zip-bin: 5.2.0
+    "@axe-core/playwright": 4.10.0
     "@electron/notarize": 2.3.2
     "@electron/rebuild": 3.6.0
     "@electron/remote": 2.1.2
@@ -16062,6 +16074,13 @@ __metadata:
   languageName: node
   linkType: hard
 
+"axe-core@npm:~4.10.0":
+  version: 4.10.1
+  resolution: "axe-core@npm:4.10.1"
+  checksum: 1e71bc4b7cdad6e99dad9e4098a174932ed69052e7400e0fb57b585fff1764cc541580db375c643755250da7d68f811ba05fe0636c31d4238aa16e3f31587869
+  languageName: node
+  linkType: hard
+
 "axios@npm:^0.25.0":
   version: 0.25.0
   resolution: "axios@npm:0.25.0"