Skip to content

Commit 96e4c4e

Browse files
committed
Add preference to disable CodeMirror
1 parent 8fc57b0 commit 96e4c4e

22 files changed

+492
-125
lines changed

public/locales/en-US.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,8 @@
175175
"themeDark": "Dark",
176176
"themeSystem": "System",
177177
"theme": "Theme",
178-
"title": "Preferences"
178+
"title": "Preferences",
179+
"useEnhancedEditors": "Use Enhanced Editors"
179180
},
180181
"passageEdit": {
181182
"editorCrashed": "Something went wrong with this editor. Try closing it and editing this passage again.",

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

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,14 @@ import {CodeAreaProps} from '../code-area';
33

44
export const CodeArea: React.FC<CodeAreaProps> = props => {
55
function handleOnChange(e: React.ChangeEvent<HTMLTextAreaElement>) {
6-
props.onBeforeChange(
7-
{
8-
historySize: () => ({
9-
redo: 0,
10-
undo: 0
11-
}),
12-
mockCodeMirrorEditor: true
13-
} as any,
14-
{
15-
mockCodeMirrorChange: true
16-
} as any,
17-
e.target.value
18-
);
6+
props.onChangeEditor?.({
7+
historySize: () => ({
8+
redo: 0,
9+
undo: 0
10+
}),
11+
mockCodeMirrorEditor: true
12+
} as any);
13+
props.onChangeText(e.target.value);
1914
}
2015

2116
return (
@@ -24,6 +19,7 @@ export const CodeArea: React.FC<CodeAreaProps> = props => {
2419
data-font-family={props.fontFamily}
2520
data-font-scale={props.fontScale}
2621
data-options={JSON.stringify(props.options)}
22+
data-use-code-mirror={props.useCodeMirror}
2723
>
2824
<label>
2925
{props.label} <textarea onChange={handleOnChange} value={props.value} />
Lines changed: 71 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {render, screen} from '@testing-library/react';
1+
import {fireEvent, render, screen} from '@testing-library/react';
22
import {axe} from 'jest-axe';
33
import * as React from 'react';
44
import {CodeArea, CodeAreaProps} from '../code-area';
@@ -9,41 +9,90 @@ describe('<CodeArea>', () => {
99
function renderComponent(props?: Partial<CodeAreaProps>) {
1010
return render(
1111
<CodeArea
12+
id="mock-id"
1213
label="mock-label"
13-
onBeforeChange={jest.fn()}
14+
onChangeEditor={jest.fn()}
15+
onChangeText={jest.fn()}
1416
value="mock-value"
1517
{...props}
1618
/>
1719
);
1820
}
1921

20-
it('renders the label prop', () => {
21-
renderComponent({label: 'test label'});
22-
expect(screen.getByLabelText('test label')).toBeInTheDocument();
23-
});
22+
describe('When CodeMirror is enabled', () => {
23+
it('renders the label prop', () => {
24+
renderComponent({label: 'test label'});
25+
expect(screen.getByLabelText('test label')).toBeInTheDocument();
26+
});
2427

25-
it('renders the value prop', () => {
26-
renderComponent({value: 'test value'});
27-
expect(screen.getByLabelText('mock-label')).toHaveValue('test value');
28-
});
28+
it('renders the value prop', () => {
29+
renderComponent({value: 'test value'});
30+
expect(screen.getByLabelText('mock-label')).toHaveValue('test value');
31+
});
2932

30-
it('sets a style on the container based on the fontFamily and fontScale props', () => {
31-
renderComponent({fontFamily: 'my-custom-font', fontScale: 1.5});
33+
it('sets a style on the container based on the fontFamily and fontScale props', () => {
34+
renderComponent({fontFamily: 'my-custom-font', fontScale: 1.5});
3235

33-
const inputStyle = window.getComputedStyle(
34-
// eslint-disable-next-line testing-library/no-node-access
35-
document.querySelector('.code-area')!
36-
);
36+
const inputStyle = window.getComputedStyle(
37+
// eslint-disable-next-line testing-library/no-node-access
38+
document.querySelector('.code-area')!
39+
);
40+
41+
expect(inputStyle.getPropertyValue('font-family')).toBe('my-custom-font');
42+
expect(inputStyle.getPropertyValue('font-size')).toBe('150%');
43+
});
44+
45+
// Changes are handled by props on react-codemirror2 and not tested here.
46+
47+
it('is accessible', async () => {
48+
const {container} = renderComponent();
3749

38-
expect(inputStyle.getPropertyValue('font-family')).toBe('my-custom-font');
39-
expect(inputStyle.getPropertyValue('font-size')).toBe('150%');
50+
expect(await axe(container)).toHaveNoViolations();
51+
});
4052
});
4153

42-
// Changes are handled by props on react-codemirror2 and not tested here.
54+
describe('When using a browser native textarea', () => {
55+
it('renders the label prop', () => {
56+
renderComponent({label: 'test label', useCodeMirror: false});
57+
expect(screen.getByLabelText('test label')).toBeInTheDocument();
58+
});
59+
60+
it('renders the value prop', () => {
61+
renderComponent({value: 'test value', useCodeMirror: false});
62+
expect(screen.getByLabelText('mock-label')).toHaveValue('test value');
63+
});
64+
65+
it('sets a style on the container based on the fontFamily and fontScale props', () => {
66+
renderComponent({
67+
fontFamily: 'my-custom-font',
68+
fontScale: 1.5,
69+
useCodeMirror: false
70+
});
71+
72+
const inputStyle = window.getComputedStyle(
73+
// eslint-disable-next-line testing-library/no-node-access
74+
document.querySelector('.code-area')!
75+
);
76+
77+
expect(inputStyle.getPropertyValue('font-family')).toBe('my-custom-font');
78+
expect(inputStyle.getPropertyValue('font-size')).toBe('150%');
79+
});
80+
81+
it('calls the onChangeText prop when the textarea is changed', () => {
82+
const onChangeText = jest.fn();
83+
84+
renderComponent({onChangeText, useCodeMirror: false});
85+
expect(onChangeText).not.toHaveBeenCalled();
86+
fireEvent.change(screen.getByLabelText('mock-label'), {
87+
target: {value: 'change'}
88+
});
89+
expect(onChangeText.mock.calls).toEqual([['change']]);
90+
});
4391

44-
it('is accessible', async () => {
45-
const {container} = renderComponent();
92+
it('is accessible', async () => {
93+
const {container} = renderComponent({useCodeMirror: false});
4694

47-
expect(await axe(container)).toHaveNoViolations();
95+
expect(await axe(container)).toHaveNoViolations();
96+
});
4897
});
4998
});

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,6 @@
99
}
1010

1111
.code-area > label {
12-
display: flex;
13-
flex: 1;
14-
flex-direction: column;
15-
}
16-
17-
.code-area > label > .label {
1812
display: block;
1913
margin-bottom: var(--control-inner-padding);
2014
}
@@ -24,7 +18,7 @@
2418
position: relative;
2519
}
2620

27-
.code-area .CodeMirror {
21+
.code-area .CodeMirror, .code-area textarea.visible {
2822
background: var(--white-translucent);
2923
border-radius: var(--corner-round);
3024
border: 1px solid var(--faint-gray);
@@ -38,6 +32,13 @@
3832
width: 100%;
3933
}
4034

35+
.code-area textarea.visible {
36+
padding: var(--grid-size);
37+
/* Undo absolute positiong above. */
38+
position: static;
39+
resize: none;
40+
}
41+
4142
/* See https://codemirror.net/doc/manual.html#styling */
4243

4344
.code-area .CodeMirror-placeholder.CodeMirror-line-like {

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

Lines changed: 64 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,32 @@ import {initPrefixTriggerGlobally} from '../../../codemirror/prefix-trigger';
2020

2121
initPrefixTriggerGlobally();
2222

23-
export interface CodeAreaProps extends IControlledCodeMirror {
23+
export interface CodeAreaProps extends Omit<IControlledCodeMirror, 'onBeforeChange'> {
2424
fontFamily?: string;
2525
fontScale?: number;
26+
// ID is required because nesting the input inside the label causes screen
27+
// readers to announce the label on every input, which is very annoying.
28+
id: string;
2629
label: string;
2730
labelHidden?: boolean;
31+
onChangeEditor?: (value: CodeMirror.Editor) => void;
32+
onChangeText: (value: string, data?: CodeMirror.EditorChange) => void;
33+
useCodeMirror?: boolean;
2834
value: string;
2935
}
3036

3137
export const CodeArea: React.FC<CodeAreaProps> = props => {
32-
const {fontFamily, fontScale, label, ...otherProps} = props;
38+
const {
39+
fontFamily,
40+
fontScale,
41+
id,
42+
label,
43+
onChangeEditor,
44+
onChangeText,
45+
useCodeMirror,
46+
...otherProps
47+
} = props;
48+
const containerRef = React.useRef<HTMLDivElement>(null);
3349
const style: React.CSSProperties = {};
3450

3551
if (fontFamily) {
@@ -42,18 +58,54 @@ export const CodeArea: React.FC<CodeAreaProps> = props => {
4258
style.fontSize = `${fontScale * 100}%`;
4359
}
4460

61+
function handleCodeMirrorBeforeChange(
62+
editor: CodeMirror.Editor,
63+
data: CodeMirror.EditorChange,
64+
text: string
65+
) {
66+
onChangeEditor?.(editor);
67+
onChangeText(text, data);
68+
}
69+
70+
// We need to set the ID of the underlying <textarea> ourselves if we're using
71+
// CodeMirror.
72+
73+
React.useEffect(() => {
74+
if (useCodeMirror && containerRef.current) {
75+
const textarea = containerRef.current.querySelector('textarea');
76+
77+
if (textarea) {
78+
textarea.setAttribute('id', id);
79+
}
80+
}
81+
}, [id, useCodeMirror]);
82+
4583
return (
46-
<div className="code-area" style={style}>
47-
<label>
48-
<span
49-
className={classnames('label', {
50-
'screen-reader-only': props.labelHidden
51-
})}
52-
>
53-
{label}
54-
</span>
55-
<CodeMirror {...otherProps} />
84+
<div className="code-area" ref={containerRef} style={style}>
85+
<label
86+
htmlFor={id}
87+
className={classnames('label', {
88+
'screen-reader-only': props.labelHidden
89+
})}
90+
>
91+
{label}
5692
</label>
93+
{useCodeMirror ? (
94+
<CodeMirror
95+
{...otherProps}
96+
onBeforeChange={handleCodeMirrorBeforeChange}
97+
/>
98+
) : (
99+
<textarea
100+
className="visible"
101+
id={id}
102+
onChange={({target}) => onChangeText(target.value)}
103+
placeholder={otherProps.options?.placeholder}
104+
style={style}
105+
>
106+
{otherProps.value}
107+
</textarea>
108+
)}
57109
</div>
58110
);
59111
};

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ describe('<AppPrefsDialog>', () => {
2727
<PrefInspector name="locale" />
2828
<PrefInspector name="passageEditorFontFamily" />
2929
<PrefInspector name="passageEditorFontScale" />
30+
<PrefInspector name="useCodeMirror" />
3031
</FakeStateProvider>
3132
);
3233
}
@@ -198,6 +199,62 @@ describe('<AppPrefsDialog>', () => {
198199
).toHaveTextContent('1.25');
199200
});
200201

202+
it('displays the CodeMirror preference', () => {
203+
renderComponent({useCodeMirror: true});
204+
205+
expect(
206+
screen.getByRole('checkbox', {
207+
name: 'dialogs.appPrefs.useEnhancedEditors'
208+
})
209+
).toBeChecked();
210+
cleanup();
211+
renderComponent({useCodeMirror: false});
212+
expect(
213+
screen.getByRole('checkbox', {
214+
name: 'dialogs.appPrefs.useEnhancedEditors'
215+
})
216+
).not.toBeChecked();
217+
});
218+
219+
it('changes the CodeMirror preference and not the cursor preference when enabled', () => {
220+
renderComponent({editorCursorBlinks: false, useCodeMirror: false});
221+
expect(
222+
screen.getByTestId('pref-inspector-useCodeMirror')
223+
).toHaveTextContent('false');
224+
fireEvent.click(
225+
screen.getByRole('checkbox', {
226+
name: 'dialogs.appPrefs.useEnhancedEditors'
227+
})
228+
);
229+
expect(
230+
screen.getByTestId('pref-inspector-editorCursorBlinks')
231+
).toHaveTextContent('false');
232+
expect(
233+
screen.getByTestId('pref-inspector-useCodeMirror')
234+
).toHaveTextContent('true');
235+
});
236+
237+
it('changes the CodeMirror preference and the cursor preference when disabled', () => {
238+
renderComponent({editorCursorBlinks: false, useCodeMirror: true});
239+
expect(
240+
screen.getByTestId('pref-inspector-editorCursorBlinks')
241+
).toHaveTextContent('false');
242+
expect(
243+
screen.getByTestId('pref-inspector-useCodeMirror')
244+
).toHaveTextContent('true');
245+
fireEvent.click(
246+
screen.getByRole('checkbox', {
247+
name: 'dialogs.appPrefs.useEnhancedEditors'
248+
})
249+
);
250+
expect(
251+
screen.getByTestId('pref-inspector-editorCursorBlinks')
252+
).toHaveTextContent('true');
253+
expect(
254+
screen.getByTestId('pref-inspector-useCodeMirror')
255+
).toHaveTextContent('false');
256+
});
257+
201258
it('is accessible', async () => {
202259
const {container} = renderComponent();
203260

0 commit comments

Comments
 (0)