Skip to content

Commit

Permalink
Desktop: Accessibility: Improve note list keyboard and screen reader …
Browse files Browse the repository at this point in the history
…accessibility (#10940)
  • Loading branch information
personalizedrefrigerator authored Aug 31, 2024
1 parent 4f2d0c8 commit d2b7d64
Show file tree
Hide file tree
Showing 25 changed files with 371 additions and 72 deletions.
5 changes: 5 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
33 changes: 28 additions & 5 deletions packages/app-desktop/gui/NoteList/NoteList2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -98,6 +103,7 @@ const NoteList = (props: Props) => {
const onNoteClick = useOnNoteClick(props.dispatch, focusNote);

const onKeyDown = useOnKeyDown(
activeNoteId,
props.selectedNoteIds,
moveNote,
makeItemIndexVisible,
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand All @@ -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}
Expand Down Expand Up @@ -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)}
Expand Down
6 changes: 6 additions & 0 deletions packages/app-desktop/gui/NoteList/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
38 changes: 38 additions & 0 deletions packages/app-desktop/gui/NoteList/utils/useActiveDescendantId.ts
Original file line number Diff line number Diff line change
@@ -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;
45 changes: 17 additions & 28 deletions packages/app-desktop/gui/NoteList/utils/useFocusNote.ts
Original file line number Diff line number Diff line change
@@ -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;
};
Expand Down
45 changes: 45 additions & 0 deletions packages/app-desktop/gui/NoteList/utils/useFocusVisible.ts
Original file line number Diff line number Diff line change
@@ -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;
Loading

0 comments on commit d2b7d64

Please sign in to comment.