Desktop: Accessibility: Improve note list keyboard and screen reader accessibility

.eslintignore

@@ -342,8 +342,10 @@ packages/app-desktop/gui/NoteList/commands/focusElementNoteList.js
 packages/app-desktop/gui/NoteList/commands/index.js
 packages/app-desktop/gui/NoteList/utils/canManuallySortNotes.js
 packages/app-desktop/gui/NoteList/utils/types.js
+packages/app-desktop/gui/NoteList/utils/useActiveDescendantId.js
 packages/app-desktop/gui/NoteList/utils/useDragAndDrop.js
 packages/app-desktop/gui/NoteList/utils/useFocusNote.js
+packages/app-desktop/gui/NoteList/utils/useFocusVisible.js
 packages/app-desktop/gui/NoteList/utils/useItemCss.js
 packages/app-desktop/gui/NoteList/utils/useMoveNote.js
 packages/app-desktop/gui/NoteList/utils/useOnKeyDown.js
@@ -364,6 +366,7 @@ packages/app-desktop/gui/NoteListHeader/utils/useContextMenu.js
 packages/app-desktop/gui/NoteListHeader/utils/validateColumns.test.js
 packages/app-desktop/gui/NoteListHeader/utils/validateColumns.js
 packages/app-desktop/gui/NoteListItem/NoteListItem.js
+packages/app-desktop/gui/NoteListItem/utils/getNoteElementIdFromJoplinId.js
 packages/app-desktop/gui/NoteListItem/utils/getNoteTitleHtml.js
 packages/app-desktop/gui/NoteListItem/utils/prepareViewProps.test.js
 packages/app-desktop/gui/NoteListItem/utils/prepareViewProps.js
@@ -458,6 +461,7 @@ packages/app-desktop/gui/style/StyledLink.js
 packages/app-desktop/gui/style/StyledMessage.js
 packages/app-desktop/gui/style/StyledTextInput.js
 packages/app-desktop/gui/utils/NoteListUtils.js
+packages/app-desktop/gui/utils/announceForAccessibility.js
 packages/app-desktop/gui/utils/convertToScreenCoordinates.js
 packages/app-desktop/gui/utils/dragAndDrop.js
 packages/app-desktop/gui/utils/loadScript.js
@@ -468,6 +472,7 @@ packages/app-desktop/integration-tests/markdownEditor.spec.js
 packages/app-desktop/integration-tests/models/GoToAnything.js
 packages/app-desktop/integration-tests/models/MainScreen.js
 packages/app-desktop/integration-tests/models/NoteEditorScreen.js
+packages/app-desktop/integration-tests/models/NoteList.js
 packages/app-desktop/integration-tests/models/SettingsScreen.js
 packages/app-desktop/integration-tests/models/Sidebar.js
 packages/app-desktop/integration-tests/noteList.spec.js

.gitignore

@@ -319,8 +319,10 @@ packages/app-desktop/gui/NoteList/commands/focusElementNoteList.js
 packages/app-desktop/gui/NoteList/commands/index.js
 packages/app-desktop/gui/NoteList/utils/canManuallySortNotes.js
 packages/app-desktop/gui/NoteList/utils/types.js
+packages/app-desktop/gui/NoteList/utils/useActiveDescendantId.js
 packages/app-desktop/gui/NoteList/utils/useDragAndDrop.js
 packages/app-desktop/gui/NoteList/utils/useFocusNote.js
+packages/app-desktop/gui/NoteList/utils/useFocusVisible.js
 packages/app-desktop/gui/NoteList/utils/useItemCss.js
 packages/app-desktop/gui/NoteList/utils/useMoveNote.js
 packages/app-desktop/gui/NoteList/utils/useOnKeyDown.js
@@ -341,6 +343,7 @@ packages/app-desktop/gui/NoteListHeader/utils/useContextMenu.js
 packages/app-desktop/gui/NoteListHeader/utils/validateColumns.test.js
 packages/app-desktop/gui/NoteListHeader/utils/validateColumns.js
 packages/app-desktop/gui/NoteListItem/NoteListItem.js
+packages/app-desktop/gui/NoteListItem/utils/getNoteElementIdFromJoplinId.js
 packages/app-desktop/gui/NoteListItem/utils/getNoteTitleHtml.js
 packages/app-desktop/gui/NoteListItem/utils/prepareViewProps.test.js
 packages/app-desktop/gui/NoteListItem/utils/prepareViewProps.js
@@ -435,6 +438,7 @@ packages/app-desktop/gui/style/StyledLink.js
 packages/app-desktop/gui/style/StyledMessage.js
 packages/app-desktop/gui/style/StyledTextInput.js
 packages/app-desktop/gui/utils/NoteListUtils.js
+packages/app-desktop/gui/utils/announceForAccessibility.js
 packages/app-desktop/gui/utils/convertToScreenCoordinates.js
 packages/app-desktop/gui/utils/dragAndDrop.js
 packages/app-desktop/gui/utils/loadScript.js
@@ -445,6 +449,7 @@ packages/app-desktop/integration-tests/markdownEditor.spec.js
 packages/app-desktop/integration-tests/models/GoToAnything.js
 packages/app-desktop/integration-tests/models/MainScreen.js
 packages/app-desktop/integration-tests/models/NoteEditorScreen.js
+packages/app-desktop/integration-tests/models/NoteList.js
 packages/app-desktop/integration-tests/models/SettingsScreen.js
 packages/app-desktop/integration-tests/models/Sidebar.js
 packages/app-desktop/integration-tests/noteList.spec.js

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

@@ -23,14 +23,18 @@ import useDragAndDrop from './utils/useDragAndDrop';
 import { itemIsInTrash } from '@joplin/lib/services/trash';
 import getEmptyFolderMessage from '@joplin/lib/components/shared/NoteList/getEmptyFolderMessage';
 import Folder from '@joplin/lib/models/Folder';
