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,Mobile: Accessibility: Announce formatting changes to the Markdown editor for screen readers #11441

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,6 @@ packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v5/utils/useScrollUtils.
packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/CodeMirror.js
packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/Editor.js
packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/useEditorCommands.js
packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/utils/localisation.js
packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/utils/useKeymap.js
packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/utils/useRefocusOnVisiblePaneChange.js
packages/app-desktop/gui/NoteEditor/NoteBody/PlainEditor/PlainEditor.js
Expand Down Expand Up @@ -895,11 +894,13 @@ packages/editor/CodeMirror/testUtil/createEditorControl.js
packages/editor/CodeMirror/testUtil/createEditorSettings.js
packages/editor/CodeMirror/testUtil/createTestEditor.js
packages/editor/CodeMirror/testUtil/forceFullParse.js
packages/editor/CodeMirror/testUtil/getLastAnnouncement.js
packages/editor/CodeMirror/testUtil/loadLanguages.js
packages/editor/CodeMirror/testUtil/pressReleaseKey.js
packages/editor/CodeMirror/testUtil/typeText.js
packages/editor/CodeMirror/theme.js
packages/editor/CodeMirror/utils/biDirectionalTextExtension.js
packages/editor/CodeMirror/utils/editorLocalizationExtension.js
packages/editor/CodeMirror/utils/formatting/RegionSpec.js
packages/editor/CodeMirror/utils/formatting/findInlineMatch.test.js
packages/editor/CodeMirror/utils/formatting/findInlineMatch.js
Expand All @@ -924,6 +925,9 @@ packages/editor/CodeMirror/utils/searchExtension.js
packages/editor/CodeMirror/utils/setupVim.js
packages/editor/SelectionFormatting.js
packages/editor/events.js
packages/editor/localization.js
packages/editor/localizationPatterns.js
packages/editor/tools/buildLocalizations.js
packages/editor/types.js
packages/fork-htmlparser2/src/CollectingHandler.js
packages/fork-htmlparser2/src/FeedHandler.spec.js
Expand Down
6 changes: 5 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v5/utils/useScrollUtils.
packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/CodeMirror.js
packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/Editor.js
packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/useEditorCommands.js
packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/utils/localisation.js
packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/utils/useKeymap.js
packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/utils/useRefocusOnVisiblePaneChange.js
packages/app-desktop/gui/NoteEditor/NoteBody/PlainEditor/PlainEditor.js
Expand Down Expand Up @@ -871,11 +870,13 @@ packages/editor/CodeMirror/testUtil/createEditorControl.js
packages/editor/CodeMirror/testUtil/createEditorSettings.js
packages/editor/CodeMirror/testUtil/createTestEditor.js
packages/editor/CodeMirror/testUtil/forceFullParse.js
packages/editor/CodeMirror/testUtil/getLastAnnouncement.js
packages/editor/CodeMirror/testUtil/loadLanguages.js
packages/editor/CodeMirror/testUtil/pressReleaseKey.js
packages/editor/CodeMirror/testUtil/typeText.js
packages/editor/CodeMirror/theme.js
packages/editor/CodeMirror/utils/biDirectionalTextExtension.js
packages/editor/CodeMirror/utils/editorLocalizationExtension.js
packages/editor/CodeMirror/utils/formatting/RegionSpec.js
packages/editor/CodeMirror/utils/formatting/findInlineMatch.test.js
packages/editor/CodeMirror/utils/formatting/findInlineMatch.js
Expand All @@ -900,6 +901,9 @@ packages/editor/CodeMirror/utils/searchExtension.js
packages/editor/CodeMirror/utils/setupVim.js
packages/editor/SelectionFormatting.js
packages/editor/events.js
packages/editor/localization.js
packages/editor/localizationPatterns.js
packages/editor/tools/buildLocalizations.js
packages/editor/types.js
packages/fork-htmlparser2/src/CollectingHandler.js
packages/fork-htmlparser2/src/FeedHandler.spec.js
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ import useKeymap from './utils/useKeymap';
import useEditorSearch from '../utils/useEditorSearchExtension';
import CommandService from '@joplin/lib/services/CommandService';
import { SearchMarkers } from '../../../utils/useSearchMarkers';
import localisation from './utils/localisation';
import { makeLocalizations, setLocalizations } from '@joplin/editor/localization';
import { rawStringInCurrentLocale } from '@joplin/lib/locale';

