Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Desktop: Accessibility: Improve note title focus handling #10932

2 changes: 2 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/shouldPasteResources.
packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/shouldPasteResources.js
packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/types.js
packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/useContextMenu.js
packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/useKeyboardRefocusHandler.js
packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/useLinkTooltips.js
packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/useScroll.js
packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/useTabIndenter.js
Expand Down Expand Up @@ -844,6 +845,7 @@ packages/editor/CodeMirror/utils/formatting/types.js
packages/editor/CodeMirror/utils/getSearchState.js
packages/editor/CodeMirror/utils/growSelectionToNode.js
packages/editor/CodeMirror/utils/handlePasteEvent.js
packages/editor/CodeMirror/utils/isCursorAtBeginning.js
packages/editor/CodeMirror/utils/isInSyntaxNode.js
packages/editor/CodeMirror/utils/searchExtension.js
packages/editor/CodeMirror/utils/setupVim.js
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/shouldPasteResources.
packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/shouldPasteResources.js
packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/types.js
packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/useContextMenu.js
packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/useKeyboardRefocusHandler.js
packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/useLinkTooltips.js
packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/useScroll.js
packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/useTabIndenter.js
Expand Down Expand Up @@ -821,6 +822,7 @@ packages/editor/CodeMirror/utils/formatting/types.js
packages/editor/CodeMirror/utils/getSearchState.js
packages/editor/CodeMirror/utils/growSelectionToNode.js
packages/editor/CodeMirror/utils/handlePasteEvent.js
packages/editor/CodeMirror/utils/isCursorAtBeginning.js
packages/editor/CodeMirror/utils/isInSyntaxNode.js
packages/editor/CodeMirror/utils/searchExtension.js
packages/editor/CodeMirror/utils/setupVim.js
Expand Down
10 changes: 7 additions & 3 deletions packages/app-desktop/commands/focusElement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,18 @@ export const declaration: CommandDeclaration = {
name: 'focusElement',
};

export interface FocusElementOptions {
moveCursorToStart: boolean;
}