+import { _ } from '@joplin/lib/locale';
+import useActiveDescendantId from './utils/useActiveDescendantId';
+import getNoteElementIdFromJoplinId from '../NoteListItem/utils/getNoteElementIdFromJoplinId';
+import useFocusVisible from './utils/useFocusVisible';
 const { connect } = require('react-redux');
 
 const commands = {
 	focusElementNoteList,
 };
 
 const NoteList = (props: Props) => {
-	const listRef = useRef(null);
+	const listRef = useRef<HTMLDivElement>(null);
 	const itemRefs = useRef<Record<string, HTMLDivElement>>({});
 	const listRenderer = props.listRenderer;
 
@@ -65,7 +69,8 @@ const NoteList = (props: Props) => {
 		props.notes.length,
 	);
 
-	const focusNote = useFocusNote(itemRefs, props.notes, makeItemIndexVisible);
+	const { activeNoteId, setActiveNoteId } = useActiveDescendantId(props.selectedFolderId, props.selectedNoteIds);
+	const focusNote = useFocusNote(listRef, props.notes, makeItemIndexVisible, setActiveNoteId);
 
 	const moveNote = useMoveNote(
 		props.notesParentType,
@@ -98,6 +103,7 @@ const NoteList = (props: Props) => {
 	const onNoteClick = useOnNoteClick(props.dispatch, focusNote);
 
 	const onKeyDown = useOnKeyDown(
+		activeNoteId,
 		props.selectedNoteIds,
 		moveNote,
 		makeItemIndexVisible,
@@ -177,6 +183,10 @@ const NoteList = (props: Props) => {
 	// 	}
 	// }, [makeItemIndexVisible, previousSelectedNoteIds, previousNoteCount, previousVisible, props.selectedNoteIds, props.notes, props.focusedField, props.visible]);
 
+	const { focusVisible, onFocus, onBlur, onKeyUp } = useFocusVisible(listRef, () => {
+		focusNote(activeNoteId);
+	});
+
 	const highlightedWords = useMemo(() => {
 		if (props.notesParentType === 'Search') {
 			const query = BaseModel.byId(props.searches, props.selectedSearchId);
@@ -197,12 +207,13 @@ const NoteList = (props: Props) => {
 	};
 
 	const renderNotes = () => {
-		if (!props.notes.length) return null;
+		if (!props.notes.length) return [];
 
 		const output: JSX.Element[] = [];
 
 		for (let i = startNoteIndex; i <= endNoteIndex; i++) {
 			const note = props.notes[i];
+			const isSelected = props.selectedNoteIds.includes(note.id);
 
 			output.push(
 				<NoteListItem
@@ -222,7 +233,9 @@ const NoteList = (props: Props) => {
 					isProvisional={props.provisionalNoteIds.includes(note.id)}
 					flow={listRenderer.flow}
 					note={note}
-					isSelected={props.selectedNoteIds.includes(note.id)}
+					tabIndex={-1}
+					focusVisible={focusVisible && activeNoteId === note.id}
+					isSelected={isSelected}
 					isWatched={props.watchedNoteFiles.includes(note.id)}
 					listRenderer={listRenderer}
 					dispatch={props.dispatch}
@@ -264,16 +277,26 @@ const NoteList = (props: Props) => {
 
 	return (
 		<div
+			role='listbox'
+			aria-label={_('Notes')}
+			aria-activedescendant={getNoteElementIdFromJoplinId(activeNoteId)}
+			aria-multiselectable={true}
+			tabIndex={0}
+
+			onFocus={onFocus}
+			onBlur={onBlur}
+
 			className="note-list"
 			style={noteListStyle}
 			ref={listRef}
 			onScroll={onScroll}
 			onKeyDown={onKeyDown}
+			onKeyUp={onKeyUp}
 			onDrop={onDrop}
 		>
 			{renderEmptyList()}
 			{renderFiller('top', topFillerStyle)}
-			<div className="notes" style={notesStyle}>
+			<div className='notes' role='presentation' style={notesStyle}>
 				{renderNotes()}
 			</div>
 			{renderFiller('bottom', bottomFillerStyle)}

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

@@ -18,6 +18,12 @@
 		background-color: var(--joplin-background-color);
 		font-family: var(--joplin-font-family);
 	}
+
+	// focus-visible is communicated by displaying the active item in a different style.
+	// As such, an outline is unnecessary.
+	&:focus-visible {
+		outline: unset;
+	}
 }
 
 .note-list-item {

packages/app-desktop/gui/NoteList/utils/useActiveDescendantId.ts

@@ -0,0 +1,38 @@
+import { useEffect, useRef, useState } from 'react';
+import usePrevious from '@joplin/lib/hooks/usePrevious';
+
+const useActiveDescendantId = (selectedFolderId: string, selectedNoteIds: string[]) => {
+	const selectedNoteIdsRef = useRef(selectedNoteIds);
+	selectedNoteIdsRef.current = selectedNoteIds;
+
+	const [activeNoteId, setActiveNoteId] = useState('');
+	useEffect(() => {
+		setActiveNoteId(selectedNoteIdsRef.current?.[0] ?? '');
+	}, [selectedFolderId]);
+
+	const previousNoteIdsRef = useRef<string[]>();
+	previousNoteIdsRef.current = usePrevious(selectedNoteIds);
+
+	useEffect(() => {
+		const previousNoteIds = previousNoteIdsRef.current ?? [];
+
+		setActiveNoteId(current => {
+			if (selectedNoteIds.includes(current)) {
+				return current;
+			} else {
+				// Prefer added items
+				for (const id of selectedNoteIds) {
+					if (!previousNoteIds.includes(id)) {
+						return id;
+					}
+				}
+
+				return selectedNoteIds[0] ?? '';
+			}
+		});
+	}, [selectedNoteIds]);
+
+	return { activeNoteId, setActiveNoteId };
+};
+
+export default useActiveDescendantId;

packages/app-desktop/gui/NoteList/utils/useFocusNote.ts

@@ -1,44 +1,33 @@
-import shim from '@joplin/lib/shim';
-import { useRef, useCallback, MutableRefObject } from 'react';
+import { useRef, useCallback, RefObject } from 'react';
 import { focus } from '@joplin/lib/utils/focusHandler';
 import { NoteEntity } from '@joplin/lib/services/database/types';
 
 export type FocusNote = (noteId: string)=> void;
-type ItemRefs = MutableRefObject<Record<string, HTMLDivElement>>;
+type ContainerRef = RefObject<HTMLElement>;
 type OnMakeIndexVisible = (i: number)=> void;
+type OnSetActiveId = (id: string)=> void;
 
-const useFocusNote = (itemRefs: ItemRefs, notes: NoteEntity[], makeItemIndexVisible: OnMakeIndexVisible) => {
-	const focusItemIID = useRef(null);
-
+const useFocusNote = (
+	containerRef: ContainerRef, notes: NoteEntity[], makeItemIndexVisible: OnMakeIndexVisible, setActiveNoteId: OnSetActiveId,
+) => {
 	const notesRef = useRef(notes);
 	notesRef.current = notes;
 
 	const focusNote: FocusNote = useCallback((noteId: string) => {
-		// - We need to focus the item manually otherwise focus might be lost when the
-		//   list is scrolled and items within it are being rebuilt.
-		// - We need to use an interval because when leaving the arrow pressed or scrolling
-		//   offscreen items into view, the rendering of items might lag behind and so the
-		//   ref is not yet available at this point.
+		if (noteId) {
+			setActiveNoteId(noteId);
+		}
 
-		if (!itemRefs.current[noteId]) {
-			const targetIndex = notesRef.current.findIndex(note => note.id === noteId);
-			if (targetIndex > -1) {
-				makeItemIndexVisible(targetIndex);
-			}
+		// The note list container should have focus even when a note list item is visibly selected.
+		// The visibly focused item is determined by activeNoteId and is communicated to accessibility
+		// tools using aria- attributes
+		focus('useFocusNote', containerRef.current);
 
-			if (focusItemIID.current) shim.clearInterval(focusItemIID.current);
-			focusItemIID.current = shim.setInterval(() => {
-				if (itemRefs.current[noteId]) {
-					focus('useFocusNote1', itemRefs.current[noteId]);
-					shim.clearInterval(focusItemIID.current);
-					focusItemIID.current = null;
-				}
-			}, 10);
-		} else {
-			if (focusItemIID.current) shim.clearInterval(focusItemIID.current);
-			focus('useFocusNote2', itemRefs.current[noteId]);
+		const targetIndex = notesRef.current.findIndex(note => note.id === noteId);
+		if (targetIndex > -1) {
+			makeItemIndexVisible(targetIndex);
 		}
-	}, [itemRefs, makeItemIndexVisible]);
+	}, [containerRef, makeItemIndexVisible, setActiveNoteId]);
 
 	return focusNote;
 };

packages/app-desktop/gui/NoteList/utils/useFocusVisible.ts

@@ -0,0 +1,45 @@
+import { useCallback, useState, useRef, RefObject } from 'react';
+
+const useFocusVisible = (containerRef: RefObject<HTMLElement>, onFocusEnter: ()=> void) => {
+	const [focusVisible, setFocusVisible] = useState(false);
+
+	const onFocusEnterRef = useRef(onFocusEnter);
+	onFocusEnterRef.current = onFocusEnter;
+	const focusVisibleRef = useRef(focusVisible);
+	focusVisibleRef.current = focusVisible;
+
+	const onFocusVisible = useCallback(() => {
+		if (!focusVisibleRef.current) {
+			setFocusVisible(true);
+			onFocusEnterRef.current();
+		}
+	}, []);
+
+	const onFocus = useCallback(() => {
+		if (containerRef.current.matches(':focus-visible')) {
+			onFocusVisible();
+		}
+	}, [containerRef, onFocusVisible]);
+
+	const onKeyUp = useCallback(() => {
+		if (containerRef.current.contains(document.activeElement)) {
+			onFocusVisible();
+		}
+	}, [containerRef, onFocusVisible]);
+
+	const onBlur = useCallback(() => setFocusVisible(false), []);
+
+	return {
+		focusVisible,
+		onFocus,
+
+		// When focus becomes visible due to a key press, but the item was already
+		// focused, no new focus event is emitted and the browser :focus-visible doesn't
+		// change. As such, we need to handle this case ourselves.
+		onKeyUp,
+
+		onBlur,
+	};
+};
+
+export default useFocusVisible;