diff --git a/.eslintignore b/.eslintignore index 3131c04ff..7113d1127 100644 --- a/.eslintignore +++ b/.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 diff --git a/.gitignore b/.gitignore index 14f306540..0353d8276 100644 --- a/.gitignore +++ b/.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 diff --git a/packages/app-desktop/gui/NoteList/NoteList2.tsx b/packages/app-desktop/gui/NoteList/NoteList2.tsx index 5208eb70c..5a5027657 100644 --- a/packages/app-desktop/gui/NoteList/NoteList2.tsx +++ b/packages/app-desktop/gui/NoteList/NoteList2.tsx @@ -23,6 +23,10 @@ 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 = { @@ -30,7 +34,7 @@ const commands = { }; const NoteList = (props: Props) => { - const listRef = useRef(null); + const listRef = useRef(null); const itemRefs = useRef>({}); 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( { 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 (
{renderEmptyList()} {renderFiller('top', topFillerStyle)} -
+
{renderNotes()}
{renderFiller('bottom', bottomFillerStyle)} diff --git a/packages/app-desktop/gui/NoteList/style.scss b/packages/app-desktop/gui/NoteList/style.scss index 6b3e03502..cb042e19e 100644 --- a/packages/app-desktop/gui/NoteList/style.scss +++ b/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 { diff --git a/packages/app-desktop/gui/NoteList/utils/useActiveDescendantId.ts b/packages/app-desktop/gui/NoteList/utils/useActiveDescendantId.ts new file mode 100644 index 000000000..a936e388c --- /dev/null +++ b/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(); + 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; diff --git a/packages/app-desktop/gui/NoteList/utils/useFocusNote.ts b/packages/app-desktop/gui/NoteList/utils/useFocusNote.ts index 89f860286..8755e18fd 100644 --- a/packages/app-desktop/gui/NoteList/utils/useFocusNote.ts +++ b/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>; +type ContainerRef = RefObject; 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 (!itemRefs.current[noteId]) { - const targetIndex = notesRef.current.findIndex(note => note.id === noteId); - if (targetIndex > -1) { - makeItemIndexVisible(targetIndex); - } - - 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]); + if (noteId) { + setActiveNoteId(noteId); } - }, [itemRefs, makeItemIndexVisible]); + + // 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); + + const targetIndex = notesRef.current.findIndex(note => note.id === noteId); + if (targetIndex > -1) { + makeItemIndexVisible(targetIndex); + } + }, [containerRef, makeItemIndexVisible, setActiveNoteId]); return focusNote; }; diff --git a/packages/app-desktop/gui/NoteList/utils/useFocusVisible.ts b/packages/app-desktop/gui/NoteList/utils/useFocusVisible.ts new file mode 100644 index 000000000..62b20dbb8 --- /dev/null +++ b/packages/app-desktop/gui/NoteList/utils/useFocusVisible.ts @@ -0,0 +1,45 @@ +import { useCallback, useState, useRef, RefObject } from 'react'; + +const useFocusVisible = (containerRef: RefObject, 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; diff --git a/packages/app-desktop/gui/NoteList/utils/useOnKeyDown.ts b/packages/app-desktop/gui/NoteList/utils/useOnKeyDown.ts index ec17c1764..41f1bed6d 100644 --- a/packages/app-desktop/gui/NoteList/utils/useOnKeyDown.ts +++ b/packages/app-desktop/gui/NoteList/utils/useOnKeyDown.ts @@ -8,8 +8,11 @@ import { Dispatch } from 'redux'; import { FocusNote } from './useFocusNote'; import { ItemFlow } from '@joplin/lib/services/plugins/api/noteListType'; import { KeyboardEventKey } from '@joplin/lib/dom'; +import announceForAccessibility from '../../utils/announceForAccessibility'; +import { _ } from '@joplin/lib/locale'; const useOnKeyDown = ( + activeNoteId: string, selectedNoteIds: string[], moveNote: (direction: number, inc: number)=> void, makeItemIndexVisible: (itemIndex: number)=> void, @@ -84,18 +87,32 @@ const useOnKeyDown = ( } } event.preventDefault(); - } else if (noteIds.length > 0 && (key === 'ArrowDown' || key === 'ArrowUp' || key === 'ArrowLeft' || key === 'ArrowRight' || key === 'PageDown' || key === 'PageUp' || key === 'End' || key === 'Home')) { - const noteId = noteIds[0]; + } else if (notes.length > 0 && (key === 'ArrowDown' || key === 'ArrowUp' || key === 'ArrowLeft' || key === 'ArrowRight' || key === 'PageDown' || key === 'PageUp' || key === 'End' || key === 'Home')) { + const noteId = activeNoteId ?? notes[0]?.id; let noteIndex = BaseModel.modelIndexById(notes, noteId); noteIndex = scrollNoteIndex(visibleItemCount, key, event.ctrlKey, event.metaKey, noteIndex); const newSelectedNote = notes[noteIndex]; - dispatch({ - type: 'NOTE_SELECT', - id: newSelectedNote.id, - }); + if (event.shiftKey) { + if (selectedNoteIds.includes(newSelectedNote.id)) { + dispatch({ + type: 'NOTE_SELECT_REMOVE', + id: noteId, + }); + } + + dispatch({ + type: 'NOTE_SELECT_ADD', + id: newSelectedNote.id, + }); + } else { + dispatch({ + type: 'NOTE_SELECT', + id: newSelectedNote.id, + }); + } makeItemIndexVisible(noteIndex); @@ -122,8 +139,7 @@ const useOnKeyDown = ( event.preventDefault(); const selectedNotes = BaseModel.modelsByIds(notes, noteIds); - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied - const todos = selectedNotes.filter((n: any) => !!n.is_todo); + const todos = selectedNotes.filter(n => !!n.is_todo); if (!todos.length) return; for (let i = 0; i < todos.length; i++) { @@ -131,7 +147,10 @@ const useOnKeyDown = ( await Note.save(toggledTodo); } + dispatch({ type: 'NOTE_SORT' }); focusNote(todos[0].id); + const wasCompleted = !!todos[0].todo_completed; + announceForAccessibility(!wasCompleted ? _('Complete') : _('Incomplete')); } if (key === 'Tab') { @@ -151,7 +170,7 @@ const useOnKeyDown = ( type: 'NOTE_SELECT_ALL', }); } - }, [moveNote, focusNote, visibleItemCount, scrollNoteIndex, makeItemIndexVisible, notes, selectedNoteIds, dispatch, flow, itemsPerLine]); + }, [moveNote, focusNote, visibleItemCount, scrollNoteIndex, makeItemIndexVisible, notes, selectedNoteIds, activeNoteId, dispatch, flow, itemsPerLine]); return onKeyDown; diff --git a/packages/app-desktop/gui/NoteListItem/NoteListItem.tsx b/packages/app-desktop/gui/NoteListItem/NoteListItem.tsx index 30d71109c..887150771 100644 --- a/packages/app-desktop/gui/NoteListItem/NoteListItem.tsx +++ b/packages/app-desktop/gui/NoteListItem/NoteListItem.tsx @@ -10,6 +10,7 @@ import Note from '@joplin/lib/models/Note'; import { NoteEntity } from '@joplin/lib/services/database/types'; import useRenderedNote from './utils/useRenderedNote'; import { Dispatch } from 'redux'; +import getNoteElementIdFromJoplinId from './utils/getNoteElementIdFromJoplinId'; interface NoteItemProps { dragIndex: number; @@ -26,8 +27,12 @@ interface NoteItemProps { onDragStart: DragEventHandler; style: CSSProperties; note: NoteEntity; - isSelected: boolean; isWatched: boolean; + + isSelected: boolean; + tabIndex: number; + focusVisible: boolean; + listRenderer: ListRenderer; columns: NoteListColumns; dispatch: Dispatch; @@ -35,7 +40,7 @@ interface NoteItemProps { const NoteListItem = (props: NoteItemProps, ref: LegacyRef) => { const noteId = props.note.id; - const elementId = `list-note-${noteId}`; + const elementId = getNoteElementIdFromJoplinId(noteId); const onInputChange: OnInputChange = useCallback(async (event: ChangeEvent) => { const getValue = (element: HTMLInputElement) => { @@ -70,6 +75,7 @@ const NoteListItem = (props: NoteItemProps, ref: LegacyRef) => { rootElement, noteId, renderedNote ? renderedNote.html : '', + props.focusVisible, props.style, props.itemSize, props.onClick, @@ -147,13 +153,18 @@ const NoteListItem = (props: NoteItemProps, ref: LegacyRef) => { id={elementId} ref={ref} draggable={true} - tabIndex={0} + tabIndex={props.tabIndex} className={className} data-id={noteId} style={{ height: props.itemSize.height }} onContextMenu={props.onContextMenu} onDragStart={props.onDragStart} onDragOver={props.onDragOver} + + aria-selected={props.isSelected} + aria-posinset={1 + props.index} + aria-setsize={props.noteCount} + role='option' >
; diff --git a/packages/app-desktop/gui/NoteListItem/utils/getNoteElementIdFromJoplinId.ts b/packages/app-desktop/gui/NoteListItem/utils/getNoteElementIdFromJoplinId.ts new file mode 100644 index 000000000..732afec47 --- /dev/null +++ b/packages/app-desktop/gui/NoteListItem/utils/getNoteElementIdFromJoplinId.ts @@ -0,0 +1,4 @@ + +const getNoteElementIdFromJoplinId = (id: string) => `list-note-${id}`; + +export default getNoteElementIdFromJoplinId; diff --git a/packages/app-desktop/gui/NoteListItem/utils/prepareViewProps.ts b/packages/app-desktop/gui/NoteListItem/utils/prepareViewProps.ts index 1f3451c33..9e7cc91b3 100644 --- a/packages/app-desktop/gui/NoteListItem/utils/prepareViewProps.ts +++ b/packages/app-desktop/gui/NoteListItem/utils/prepareViewProps.ts @@ -2,6 +2,7 @@ import { ListRendererDependency } from '@joplin/lib/services/plugins/api/noteLis import { FolderEntity, NoteEntity, TagEntity } from '@joplin/lib/services/database/types'; import { Size } from '@joplin/utils/types'; import Note from '@joplin/lib/models/Note'; +import { _ } from '@joplin/lib/locale'; const prepareViewProps = async ( dependencies: ListRendererDependency[], @@ -33,6 +34,12 @@ const prepareViewProps = async ( } else if (dep === 'note.folder.title') { if (!output.note.folder) output.note.folder = {}; output.note.folder[propName] = folder.title; + } else if (dep === 'note.todoStatusText') { + let taskStatus = ''; + if (note.is_todo) { + taskStatus = note.todo_completed ? _('Complete to-do') : _('Incomplete to-do'); + } + output.note[propName] = taskStatus; } else { // The notes in the state only contain the properties defined in // Note.previewFields(). It means that if a view request a diff --git a/packages/app-desktop/gui/NoteListItem/utils/useItemElement.ts b/packages/app-desktop/gui/NoteListItem/utils/useItemElement.ts index 16a3b507e..109671ca8 100644 --- a/packages/app-desktop/gui/NoteListItem/utils/useItemElement.ts +++ b/packages/app-desktop/gui/NoteListItem/utils/useItemElement.ts @@ -3,8 +3,9 @@ import { Size } from '@joplin/utils/types'; import { useEffect, useState } from 'react'; import { ItemFlow } from '@joplin/lib/services/plugins/api/noteListType'; -// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied -const useItemElement = (rootElement: HTMLDivElement, noteId: string, noteHtml: string, style: any, itemSize: Size, onClick: React.MouseEventHandler, flow: ItemFlow) => { +const useItemElement = ( + rootElement: HTMLDivElement, noteId: string, noteHtml: string, focusVisible: boolean, style: React.CSSProperties, itemSize: Size, onClick: React.MouseEventHandler, flow: ItemFlow, +) => { const [itemElement, setItemElement] = useState(null); useEffect(() => { @@ -32,6 +33,16 @@ const useItemElement = (rootElement: HTMLDivElement, noteId: string, noteHtml: s }; }, [rootElement, itemSize, noteHtml, noteId, style, onClick, flow]); + useEffect(() => { + if (!itemElement) return; + + if (focusVisible) { + itemElement.classList.add('-focus-visible'); + } else { + itemElement.classList.remove('-focus-visible'); + } + }, [focusVisible, itemElement]); + return itemElement; }; diff --git a/packages/app-desktop/gui/utils/announceForAccessibility.ts b/packages/app-desktop/gui/utils/announceForAccessibility.ts new file mode 100644 index 000000000..91d5f7749 --- /dev/null +++ b/packages/app-desktop/gui/utils/announceForAccessibility.ts @@ -0,0 +1,33 @@ +import shim from '@joplin/lib/shim'; +import Logger from '@joplin/utils/Logger'; + +const logger = Logger.create('announceForAccessibility'); + +const announceForAccessibility = (message: string) => { + const id = 'joplin-announce-for-accessibility-live-region'; + let announcementArea = document.querySelector(`#${id}`); + + if (!announcementArea) { + announcementArea = document.createElement('div'); + announcementArea.ariaLive = 'polite'; + announcementArea.role = 'status'; + announcementArea.id = id; + announcementArea.classList.add('visually-hidden'); + + document.body.appendChild(announcementArea); + } else { + // Allows messages to be re-announced. + announcementArea.textContent = ''; + } + + // Defer the announcement. Because `aria-live: polite` causes **changes** in + // content to be announced, a slight delay is needed when there would otherwise + // be no change in content. + const announce = () => { + logger.debug('Announcing:', message); + announcementArea.textContent = message; + }; + shim.setTimeout(announce, 100); +}; + +export default announceForAccessibility; diff --git a/packages/app-desktop/integration-tests/main.spec.ts b/packages/app-desktop/integration-tests/main.spec.ts index 77fd82c94..f6e4e6728 100644 --- a/packages/app-desktop/integration-tests/main.spec.ts +++ b/packages/app-desktop/integration-tests/main.spec.ts @@ -24,7 +24,7 @@ test.describe('main', () => { const editor = await mainScreen.createNewNote('Test note'); // Note list should contain the new note - await expect(mainScreen.noteListContainer.getByText('Test note')).toBeVisible(); + await expect(mainScreen.noteList.getNoteItemByTitle('Test note')).toBeVisible(); // Focus the editor await editor.codeMirrorEditor.click(); diff --git a/packages/app-desktop/integration-tests/markdownEditor.spec.ts b/packages/app-desktop/integration-tests/markdownEditor.spec.ts index 4c27f4d02..ff1dff1a7 100644 --- a/packages/app-desktop/integration-tests/markdownEditor.spec.ts +++ b/packages/app-desktop/integration-tests/markdownEditor.spec.ts @@ -15,7 +15,8 @@ test.describe('markdownEditor', () => { await importedFolder.waitFor(); await importedFolder.click(); - const importedHtmlFileItem = mainScreen.noteListContainer.getByText('test-html-file-with-image'); + await mainScreen.noteList.focusContent(electronApp); + const importedHtmlFileItem = mainScreen.noteList.getNoteItemByTitle('test-html-file-with-image'); await importedHtmlFileItem.click(); const viewerFrame = mainScreen.noteEditor.getNoteViewerIframe(); diff --git a/packages/app-desktop/integration-tests/models/MainScreen.ts b/packages/app-desktop/integration-tests/models/MainScreen.ts index 0a8a4f5bd..8ddd69a26 100644 --- a/packages/app-desktop/integration-tests/models/MainScreen.ts +++ b/packages/app-desktop/integration-tests/models/MainScreen.ts @@ -4,10 +4,11 @@ import activateMainMenuItem from '../util/activateMainMenuItem'; import Sidebar from './Sidebar'; import GoToAnything from './GoToAnything'; import setFilePickerResponse from '../util/setFilePickerResponse'; +import NoteList from './NoteList'; export default class MainScreen { public readonly newNoteButton: Locator; - public readonly noteListContainer: Locator; + public readonly noteList: NoteList; public readonly sidebar: Sidebar; public readonly dialog: Locator; public readonly noteEditor: NoteEditorScreen; @@ -15,7 +16,7 @@ export default class MainScreen { public constructor(private page: Page) { this.newNoteButton = page.locator('.new-note-button'); - this.noteListContainer = page.locator('.rli-noteList'); + this.noteList = new NoteList(page); this.sidebar = new Sidebar(page, this); this.dialog = page.locator('.dialog-modal-layer'); this.noteEditor = new NoteEditorScreen(page); @@ -24,7 +25,7 @@ export default class MainScreen { public async waitFor() { await this.newNoteButton.waitFor(); - await this.noteListContainer.waitFor(); + await this.noteList.waitFor(); } // Follows the steps a user would use to create a new note. diff --git a/packages/app-desktop/integration-tests/models/NoteList.ts b/packages/app-desktop/integration-tests/models/NoteList.ts new file mode 100644 index 000000000..0aac1d65a --- /dev/null +++ b/packages/app-desktop/integration-tests/models/NoteList.ts @@ -0,0 +1,38 @@ +import activateMainMenuItem from '../util/activateMainMenuItem'; +import { ElectronApplication, Locator, Page, expect } from '@playwright/test'; + +export default class NoteList { + public readonly container: Locator; + + public constructor(page: Page) { + this.container = page.locator('.rli-noteList'); + } + + public waitFor() { + return this.container.waitFor(); + } + + private async sortBy(electronApp: ElectronApplication, sortMethod: string) { + const success = await activateMainMenuItem(electronApp, sortMethod, 'Sort notes by'); + if (!success) { + throw new Error(`Unable to find sorting menu item: ${sortMethod}`); + } + } + + public async sortByTitle(electronApp: ElectronApplication) { + return this.sortBy(electronApp, 'Title'); + } + + public async focusContent(electronApp: ElectronApplication) { + await activateMainMenuItem(electronApp, 'Note list', 'Focus'); + } + + // The resultant locator may fail to resolve if the item is not visible + public getNoteItemByTitle(title: string|RegExp) { + return this.container.getByRole('option', { name: title }); + } + + public async expectNoteToBeSelected(title: string|RegExp) { + await expect(this.getNoteItemByTitle(title)).toHaveAttribute('aria-selected', 'true'); + } +} diff --git a/packages/app-desktop/integration-tests/noteList.spec.ts b/packages/app-desktop/integration-tests/noteList.spec.ts index a54f84c40..9fede3ee1 100644 --- a/packages/app-desktop/integration-tests/noteList.spec.ts +++ b/packages/app-desktop/integration-tests/noteList.spec.ts @@ -1,10 +1,9 @@ import { test, expect } from './util/test'; import MainScreen from './models/MainScreen'; -import activateMainMenuItem from './util/activateMainMenuItem'; import setMessageBoxResponse from './util/setMessageBoxResponse'; test.describe('noteList', () => { - test('should be possible to edit notes in a different notebook when searching', async ({ mainWindow }) => { + test('should be possible to edit notes in a different notebook when searching', async ({ mainWindow, electronApp }) => { const mainScreen = new MainScreen(mainWindow); const sidebar = mainScreen.sidebar; @@ -22,7 +21,8 @@ test.describe('noteList', () => { // Search for and focus a note different from the folder we were in before searching. await mainScreen.search('/note-1'); - const note1Result = mainScreen.noteListContainer.getByText('note-1'); + await mainScreen.noteList.focusContent(electronApp); + const note1Result = mainScreen.noteList.getNoteItemByTitle('note-1'); await expect(note1Result).toBeAttached(); await note1Result.click(); @@ -49,9 +49,10 @@ test.describe('noteList', () => { await mainScreen.createNewNote('test note 1'); await mainScreen.createNewNote('test note 2'); - await activateMainMenuItem(electronApp, 'Note list', 'Focus'); - await expect(mainScreen.noteListContainer.getByText('test note 1')).toBeVisible(); - await expect(mainScreen.noteListContainer.getByText('test note 2')).toBeVisible(); + const noteList = mainScreen.noteList; + await noteList.focusContent(electronApp); + await expect(noteList.getNoteItemByTitle('test note 1')).toBeVisible(); + await expect(noteList.getNoteItemByTitle('test note 2')).toBeVisible(); await setMessageBoxResponse(electronApp, /^Delete/i); @@ -62,7 +63,7 @@ test.describe('noteList', () => { await mainWindow.keyboard.up('Shift'); }; await pressShiftDelete(); - await expect(mainScreen.noteListContainer.getByText('test note 2')).not.toBeVisible(); + await expect(noteList.getNoteItemByTitle('test note 2')).not.toBeVisible(); // Should not delete when the editor is focused await mainScreen.noteEditor.focusCodeMirrorEditor(); @@ -71,6 +72,35 @@ test.describe('noteList', () => { await folderBHeader.click(); await folderAHeader.click(); - await expect(mainScreen.noteListContainer.getByText('test note 1')).toBeVisible(); + await expect(noteList.getNoteItemByTitle('test note 1')).toBeVisible(); + }); + + test('arrow keys should navigate the note list', async ({ electronApp, mainWindow }) => { + const mainScreen = new MainScreen(mainWindow); + const sidebar = mainScreen.sidebar; + + await sidebar.createNewFolder('Folder'); + + await mainScreen.createNewNote('note_1'); + await mainScreen.createNewNote('note_2'); + await mainScreen.createNewNote('note_3'); + await mainScreen.createNewNote('note_4'); + + const noteList = mainScreen.noteList; + await noteList.sortByTitle(electronApp); + await noteList.focusContent(electronApp); + // The most recently-created note should be visible + const note4Item = noteList.getNoteItemByTitle('note_4'); + await expect(note4Item).toBeVisible(); + await noteList.expectNoteToBeSelected('note_4'); + + await noteList.container.press('ArrowUp'); + await noteList.expectNoteToBeSelected('note_3'); + + await noteList.container.press('ArrowUp'); + await noteList.expectNoteToBeSelected('note_2'); + + await noteList.container.press('ArrowDown'); + await noteList.expectNoteToBeSelected('note_3'); }); }); diff --git a/packages/app-desktop/integration-tests/settings.spec.ts b/packages/app-desktop/integration-tests/settings.spec.ts index adba65323..db26b46df 100644 --- a/packages/app-desktop/integration-tests/settings.spec.ts +++ b/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.getByRole('button', { name: 'Toggle sort order' }); + const sortOrderLocator = mainScreen.noteList.container.getByRole('button', { name: 'Toggle sort order' }); await expect(sortOrderLocator).toBeVisible(); await mainScreen.openSettings(electronApp); diff --git a/packages/app-desktop/main.scss b/packages/app-desktop/main.scss index d99d8188a..081330db6 100644 --- a/packages/app-desktop/main.scss +++ b/packages/app-desktop/main.scss @@ -249,6 +249,22 @@ p.info-text { color: var(--joplin-color-faded); } +// Hides elements, but still exposes them to accessibility tools. +// Avoids `visibility: hidden` and `display: none`, because this may cause some +// screen readers to ignore the element. +// See https://www.w3.org/WAI/tutorials/forms/labels/#hiding-label-text +.visually-hidden { + opacity: 0; + z-index: -1; + width: 1px; + height: 1px; + margin: -1px; + padding: 0; + overflow: hidden; + pointer-events: none; + position: absolute; +} + /* ========================================================================================= Component-specific classes ========================================================================================= */ diff --git a/packages/lib/BaseModel.ts b/packages/lib/BaseModel.ts index 92fe1fd61..8ef48ed05 100644 --- a/packages/lib/BaseModel.ts +++ b/packages/lib/BaseModel.ts @@ -5,7 +5,7 @@ import time from './time'; import JoplinDatabase, { TableField } from './JoplinDatabase'; import { LoadOptions, SaveOptions } from './models/utils/types'; import ActionLogger, { ItemActionType as ItemActionType } from './utils/ActionLogger'; -import { SqlQuery } from './services/database/types'; +import { BaseItemEntity, SqlQuery } from './services/database/types'; const Mutex = require('async-mutex').Mutex; // New code should make use of this enum @@ -167,8 +167,7 @@ class BaseModel { return -1; } - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied - public static modelsByIds(items: any[], ids: string[]) { + public static modelsByIds(items: T[], ids: string[]): T[] { const output = []; for (let i = 0; i < items.length; i++) { if (ids.indexOf(items[i].id) >= 0) { diff --git a/packages/lib/services/noteList/defaultListRenderer.ts b/packages/lib/services/noteList/defaultListRenderer.ts index e77244a36..d2acbfe05 100644 --- a/packages/lib/services/noteList/defaultListRenderer.ts +++ b/packages/lib/services/noteList/defaultListRenderer.ts @@ -40,6 +40,7 @@ const renderer: ListRenderer = { 'note.isWatched', 'note.title', 'note.todo_completed', + 'note.todoStatusText', ], itemCss: // css @@ -57,7 +58,7 @@ const renderer: ListRenderer = { background-color: var(--joplin-selected-color); } - &:hover, :focus-visible > & > .content { + &:hover, &.-focus-visible > .content { background-color: var(--joplin-background-color-hover3); } @@ -133,7 +134,13 @@ const renderer: ListRenderer = {
{{#note.is_todo}}
- +
{{/note.is_todo}}
diff --git a/packages/lib/services/noteList/defaultMultiColumnsRenderer.ts b/packages/lib/services/noteList/defaultMultiColumnsRenderer.ts index a26f44670..083200ea7 100644 --- a/packages/lib/services/noteList/defaultMultiColumnsRenderer.ts +++ b/packages/lib/services/noteList/defaultMultiColumnsRenderer.ts @@ -33,10 +33,6 @@ const renderer: ListRenderer = { display: flex; height: 100%; - &:hover, :focus-visible { - background-color: var(--joplin-background-color-hover3); - } - > .item { display: flex; align-items: center; @@ -83,6 +79,10 @@ const renderer: ListRenderer = { > .row.-completed { opacity: 0.5; } + + > .row:hover, &.-focus-visible > .row { + background-color: var(--joplin-background-color-hover3); + } `, onHeaderClick: async (event: OnClickEvent) => { @@ -108,7 +108,12 @@ const renderer: ListRenderer = { ` {{#note.is_todo}}
- +
{{/note.is_todo}} `, diff --git a/packages/lib/services/plugins/api/noteListType.ts b/packages/lib/services/plugins/api/noteListType.ts index a2657020e..ad00453ac 100644 --- a/packages/lib/services/plugins/api/noteListType.ts +++ b/packages/lib/services/plugins/api/noteListType.ts @@ -35,6 +35,10 @@ export type OnClickHandler = (event: OnClickEvent)=> Promise; * complemented with special properties such as `note.isWatched`, to know if a note is currently * opened in the external editor, and `note.tags` to get the list tags associated with the note. * + * The `note.todoStatusText` property is a localised description of the to-do status (e.g. + * "to-do, incomplete"). If you include an `` for to-do items that would + * otherwise be unlabelled, consider adding `note.todoStatusText` as the checkbox's `aria-label`. + * * ## Item properties * * The `item.*` properties are specific to the rendered item. The most important being @@ -49,6 +53,7 @@ export type ListRendererDependency = 'note.folder.title' | 'note.isWatched' | 'note.tags' | + 'note.todoStatusText' | 'note.titleHtml'; export type ListRendererItemValueTemplates = Record; diff --git a/packages/tools/cspell/dictionary4.txt b/packages/tools/cspell/dictionary4.txt index 5cd3a4bab..cdd49c004 100644 --- a/packages/tools/cspell/dictionary4.txt +++ b/packages/tools/cspell/dictionary4.txt @@ -129,4 +129,5 @@ entypo Zocial agplv Famegear -rcompare \ No newline at end of file +rcompare +tabindex \ No newline at end of file