Merge pull request #1564 from klembot/add-vanilla-editor-pref

Add preference to disable CodeMirror

public/locales/en-US.json

@@ -175,7 +175,8 @@
 			"themeDark": "Dark",
 			"themeSystem": "System",
 			"theme": "Theme",
-			"title": "Preferences"
+			"title": "Preferences",
+			"useEnhancedEditors": "Use Enhanced Editors"
 		},
 		"passageEdit": {
 			"editorCrashed": "Something went wrong with this editor. Try closing it and editing this passage again.",

src/components/control/code-area/__mocks__/code-area.tsx

@@ -3,19 +3,14 @@ import {CodeAreaProps} from '../code-area';
 
 export const CodeArea: React.FC<CodeAreaProps> = props => {
 	function handleOnChange(e: React.ChangeEvent<HTMLTextAreaElement>) {
-		props.onBeforeChange(
-			{
-				historySize: () => ({
-					redo: 0,
-					undo: 0
-				}),
-				mockCodeMirrorEditor: true
-			} as any,
-			{
-				mockCodeMirrorChange: true
-			} as any,
-			e.target.value
-		);
+		props.onChangeEditor?.({
+			historySize: () => ({
+				redo: 0,
+				undo: 0
+			}),
+			mockCodeMirrorEditor: true
+		} as any);
+		props.onChangeText(e.target.value);
 	}
 
 	return (
@@ -24,6 +19,7 @@ export const CodeArea: React.FC<CodeAreaProps> = props => {
 			data-font-family={props.fontFamily}
 			data-font-scale={props.fontScale}
 			data-options={JSON.stringify(props.options)}
+			data-use-code-mirror={props.useCodeMirror}
 		>
 			<label>
 				{props.label} <textarea onChange={handleOnChange} value={props.value} />

src/components/control/code-area/__tests__/code-area.test.tsx

@@ -1,4 +1,4 @@
-import {render, screen} from '@testing-library/react';
+import {fireEvent, render, screen} from '@testing-library/react';
 import {axe} from 'jest-axe';
 import * as React from 'react';
 import {CodeArea, CodeAreaProps} from '../code-area';
@@ -9,41 +9,90 @@ describe('<CodeArea>', () => {
 	function renderComponent(props?: Partial<CodeAreaProps>) {
 		return render(
 			<CodeArea
+				id="mock-id"
 				label="mock-label"
-				onBeforeChange={jest.fn()}
+				onChangeEditor={jest.fn()}
+				onChangeText={jest.fn()}
 				value="mock-value"
 				{...props}
 			/>
 		);
 	}
 
-	it('renders the label prop', () => {
-		renderComponent({label: 'test label'});
-		expect(screen.getByLabelText('test label')).toBeInTheDocument();
-	});
+	describe('When CodeMirror is enabled', () => {
+		it('renders the label prop', () => {
+			renderComponent({label: 'test label'});
+			expect(screen.getByLabelText('test label')).toBeInTheDocument();
+		});
 
-	it('renders the value prop', () => {
-		renderComponent({value: 'test value'});
-		expect(screen.getByLabelText('mock-label')).toHaveValue('test value');
-	});
+		it('renders the value prop', () => {
+			renderComponent({value: 'test value'});
+			expect(screen.getByLabelText('mock-label')).toHaveValue('test value');
+		});
 
-	it('sets a style on the container based on the fontFamily and fontScale props', () => {
-		renderComponent({fontFamily: 'my-custom-font', fontScale: 1.5});
+		it('sets a style on the container based on the fontFamily and fontScale props', () => {
+			renderComponent({fontFamily: 'my-custom-font', fontScale: 1.5});
 
-		const inputStyle = window.getComputedStyle(
-			// eslint-disable-next-line testing-library/no-node-access
-			document.querySelector('.code-area')!
-		);
+			const inputStyle = window.getComputedStyle(
+				// eslint-disable-next-line testing-library/no-node-access
+				document.querySelector('.code-area')!
+			);
+
+			expect(inputStyle.getPropertyValue('font-family')).toBe('my-custom-font');
+			expect(inputStyle.getPropertyValue('font-size')).toBe('150%');
+		});
+
+		// Changes are handled by props on react-codemirror2 and not tested here.
+
+		it('is accessible', async () => {
+			const {container} = renderComponent();
 
-		expect(inputStyle.getPropertyValue('font-family')).toBe('my-custom-font');
-		expect(inputStyle.getPropertyValue('font-size')).toBe('150%');
+			expect(await axe(container)).toHaveNoViolations();
+		});
 	});
 
-	// Changes are handled by props on react-codemirror2 and not tested here.
+	describe('When using a browser native textarea', () => {
+		it('renders the label prop', () => {
+			renderComponent({label: 'test label', useCodeMirror: false});
+			expect(screen.getByLabelText('test label')).toBeInTheDocument();
+		});
+
+		it('renders the value prop', () => {
+			renderComponent({value: 'test value', useCodeMirror: false});
+			expect(screen.getByLabelText('mock-label')).toHaveValue('test value');
+		});
+
+		it('sets a style on the container based on the fontFamily and fontScale props', () => {
+			renderComponent({
+				fontFamily: 'my-custom-font',
+				fontScale: 1.5,
+				useCodeMirror: false
+			});
+
+			const inputStyle = window.getComputedStyle(
+				// eslint-disable-next-line testing-library/no-node-access
+				document.querySelector('.code-area')!
+			);
+
+			expect(inputStyle.getPropertyValue('font-family')).toBe('my-custom-font');
+			expect(inputStyle.getPropertyValue('font-size')).toBe('150%');
+		});
+
+		it('calls the onChangeText prop when the textarea is changed', () => {
+			const onChangeText = jest.fn();
+
+			renderComponent({onChangeText, useCodeMirror: false});
+			expect(onChangeText).not.toHaveBeenCalled();
+			fireEvent.change(screen.getByLabelText('mock-label'), {
+				target: {value: 'change'}
+			});
+			expect(onChangeText.mock.calls).toEqual([['change']]);
+		});
 
-	it('is accessible', async () => {
-		const {container} = renderComponent();
+		it('is accessible', async () => {
+			const {container} = renderComponent({useCodeMirror: false});
 
-		expect(await axe(container)).toHaveNoViolations();
+			expect(await axe(container)).toHaveNoViolations();
+		});
 	});
 });

src/components/control/code-area/code-area.css

@@ -9,12 +9,6 @@
 }
 
 .code-area > label {
-	display: flex;
-	flex: 1;
-	flex-direction: column;
-}
-
-.code-area > label > .label {
 	display: block;
 	margin-bottom: var(--control-inner-padding);
 }
@@ -24,7 +18,7 @@
 	position: relative;
 }
 
-.code-area .CodeMirror {
+.code-area .CodeMirror, .code-area textarea.visible {
 	background: var(--white-translucent);
 	border-radius: var(--corner-round);
 	border: 1px solid var(--faint-gray);
@@ -38,6 +32,13 @@
 	width: 100%;
 }
 
+.code-area textarea.visible {
+	padding: var(--grid-size);
+	/* Undo absolute positiong above. */
+	position: static;
+	resize: none;
+}
+
 /* See https://codemirror.net/doc/manual.html#styling */
 
 .code-area .CodeMirror-placeholder.CodeMirror-line-like {

src/components/control/code-area/code-area.tsx

@@ -20,16 +20,32 @@ import {initPrefixTriggerGlobally} from '../../../codemirror/prefix-trigger';
 
 initPrefixTriggerGlobally();
 
-export interface CodeAreaProps extends IControlledCodeMirror {
+export interface CodeAreaProps extends Omit<IControlledCodeMirror, 'onBeforeChange'> {
 	fontFamily?: string;
 	fontScale?: number;
+	// ID is required because nesting the input inside the label causes screen
+	// readers to announce the label on every input, which is very annoying.
+	id: string;
 	label: string;
 	labelHidden?: boolean;
+	onChangeEditor?: (value: CodeMirror.Editor) => void;
+	onChangeText: (value: string, data?: CodeMirror.EditorChange) => void;
+	useCodeMirror?: boolean;
 	value: string;
 }
 
 export const CodeArea: React.FC<CodeAreaProps> = props => {
-	const {fontFamily, fontScale, label, ...otherProps} = props;
+	const {
+		fontFamily,
+		fontScale,
+		id,
+		label,
+		onChangeEditor,
+		onChangeText,
+		useCodeMirror,
+		...otherProps
+	} = props;
+	const containerRef = React.useRef<HTMLDivElement>(null);
 	const style: React.CSSProperties = {};
 
 	if (fontFamily) {
@@ -42,18 +58,54 @@ export const CodeArea: React.FC<CodeAreaProps> = props => {
 		style.fontSize = `${fontScale * 100}%`;
 	}
 
+	function handleCodeMirrorBeforeChange(
+		editor: CodeMirror.Editor,
+		data: CodeMirror.EditorChange,
+		text: string
+	) {
+		onChangeEditor?.(editor);
+		onChangeText(text, data);
+	}
+
+	// We need to set the ID of the underlying <textarea> ourselves if we're using
+	// CodeMirror.
+
+	React.useEffect(() => {
+		if (useCodeMirror && containerRef.current) {
+			const textarea = containerRef.current.querySelector('textarea');
+
+			if (textarea) {
+				textarea.setAttribute('id', id);
+			}
+		}
+	}, [id, useCodeMirror]);
+
 	return (
-		<div className="code-area" style={style}>
-			<label>
-				<span
-					className={classnames('label', {
-						'screen-reader-only': props.labelHidden
-					})}
-				>
-					{label}
-				</span>
-				<CodeMirror {...otherProps} />
+		<div className="code-area" ref={containerRef} style={style}>
+			<label
+				htmlFor={id}
+				className={classnames('label', {
+					'screen-reader-only': props.labelHidden
+				})}
+			>
+				{label}
 			</label>
+			{useCodeMirror ? (
+				<CodeMirror
+					{...otherProps}
+					onBeforeChange={handleCodeMirrorBeforeChange}
+				/>
+			) : (
+				<textarea
+					className="visible"
+					id={id}
+					onChange={({target}) => onChangeText(target.value)}
+					placeholder={otherProps.options?.placeholder}
+					style={style}
+				>
+					{otherProps.value}
+				</textarea>
+			)}
 		</div>
 	);
 };

src/dialogs/__tests__/app-prefs.test.tsx

@@ -27,6 +27,7 @@ describe('<AppPrefsDialog>', () => {
 				<PrefInspector name="locale" />
 				<PrefInspector name="passageEditorFontFamily" />
 				<PrefInspector name="passageEditorFontScale" />
+				<PrefInspector name="useCodeMirror" />
 			</FakeStateProvider>
 		);
 	}
@@ -198,6 +199,62 @@ describe('<AppPrefsDialog>', () => {
 		).toHaveTextContent('1.25');
 	});
 
+	it('displays the CodeMirror preference', () => {
+		renderComponent({useCodeMirror: true});
+
+		expect(
+			screen.getByRole('checkbox', {
+				name: 'dialogs.appPrefs.useEnhancedEditors'
+			})
+		).toBeChecked();
+		cleanup();
+		renderComponent({useCodeMirror: false});
+		expect(
+			screen.getByRole('checkbox', {
+				name: 'dialogs.appPrefs.useEnhancedEditors'
+			})
+		).not.toBeChecked();
+	});
+
+	it('changes the CodeMirror preference and not the cursor preference when enabled', () => {
+		renderComponent({editorCursorBlinks: false, useCodeMirror: false});
+		expect(
+			screen.getByTestId('pref-inspector-useCodeMirror')
+		).toHaveTextContent('false');
+		fireEvent.click(
+			screen.getByRole('checkbox', {
+				name: 'dialogs.appPrefs.useEnhancedEditors'
+			})
+		);
+		expect(
+			screen.getByTestId('pref-inspector-editorCursorBlinks')
+		).toHaveTextContent('false');
+		expect(
+			screen.getByTestId('pref-inspector-useCodeMirror')
+		).toHaveTextContent('true');
+	});
+
+	it('changes the CodeMirror preference and the cursor preference when disabled', () => {
+		renderComponent({editorCursorBlinks: false, useCodeMirror: true});
+		expect(
+			screen.getByTestId('pref-inspector-editorCursorBlinks')
+		).toHaveTextContent('false');
+		expect(
+			screen.getByTestId('pref-inspector-useCodeMirror')
+		).toHaveTextContent('true');
+		fireEvent.click(
+			screen.getByRole('checkbox', {
+				name: 'dialogs.appPrefs.useEnhancedEditors'
+			})
+		);
+		expect(
+			screen.getByTestId('pref-inspector-editorCursorBlinks')
+		).toHaveTextContent('true');
+		expect(
+			screen.getByTestId('pref-inspector-useCodeMirror')
+		).toHaveTextContent('false');
+	});
+
 	it('is accessible', async () => {
 		const {container} = renderComponent();