interface Props extends EditorProps {
style: React.CSSProperties;
Expand Down Expand Up @@ -99,11 +100,12 @@ const Editor = (props: Props, ref: ForwardedRef<CodeMirrorControl>) => {

const editorProps: EditorProps = {
...props,
localisations: localisation(),
onEvent: event => onEventRef.current(event),
onLogMessage: message => onLogMessageRef.current(message),
};

setLocalizations(makeLocalizations(rawStringInCurrentLocale));

const editor = createEditor(editorContainerRef.current, editorProps);
editor.addStyles({
'.cm-scroller': { overflow: 'auto' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ const useEditorCommands = (props: Props) => {
const url = await dialogs.prompt(_('Insert Hyperlink'));
focus('useEditorCommands::textLink', editorRef.current);
if (url) {
editorRef.current.wrapSelections('[', `](${url})`);
editorRef.current.wrapSelections('[', `](${url})`, _('Hyperlink'));
}
},
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,17 @@ import CodeMirrorControl from '@joplin/editor/CodeMirror/CodeMirrorControl';
import WebViewToRNMessenger from '../../../utils/ipc/WebViewToRNMessenger';
import { WebViewToEditorApi } from '../types';
import { focus } from '@joplin/lib/utils/focusHandler';
import { Localizations, setLocalizations } from '@joplin/editor/localization';

export const initCodeMirror = (
parentElement: HTMLElement,
initialText: string,
localizations: Localizations,
settings: EditorSettings,
): CodeMirrorControl => {
const messenger = new WebViewToRNMessenger<CodeMirrorControl, WebViewToEditorApi>('editor', null);

setLocalizations(localizations);
const control = createEditor(parentElement, {
initialText,
settings,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
indentWithTabs: false,
};

window.cm = codeMirrorBundle.initCodeMirror(parent, initialText, settings);
window.cm = codeMirrorBundle.initCodeMirror(parent, initialText, {}, settings);
</script>
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ const useHeaderButtons = ({ selectionState, editorControl, readOnly }: ButtonRow
description: _('Header %d', level),
active,

// We only call addHeaderButton 5 times and in the same order, so
// the linter error is safe to ignore.
// eslint-disable-next-line @seiyab/react-hooks/rules-of-hooks
onPress: () => {
editorControl.toggleHeaderLevel(level);
},
Expand Down
6 changes: 4 additions & 2 deletions packages/app-mobile/components/NoteEditor/NoteEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { editorFont } from '../global-style';

import { EditorControl as EditorBodyControl, ContentScriptData } from '@joplin/editor/types';
import { EditorControl, EditorSettings, SelectionRange, WebViewToEditorApi } from './types';
import { _ } from '@joplin/lib/locale';
import { _, rawStringInCurrentLocale } from '@joplin/lib/locale';
import MarkdownToolbar from './MarkdownToolbar/MarkdownToolbar';
import { ChangeEvent, EditorEvent, EditorEventType, SelectionRangeChangeEvent, UndoRedoDepthChangeEvent } from '@joplin/editor/events';
import { EditorCommandType, EditorKeymap, EditorLanguageType, SearchState } from '@joplin/editor/types';
Expand All @@ -30,6 +30,7 @@ import { OnMessageEvent } from '../ExtendedWebView/types';
import { join, dirname } from 'path';
import * as mimeUtils from '@joplin/lib/mime-utils';
import uuid from '@joplin/lib/uuid';
import { makeLocalizations } from '@joplin/editor/localization';

type ChangeEventHandler = (event: ChangeEvent)=> void;
type UndoRedoDepthChangeHandler = (event: UndoRedoDepthChangeEvent)=> void;
Expand Down Expand Up @@ -376,8 +377,9 @@ function NoteEditor(props: Props, ref: any) {
const parentElement = document.getElementsByClassName('CodeMirror')[0];
const initialText = ${JSON.stringify(props.initialText)};
const settings = ${JSON.stringify(editorSettings)};
const localizations = ${JSON.stringify(makeLocalizations(rawStringInCurrentLocale))};

window.cm = codeMirrorBundle.initCodeMirror(parentElement, initialText, settings);
window.cm = codeMirrorBundle.initCodeMirror(parentElement, initialText, localizations, settings);

${setInitialSelectionJS}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ const SearchScreenComponent: React.FC<Props> = props => {
<TextInput
style={styles.searchTextInput}
autoFocus={props.visible}
accessibilityLabel={_('Search')}
underlineColorAndroid="#ffffff00"
onChangeText={setQuery}
value={query}
Expand Down
4 changes: 2 additions & 2 deletions packages/editor/CodeMirror/CodeMirrorControl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ export default class CodeMirrorControl extends CodeMirror5Emulation implements E
this.editor.dispatch(this.editor.state.replaceSelection(text), { userEvent });
}

public wrapSelections(start: string, end: string) {
const regionSpec = RegionSpec.of({ template: { start, end } });
public wrapSelections(start: string, end: string, accessibleName = '') {
const regionSpec = RegionSpec.of({ template: { start, end }, accessibleName });

this.editor.dispatch(
this.editor.state.changeByRange(range => {
Expand Down
3 changes: 2 additions & 1 deletion packages/editor/CodeMirror/createEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import biDirectionalTextExtension from './utils/biDirectionalTextExtension';
import searchExtension from './utils/searchExtension';
import isCursorAtBeginning from './utils/isCursorAtBeginning';
import overwriteModeExtension from './utils/overwriteModeExtension';
import editorLocalizationExtension from './utils/editorLocalizationExtension';

// Newer versions of CodeMirror by default use Chrome's EditContext API.
// While this might be stable enough for desktop use, it causes significant
Expand Down Expand Up @@ -276,7 +277,7 @@ const createEditor = (
biDirectionalTextExtension,
overwriteModeExtension,

props.localisations ? EditorState.phrases.of(props.localisations) : [],
editorLocalizationExtension(),

// Adds additional CSS classes to tokens (the default CSS classes are
// auto-generated and thus unstable).
Expand Down
46 changes: 46 additions & 0 deletions packages/editor/CodeMirror/markdown/markdownCommands.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {
} from './markdownCommands';
import createTestEditor from '../testUtil/createTestEditor';
import { blockMathTagName } from './markdownMathParser';
import getLastAnnouncement from '../testUtil/getLastAnnouncement';
import typeText from '../testUtil/typeText';

describe('markdownCommands', () => {

Expand Down Expand Up @@ -38,6 +40,23 @@ describe('markdownCommands', () => {
expect(editor.state.doc.toString()).toBe('Testing...');
});

it('should announce for accessibility after bolding or removing bold formatting from content', async () => {
const initialDocText = 'Test';
const editor = await createTestEditor(
initialDocText, EditorSelection.range(0, initialDocText.length), [],
);

toggleBolded(editor);

expect(editor.state.doc.toString()).toBe('**Test**');
expect(getLastAnnouncement(editor)).toBe('Added Bold markup');

toggleBolded(editor);

expect(editor.state.doc.toString()).toBe('Test');
expect(getLastAnnouncement(editor)).toBe('Removed Bold markup');
});

it('for a cursor, bolding, then italicizing, should produce a bold-italic region', async () => {
const initialDocText = '';
const editor = await createTestEditor(
Expand Down Expand Up @@ -84,6 +103,19 @@ describe('markdownCommands', () => {
expect(editor.state.doc.toString()).toBe('Testing...\n\n`f(x) = ...` is a function.');
});

it('should announce for accessibility when navigating out of an inline code region', async () => {
const editor = await createTestEditor('', EditorSelection.cursor(0), []);

toggleCode(editor);
expect(getLastAnnouncement(editor)).toBe('Added Inline code markup');
typeText(editor, 'test');
toggleCode(editor);
expect(getLastAnnouncement(editor)).toBe('Moved cursor out of Inline code markup');

expect(editor.state.doc.toString()).toBe('`test`');
expect(editor.state.selection.main.head).toBe(editor.state.doc.length);
});

it('should set headers to the proper levels (when toggling)', async () => {
const initialDocText = 'Testing...\nThis is a test.';
const editor = await createTestEditor(initialDocText, EditorSelection.cursor('Testing...'.length), []);
Expand Down Expand Up @@ -254,6 +286,20 @@ describe('markdownCommands', () => {
expect(editor.state.doc.toString()).toBe('> \tTesting...\n> \tTest.');
});

it('insertOrIncreaseIndent announce the change when text is selected', async () => {
const initialText = 'Test.';
const editor = await createTestEditor(
initialText,
EditorSelection.range(0, initialText.length),
[],
);

insertOrIncreaseIndent(editor);

expect(getLastAnnouncement(editor)).toBe('Added Indent markup to line start');
expect(editor.state.doc.toString()).toBe('\tTest.');
});

it('insertOrIncreaseIndent should insert tabs when selection is empty, in a paragraph', async () => {
const initialText = 'This is a test\nof indentation.';
const editor = await createTestEditor(
Expand Down
Loading