export const runtime = (): CommandRuntime => {
return {
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
execute: async (_context: any, target: string) => {
if (target === 'noteBody') return CommandService.instance().execute('focusElementNoteBody');
execute: async (_context: any, target: string, options?: FocusElementOptions) => {
if (target === 'noteBody') return CommandService.instance().execute('focusElementNoteBody', options);
if (target === 'noteList') return CommandService.instance().execute('focusElementNoteList');
if (target === 'sideBar') return CommandService.instance().execute('focusElementSideBar');
if (target === 'noteTitle') return CommandService.instance().execute('focusElementNoteTitle');
if (target === 'noteTitle') return CommandService.instance().execute('focusElementNoteTitle', options);
throw new Error(`Invalid focus target: ${target}`);
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import useContextMenu from '../utils/useContextMenu';
import useWebviewIpcMessage from '../utils/useWebviewIpcMessage';
import Toolbar from '../Toolbar';
import useEditorSearchHandler from '../utils/useEditorSearchHandler';
import CommandService from '@joplin/lib/services/CommandService';

const logger = Logger.create('CodeMirror6');
const logDebug = (message: string) => logger.debug(message);
Expand Down Expand Up @@ -354,6 +355,10 @@ const CodeMirror = (props: NoteBodyEditorProps, ref: ForwardedRef<NoteBodyEditor
}
}, [editor_scroll, codeMirror_change, props.setLocalSearch, props.setShowLocalSearch]);

const onSelectPastBeginning = useCallback(() => {
void CommandService.instance().execute('focusElement', 'noteTitle');
}, []);

const editorSettings = useMemo((): EditorSettings => {
const isHTMLNote = props.contentMarkupLanguage === MarkupToHtml.MARKUP_LANGUAGE_HTML;

Expand Down Expand Up @@ -403,6 +408,7 @@ const CodeMirror = (props: NoteBodyEditorProps, ref: ForwardedRef<NoteBodyEditor
onEvent={onEditorEvent}
onLogMessage={logDebug}
onEditorPaste={onEditorPaste}
onSelectPastBeginning={onSelectPastBeginning}
externalSearch={props.searchMarkers}
useLocalSearch={props.useLocalSearch}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import Logger from '@joplin/utils/Logger';
import CodeMirrorControl from '@joplin/editor/CodeMirror/CodeMirrorControl';
import { MarkupLanguage } from '@joplin/renderer';
import { focus } from '@joplin/lib/utils/focusHandler';
import { FocusElementOptions } from '../../../../../commands/focusElement';

const logger = Logger.create('CodeMirror 6 commands');

Expand Down Expand Up @@ -103,9 +104,15 @@ const useEditorCommands = (props: Props) => {
logger.warn('CodeMirror execCommand: unsupported command: ', value.name);
}
},
'editor.focus': () => {
'editor.focus': (options?: FocusElementOptions) => {
if (props.visiblePanes.indexOf('editor') >= 0) {
focus('useEditorCommands::editor.focus', editorRef.current.editor);
if (options?.moveCursorToStart) {
editorRef.current.editor.dispatch({
selection: { anchor: 0 },
scrollIntoView: true,
});
}
} else {
// If we just call focus() then the iframe is focused,
// but not its content, such that scrolling up / down
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const { clipboard } = require('electron');
const supportedLocales = require('./supportedLocales');
import { hasProtocol } from '@joplin/utils/url';
import useTabIndenter from './utils/useTabIndenter';
import useKeyboardRefocusHandler from './utils/useKeyboardRefocusHandler';

const logger = Logger.create('TinyMCE');

Expand Down Expand Up @@ -130,6 +131,7 @@ const TinyMCE = (props: NoteBodyEditorProps, ref: any) => {
usePluginServiceRegistration(ref);
useContextMenu(editor, props.plugins, props.dispatch, props.htmlToMarkdown, props.markupToHtml);
useTabIndenter(editor);
useKeyboardRefocusHandler(editor);

// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
const dispatchDidUpdate = (editor: any) => {
Expand Down Expand Up @@ -218,6 +220,13 @@ const TinyMCE = (props: NoteBodyEditorProps, ref: any) => {
editor.insertContent(result.html);
} else if (cmd.name === 'editor.focus') {
focus('TinyMCE::editor.focus', editor);
if (cmd.value?.moveCursorToStart) {
editor.selection.placeCaretAt(0, 0);
editor.selection.setCursorLocation(
editor.dom.root,
0,
);
}
} else if (cmd.name === 'editor.execCommand') {
if (!('ui' in cmd.value)) cmd.value.ui = false;
if (!('value' in cmd.value)) cmd.value.value = null;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import CommandService from '@joplin/lib/services/CommandService';
import { useEffect } from 'react';
import type { Editor, EditorEvent } from 'tinymce';

const useKeyboardRefocusHandler = (editor: Editor) => {
useEffect(() => {
if (!editor) return () => {};

const isAtBeginningOf = (element: Node, parent: HTMLElement) => {
if (!parent.contains(element)) return false;

while (
element &&
element.parentNode !== parent &&
element.parentNode?.firstChild === element
) {
element = element.parentNode;
}

return !!element && element.parentNode?.firstChild === element;
};

const eventHandler = (event: EditorEvent<KeyboardEvent>) => {
if (!event.isDefaultPrevented() && event.key === 'ArrowUp') {
const selection = editor.selection.getRng();
if (selection.startOffset === 0 &&
selection.collapsed &&
isAtBeginningOf(selection.startContainer, editor.dom.getRoot())
) {
event.preventDefault();
void CommandService.instance().execute('focusElement', 'noteTitle');
}
}
};

editor.on('keydown', eventHandler);
return () => {
editor.off('keydown', eventHandler);
};
}, [editor]);
};

export default useKeyboardRefocusHandler;
17 changes: 6 additions & 11 deletions packages/app-desktop/gui/NoteEditor/NoteTitle/NoteTitleBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,18 +78,13 @@ function styles_(props: Props) {
export default function NoteTitleBar(props: Props) {
const styles = styles_(props);

// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
const onTitleKeydown = useCallback((event: any) => {
const keyCode = event.keyCode;

if (keyCode === 9) { // TAB
const onTitleKeydown: React.KeyboardEventHandler<HTMLInputElement> = useCallback((event) => {
const titleElement = event.currentTarget;
const selectionAtEnd = titleElement.selectionEnd === titleElement.value.length;
if ((event.key === 'ArrowDown' && selectionAtEnd) || event.key === 'Enter') {
event.preventDefault();

if (event.shiftKey) {
void CommandService.instance().execute('focusElement', 'noteList');
} else {
void CommandService.instance().execute('focusElement', 'noteBody');
}
const moveCursorToStart = event.key === 'ArrowDown';
void CommandService.instance().execute('focusElement', 'noteBody', { moveCursorToStart });
}
}, []);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { CommandRuntime, CommandDeclaration } from '@joplin/lib/services/CommandService';
import { _ } from '@joplin/lib/locale';
import { FocusElementOptions } from '../../../commands/focusElement';

export const declaration: CommandDeclaration = {
name: 'focusElementNoteBody',
Expand All @@ -10,8 +11,8 @@ export const declaration: CommandDeclaration = {
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
export const runtime = (comp: any): CommandRuntime => {
return {
execute: async () => {
comp.editorRef.current.execCommand({ name: 'editor.focus' });
execute: async (_context: unknown, options?: FocusElementOptions) => {
comp.editorRef.current.execCommand({ name: 'editor.focus', value: options });
},
enabledCondition: 'oneNoteSelected',
};
Expand Down
9 changes: 8 additions & 1 deletion packages/app-desktop/integration-tests/main.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,19 @@ test.describe('main', () => {
'',
'Sum: $\\sum_{x=0}^{100} \\tan x$',
];
let firstLine = true;
for (const line of noteText) {
if (line) {
await mainWindow.keyboard.press('Shift+Tab');
if (!firstLine) {
// Remove any auto-indentation, but avoid pressing shift-tab at
// the beginning of the editor.
await mainWindow.keyboard.press('Shift+Tab');
}

await mainWindow.keyboard.type(line);
}
await mainWindow.keyboard.press('Enter');
firstLine = false;
}

// Should render mermaid
Expand Down
24 changes: 24 additions & 0 deletions packages/app-desktop/integration-tests/richTextEditor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,5 +119,29 @@ test.describe('richTextEditor', () => {
await editor.toggleEditorsButton.click();
await expect(editor.codeMirrorEditor).toHaveText('This is a test. Test! Another: !');
});

test('should be possible to navigate between the note title and rich text editor with enter/down/up keys', async ({ mainWindow }) => {
const mainScreen = new MainScreen(mainWindow);
await mainScreen.createNewNote('Testing keyboard navigation!');

const editor = mainScreen.noteEditor;
await editor.toggleEditorsButton.click();

await editor.richTextEditor.waitFor();

await editor.noteTitleInput.click();
await expect(editor.noteTitleInput).toBeFocused();

await mainWindow.keyboard.press('End');
await mainWindow.keyboard.press('ArrowDown');
await expect(editor.richTextEditor).toBeFocused();

await mainWindow.keyboard.press('ArrowUp');
await expect(editor.noteTitleInput).toBeFocused();

await mainWindow.keyboard.press('Enter');
await expect(editor.noteTitleInput).not.toBeFocused();
await expect(editor.richTextEditor).toBeFocused();
});
});

19 changes: 18 additions & 1 deletion packages/editor/CodeMirror/createEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import insertLineAfter from './editorCommands/insertLineAfter';
import handlePasteEvent from './utils/handlePasteEvent';
import biDirectionalTextExtension from './utils/biDirectionalTextExtension';
import searchExtension from './utils/searchExtension';
import isCursorAtBeginning from './utils/isCursorAtBeginning';

const createEditor = (
parentElement: HTMLElement, props: EditorProps,
Expand Down Expand Up @@ -176,12 +177,28 @@ const createEditor = (
return true;
}),
keyCommand('Tab', insertOrIncreaseIndent, true),
keyCommand('Shift-Tab', decreaseIndent, true),
keyCommand('Shift-Tab', (view) => {
// When at the beginning of the editor, allow shift-tab to act
// normally.
if (isCursorAtBeginning(view.state)) {
return false;
}

return decreaseIndent(view);
}, true),
keyCommand('Mod-Enter', (_: EditorView) => {
insertLineAfter(_);
return true;
}, true),

keyCommand('ArrowUp', (view: EditorView) => {
if (isCursorAtBeginning(view.state) && props.onSelectPastBeginning) {
props.onSelectPastBeginning();
return true;
}
return false;
}, true),

...standardKeymap, ...historyKeymap, ...searchKeymap,
]));

Expand Down
8 changes: 8 additions & 0 deletions packages/editor/CodeMirror/utils/isCursorAtBeginning.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { EditorState } from '@codemirror/state';

const isCursorAtBeginning = (state: EditorState) => {
const selection = state.selection;
return selection.ranges.length === 1 && selection.main.empty && selection.main.anchor === 0;
};

export default isCursorAtBeginning;
2 changes: 2 additions & 0 deletions packages/editor/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,13 +169,15 @@ export interface EditorSettings {
export type LogMessageCallback = (message: string)=> void;
export type OnEventCallback = (event: EditorEvent)=> void;
export type PasteFileCallback = (data: File)=> Promise<void>;
type OnScrollPastBeginningCallback = ()=> void;

export interface EditorProps {
settings: EditorSettings;
initialText: string;

// If null, paste and drag-and-drop will not work for resources unless handled elsewhere.
onPasteFile: PasteFileCallback|null;
onSelectPastBeginning?: OnScrollPastBeginningCallback;
onEvent: OnEventCallback;
onLogMessage: LogMessageCallback;
}
Expand Down
Loading