Commit b7ff2487 authored by Robert Knight's avatar Robert Knight

Re-implement `annotationFontFamily` theme property

This property was not implemented when the annotation body components
were converted from Angular to Preact.

 - Add `textStyle` prop to `MarkdownView` and `MarkdownEditor` which
   sets additional CSS properties to apply to the rendered annotation
   body
 - Read the theme style properties for annotation bodies in
   `AnnotationBody` and pass the resulting `textStyle` down to
   `MarkdownEditor` and `MarkdownView`

Fixes #2444
parent f8111265
...@@ -3,6 +3,8 @@ import { useState } from 'preact/hooks'; ...@@ -3,6 +3,8 @@ import { useState } from 'preact/hooks';
import propTypes from 'prop-types'; import propTypes from 'prop-types';
import { isHidden } from '../util/annotation-metadata'; import { isHidden } from '../util/annotation-metadata';
import { withServices } from '../util/service-context';
import { applyTheme } from '../util/theme';
import Button from './button'; import Button from './button';
import Excerpt from './excerpt'; import Excerpt from './excerpt';
...@@ -11,7 +13,10 @@ import MarkdownView from './markdown-view'; ...@@ -11,7 +13,10 @@ import MarkdownView from './markdown-view';
import TagEditor from './tag-editor'; import TagEditor from './tag-editor';
import TagList from './tag-list'; import TagList from './tag-list';
/** @typedef {import("../../types/api").Annotation} Annotation */ /**
* @typedef {import("../../types/api").Annotation} Annotation
* @typedef {import("../../types/config").MergedConfig} MergedConfig
*/
/** /**
* @typedef AnnotationBodyProps * @typedef AnnotationBodyProps
...@@ -23,6 +28,7 @@ import TagList from './tag-list'; ...@@ -23,6 +28,7 @@ import TagList from './tag-list';
* @prop {string} text - * @prop {string} text -
* The markdown annotation body, which is either rendered as HTML (if `isEditing` * The markdown annotation body, which is either rendered as HTML (if `isEditing`
* is false) or displayed in a text area otherwise. * is false) or displayed in a text area otherwise.
* @prop {MergedConfig} [settings]
*/ */
/** /**
...@@ -30,13 +36,14 @@ import TagList from './tag-list'; ...@@ -30,13 +36,14 @@ import TagList from './tag-list';
* *
* @param {AnnotationBodyProps} props * @param {AnnotationBodyProps} props
*/ */
export default function AnnotationBody({ function AnnotationBody({
annotation, annotation,
isEditing, isEditing,
onEditTags, onEditTags,
onEditText, onEditText,
tags, tags,
text, text,
settings,
}) { }) {
// Should the text content of `Excerpt` be rendered in a collapsed state, // Should the text content of `Excerpt` be rendered in a collapsed state,
// assuming it is collapsible (exceeds allotted collapsed space)? // assuming it is collapsible (exceeds allotted collapsed space)?
...@@ -50,6 +57,8 @@ export default function AnnotationBody({ ...@@ -50,6 +57,8 @@ export default function AnnotationBody({
const showExcerpt = !isEditing && text.length > 0; const showExcerpt = !isEditing && text.length > 0;
const showTagList = !isEditing && tags.length > 0; const showTagList = !isEditing && tags.length > 0;
const textStyle = applyTheme(['annotationFontFamily'], settings);
return ( return (
<div className="annotation-body"> <div className="annotation-body">
{showExcerpt && ( {showExcerpt && (
...@@ -62,6 +71,7 @@ export default function AnnotationBody({ ...@@ -62,6 +71,7 @@ export default function AnnotationBody({
overflowThreshold={20} overflowThreshold={20}
> >
<MarkdownView <MarkdownView
textStyle={textStyle}
markdown={text} markdown={text}
textClass={{ textClass={{
'annotation-body__text': true, 'annotation-body__text': true,
...@@ -72,6 +82,7 @@ export default function AnnotationBody({ ...@@ -72,6 +82,7 @@ export default function AnnotationBody({
)} )}
{isEditing && ( {isEditing && (
<MarkdownEditor <MarkdownEditor
textStyle={textStyle}
label="Annotation body" label="Annotation body"
text={text} text={text}
onEditText={onEditText} onEditText={onEditText}
...@@ -105,4 +116,9 @@ AnnotationBody.propTypes = { ...@@ -105,4 +116,9 @@ AnnotationBody.propTypes = {
onEditText: propTypes.func, onEditText: propTypes.func,
tags: propTypes.array.isRequired, tags: propTypes.array.isRequired,
text: propTypes.string, text: propTypes.string,
settings: propTypes.object,
}; };
AnnotationBody.injectedProps = ['settings'];
export default withServices(AnnotationBody);
...@@ -393,6 +393,8 @@ Toolbar.propTypes = { ...@@ -393,6 +393,8 @@ Toolbar.propTypes = {
/** /**
* @typedef MarkdownEditorProps * @typedef MarkdownEditorProps
* @prop {string} label - An accessible label for the input field. * @prop {string} label - An accessible label for the input field.
* @prop {Object.<string,string>} [textStyle] -
* Additional CSS properties to apply to the input field and rendered preview
* @prop {string} [text] - The markdown text to edit. * @prop {string} [text] - The markdown text to edit.
* @prop {(a?: Object<'text', string>) => void} [onEditText] * @prop {(a?: Object<'text', string>) => void} [onEditText]
* - Callback invoked with `{ text }` object when user edits text. * - Callback invoked with `{ text }` object when user edits text.
...@@ -409,6 +411,7 @@ export default function MarkdownEditor({ ...@@ -409,6 +411,7 @@ export default function MarkdownEditor({
label = '', label = '',
onEditText = () => {}, onEditText = () => {},
text = '', text = '',
textStyle = {},
}) { }) {
// Whether the preview mode is currently active. // Whether the preview mode is currently active.
const [preview, setPreview] = useState(false); const [preview, setPreview] = useState(false);
...@@ -452,8 +455,9 @@ export default function MarkdownEditor({ ...@@ -452,8 +455,9 @@ export default function MarkdownEditor({
/> />
{preview ? ( {preview ? (
<MarkdownView <MarkdownView
textClass={{ 'markdown-editor__preview': true }}
markdown={text} markdown={text}
textClass={{ 'markdown-editor__preview': true }}
textStyle={textStyle}
/> />
) : ( ) : (
<textarea <textarea
...@@ -469,6 +473,7 @@ export default function MarkdownEditor({ ...@@ -469,6 +473,7 @@ export default function MarkdownEditor({
}); });
}} }}
value={text} value={text}
style={textStyle}
/> />
)} )}
</div> </div>
...@@ -476,6 +481,7 @@ export default function MarkdownEditor({ ...@@ -476,6 +481,7 @@ export default function MarkdownEditor({
} }
MarkdownEditor.propTypes = { MarkdownEditor.propTypes = {
textStyle: propTypes.object,
label: propTypes.string.isRequired, label: propTypes.string.isRequired,
text: propTypes.string, text: propTypes.string,
onEditText: propTypes.func, onEditText: propTypes.func,
......
...@@ -6,11 +6,26 @@ import propTypes from 'prop-types'; ...@@ -6,11 +6,26 @@ import propTypes from 'prop-types';
import { replaceLinksWithEmbeds } from '../media-embedder'; import { replaceLinksWithEmbeds } from '../media-embedder';
import renderMarkdown from '../render-markdown'; import renderMarkdown from '../render-markdown';
/**
* @typedef MarkdownViewProps
* @prop {string} markdown - The string of markdown to display
* @prop {Object.<string,string>} [textStyle] -
* Additional CSS properties to apply to the rendered markdown
* @prop {Object.<string,boolean>} [textClass] -
* Map of classes to apply to the container of the rendered markdown
*/
/** /**
* A component which renders markdown as HTML and replaces recognized links * A component which renders markdown as HTML and replaces recognized links
* with embedded video/audio. * with embedded video/audio.
*
* @param {MarkdownViewProps} props
*/ */
export default function MarkdownView({ markdown = '', textClass = {} }) { export default function MarkdownView({
markdown = '',
textClass = {},
textStyle = {},
}) {
const html = useMemo(() => (markdown ? renderMarkdown(markdown) : ''), [ const html = useMemo(() => (markdown ? renderMarkdown(markdown) : ''), [
markdown, markdown,
]); ]);
...@@ -34,17 +49,13 @@ export default function MarkdownView({ markdown = '', textClass = {} }) { ...@@ -34,17 +49,13 @@ export default function MarkdownView({ markdown = '', textClass = {} }) {
lang={contentLanguage} lang={contentLanguage}
ref={content} ref={content}
dangerouslySetInnerHTML={{ __html: html }} dangerouslySetInnerHTML={{ __html: html }}
style={textStyle}
/> />
); );
} }
MarkdownView.propTypes = { MarkdownView.propTypes = {
/** The string of markdown to display. */
markdown: propTypes.string, markdown: propTypes.string,
/**
* A CSS classname-to-boolean map of classes to apply to the container of
* the rendered markdown.
*/
textClass: propTypes.object, textClass: propTypes.object,
textStyle: propTypes.object,
}; };
...@@ -11,6 +11,9 @@ import { checkAccessibility } from '../../../test-util/accessibility'; ...@@ -11,6 +11,9 @@ import { checkAccessibility } from '../../../test-util/accessibility';
import mockImportedComponents from '../../../test-util/mock-imported-components'; import mockImportedComponents from '../../../test-util/mock-imported-components';
describe('AnnotationBody', () => { describe('AnnotationBody', () => {
let fakeApplyTheme;
let fakeSettings;
function createBody(props = {}) { function createBody(props = {}) {
return mount( return mount(
<AnnotationBody <AnnotationBody
...@@ -19,13 +22,20 @@ describe('AnnotationBody', () => { ...@@ -19,13 +22,20 @@ describe('AnnotationBody', () => {
onEditTags={() => null} onEditTags={() => null}
tags={[]} tags={[]}
text="test comment" text="test comment"
settings={fakeSettings}
{...props} {...props}
/> />
); );
} }
beforeEach(() => { beforeEach(() => {
fakeApplyTheme = sinon.stub();
fakeSettings = {};
$imports.$mock(mockImportedComponents()); $imports.$mock(mockImportedComponents());
$imports.$mock({
'../util/theme': { applyTheme: fakeApplyTheme },
});
}); });
afterEach(() => { afterEach(() => {
...@@ -119,6 +129,22 @@ describe('AnnotationBody', () => { ...@@ -119,6 +129,22 @@ describe('AnnotationBody', () => {
}); });
}); });
it('applies theme', () => {
const textStyle = { fontFamily: 'serif' };
fakeApplyTheme
.withArgs(['annotationFontFamily'], fakeSettings)
.returns(textStyle);
const wrapper = createBody();
assert.deepEqual(wrapper.find('MarkdownView').prop('textStyle'), textStyle);
wrapper.setProps({ isEditing: true });
assert.deepEqual(
wrapper.find('MarkdownEditor').prop('textStyle'),
textStyle
);
});
it( it(
'should pass a11y checks', 'should pass a11y checks',
checkAccessibility([ checkAccessibility([
......
...@@ -486,6 +486,24 @@ describe('MarkdownEditor', () => { ...@@ -486,6 +486,24 @@ describe('MarkdownEditor', () => {
}); });
}); });
it('applies `textStyle` style to <textarea>', () => {
const textStyle = { fontFamily: 'serif' };
const wrapper = createComponent({ textStyle });
assert.deepEqual(wrapper.find('textarea').prop('style'), textStyle);
});
it('applies `textStyle` style to preview', () => {
const textStyle = { fontFamily: 'serif' };
const wrapper = createComponent({ textStyle });
act(() => {
wrapper.find('Toolbar').props().onTogglePreview();
});
wrapper.update();
assert.deepEqual(wrapper.find('MarkdownView').prop('textStyle'), textStyle);
});
it( it(
'should pass a11y checks', 'should pass a11y checks',
checkAccessibility([ checkAccessibility([
......
...@@ -59,6 +59,15 @@ describe('MarkdownView', () => { ...@@ -59,6 +59,15 @@ describe('MarkdownView', () => {
assert.isTrue(wrapper.find('.markdown-view.fancy-effect').exists()); assert.isTrue(wrapper.find('.markdown-view.fancy-effect').exists());
}); });
it('applies `textStyle` style to container', () => {
const wrapper = mount(
<MarkdownView markdown="foo" textStyle={{ fontFamily: 'serif' }} />
);
assert.deepEqual(wrapper.find('.markdown-view').prop('style'), {
fontFamily: 'serif',
});
});
it( it(
'should pass a11y checks', 'should pass a11y checks',
checkAccessibility({ checkAccessibility({
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment