Skip to content

C
coopwire-hypothesis
Commit 5baa223b authored by Lyza Danger Gardner's avatar Lyza Danger Gardner Committed by Lyza Gardner
Browse files
Options

Extract StyledText and refactor props on related components 

Extract a StyledText component that can be used by MarkdownView and
AnnotationQuote. Update some props on Markdown components to match
conventions.
parent 17e990bb
Hide whitespace changes
Inline Side-by-side
Showing with 82 additions and 93 deletions
src/sidebar/components/Annotation/AnnotationBody.js
View file @ 5baa223b
...@@ -76,11 +76,11 @@ function AnnotationBody({ annotation, settings }) { ...@@ -76,11 +76,11 @@ function AnnotationBody({ annotation, settings }) {
overflowThreshold={20} overflowThreshold={20}
> >
<MarkdownView <MarkdownView
textStyle={textStyle}
markdown={text} markdown={text}
textClass={{ classes={classnames({
'p-redacted-content': isHidden(annotation), 'line-through grayscale contrast-50': isHidden(annotation),
}} })}
style={textStyle}
/> />
</Excerpt> </Excerpt>
)} )}
......
src/sidebar/components/Annotation/AnnotationQuote.js
View file @ 5baa223b
...@@ -4,51 +4,16 @@ import { withServices } from '../../service-context'; ...@@ -4,51 +4,16 @@ import { withServices } from '../../service-context';
import { applyTheme } from '../../helpers/theme'; import { applyTheme } from '../../helpers/theme';
import Excerpt from '../Excerpt'; import Excerpt from '../Excerpt';
import StyledText from '../StyledText';
/** /**
* @typedef {import('../../../types/api').Annotation} Annotation
* @typedef {import('../../../types/config').SidebarSettings} SidebarSettings * @typedef {import('../../../types/config').SidebarSettings} SidebarSettings
*/ */
/**
* Style content as quoted text
*
* @param {object} props
* @param {import('preact').ComponentChildren} props.children
* @param {string} [props.classes] - Additional CSS classes
* @param {object} [props.style] - Inline style object
*/
function QuotedText({ children, classes, style }) {
// The language for the quote may be different than the client's UI (set by
// `<html lang="...">`).
//
// Use a blank string to indicate that it is unknown and it is up to the user
// agent to pick a default or analyze the content and guess.
//
// For web documents we could do better here and gather language information
// as part of the annotation anchoring process.
const documentLanguage = '';
return (
<blockquote
className={classnames(
'border-l-[3px] border-grey-3 hover:border-l-blue-quote',
'italic text-color-text-light px-[1em]',
classes
)}
dir="auto"
lang={documentLanguage}
style={style}
>
{children}
</blockquote>
);
}
/** /**
* @typedef AnnotationQuoteProps * @typedef AnnotationQuoteProps
* @prop {string} quote * @prop {string} quote
* @prop {boolean} [isFocused] - Is this annotation currently focused? * @prop {boolean} [isFocused]
* @prop {boolean} [isOrphan] * @prop {boolean} [isOrphan]
* @prop {SidebarSettings} [settings] - Used for theming. * @prop {SidebarSettings} [settings] - Used for theming.
*/ */
...@@ -61,15 +26,18 @@ function QuotedText({ children, classes, style }) { ...@@ -61,15 +26,18 @@ function QuotedText({ children, classes, style }) {
function AnnotationQuote({ quote, isFocused, isOrphan, settings = {} }) { function AnnotationQuote({ quote, isFocused, isOrphan, settings = {} }) {
return ( return (
<Excerpt collapsedHeight={35} inlineControls={true} overflowThreshold={20}> <Excerpt collapsedHeight={35} inlineControls={true} overflowThreshold={20}>
<QuotedText <StyledText
classes={classnames({ classes={classnames({ 'line-through grayscale contrast-50': isOrphan })}
'border-l-blue-quote': isFocused,
'line-through grayscale contrast-50': isOrphan,
})}
style={applyTheme(['selectionFontFamily'], settings)}
> >
{quote} <blockquote
</QuotedText> className={classnames('hover:border-l-blue-quote', {
'border-l-blue-quote': isFocused,
})}
style={applyTheme(['selectionFontFamily'], settings)}
>
{quote}
</blockquote>
</StyledText>
</Excerpt> </Excerpt>
); );
} }
......
src/sidebar/components/Annotation/test/AnnotationBody-test.js
View file @ 5baa223b
...@@ -179,10 +179,7 @@ describe('AnnotationBody', () => { ...@@ -179,10 +179,7 @@ describe('AnnotationBody', () => {
.returns(textStyle); .returns(textStyle);
const wrapper = createBody(); const wrapper = createBody();
assert.deepEqual( assert.deepEqual(wrapper.find('MarkdownView').prop('style'), textStyle);
wrapper.find('MarkdownView').prop('textStyle'),
textStyle
);
}); });
}); });
......
src/sidebar/components/MarkdownEditor.js
View file @ 5baa223b
...@@ -438,12 +438,8 @@ export default function MarkdownEditor({ ...@@ -438,12 +438,8 @@ export default function MarkdownEditor({
{preview ? ( {preview ? (
<MarkdownView <MarkdownView
markdown={text} markdown={text}
textClass={{ classes="border bg-grey-1 p-2"
'hyp-u-border': true, style={textStyle}
'hyp-u-bg-color--grey-1': true,
'hyp-u-padding': true,
}}
textStyle={textStyle}
/> />
) : ( ) : (
<textarea <textarea
......
src/sidebar/components/MarkdownView.js
View file @ 5baa223b
import classnames from 'classnames';
import { useEffect, useMemo, useRef } from 'preact/hooks'; import { useEffect, useMemo, useRef } from 'preact/hooks';
import { replaceLinksWithEmbeds } from '../media-embedder'; import { replaceLinksWithEmbeds } from '../media-embedder';
import { renderMathAndMarkdown } from '../render-markdown'; import { renderMathAndMarkdown } from '../render-markdown';
import StyledText from './StyledText';
/** /**
* @typedef MarkdownViewProps * @typedef MarkdownViewProps
* @prop {string} markdown - The string of markdown to display * @prop {string} markdown - The string of markdown to display
* @prop {Record<string,string>} [textStyle] - * @prop {string} [classes]
* Additional CSS properties to apply to the rendered markdown * @prop {Record<string,string>} [style]
* @prop {Record<string,boolean>} [textClass] -
* Map of classes to apply to the container of the rendered markdown
*/ */
/** /**
...@@ -19,11 +19,7 @@ import { renderMathAndMarkdown } from '../render-markdown'; ...@@ -19,11 +19,7 @@ import { renderMathAndMarkdown } from '../render-markdown';
* *
* @param {MarkdownViewProps} props * @param {MarkdownViewProps} props
*/ */
export default function MarkdownView({ export default function MarkdownView({ markdown, classes, style }) {
markdown = '',
textClass = {},
textStyle = {},
}) {
const html = useMemo( const html = useMemo(
() => (markdown ? renderMathAndMarkdown(markdown) : ''), () => (markdown ? renderMathAndMarkdown(markdown) : ''),
[markdown] [markdown]
...@@ -39,24 +35,22 @@ export default function MarkdownView({ ...@@ -39,24 +35,22 @@ export default function MarkdownView({
}); });
}, [markdown]); }, [markdown]);
// Use a blank string to indicate that the content language is unknown and may be // NB: The following could be implemented by setting attribute props directly
// different than the client UI. The user agent may pick a default or analyze // on `StyledText` (which renders a `div` itself), versus introducing a child
// the content to guess. // `div` as is done here. However, in initial testing, this interfered with
const contentLanguage = ''; // some overflow calculations in the `Excerpt` element. This could be worth
// a review in the future.
return ( return (
<div <div className="w-full break-words cursor-text">
className="w-full break-words cursor-text" <StyledText>
dir="auto" <div
lang={contentLanguage} className={classes}
> data-testid="markdown-text"
<div ref={content}
className={classnames('styled-text', textClass)} dangerouslySetInnerHTML={{ __html: html }}
data-testid="styled-text" style={style}
ref={content} />
dangerouslySetInnerHTML={{ __html: html }} </StyledText>
style={textStyle}
/>
</div> </div>
); );
} }
src/sidebar/components/StyledText.js 0 → 100644
View file @ 5baa223b
import classnames from 'classnames';
/**
* @typedef StyledTextProps
* @prop {import('preact').ComponentChildren} children
* @prop {string} [classes]
*/
/**
* Render children as styled text: basic prose styling for HTML
*
* @param {StyledTextProps & import('preact').JSX.HTMLAttributes<HTMLDivElement>} props
*/
export default function StyledText({ children, classes, ...restProps }) {
// The language for the quote may be different than the client's UI (set by
// `<html lang="...">`).
//
// Use a blank string to indicate that it is unknown and it is up to the user
// agent to pick a default or analyze the content and guess.
//
// For web documents we could do better here and gather language information
// as part of the annotation anchoring process.
const documentLanguage = '';
return (
<div
dir="auto"
lang={documentLanguage}
className={classnames('StyledText', classes)}
{...restProps}
>
{children}
</div>
);
}
src/sidebar/components/test/MarkdownEditor-test.js
View file @ 5baa223b
...@@ -500,7 +500,7 @@ describe('MarkdownEditor', () => { ...@@ -500,7 +500,7 @@ describe('MarkdownEditor', () => {
}); });
wrapper.update(); wrapper.update();
assert.deepEqual(wrapper.find('MarkdownView').prop('textStyle'), textStyle); assert.deepEqual(wrapper.find('MarkdownView').prop('style'), textStyle);
}); });
it( it(
......
src/sidebar/components/test/MarkdownView-test.js
View file @ 5baa223b
...@@ -8,7 +8,7 @@ describe('MarkdownView', () => { ...@@ -8,7 +8,7 @@ describe('MarkdownView', () => {
let fakeRenderMathAndMarkdown; let fakeRenderMathAndMarkdown;
let fakeReplaceLinksWithEmbeds; let fakeReplaceLinksWithEmbeds;
const markdownSelector = '[data-testid="styled-text"]'; const markdownSelector = '[data-testid="markdown-text"]';
beforeEach(() => { beforeEach(() => {
fakeRenderMathAndMarkdown = markdown => `rendered:${markdown}`; fakeRenderMathAndMarkdown = markdown => `rendered:${markdown}`;
...@@ -56,14 +56,14 @@ describe('MarkdownView', () => { ...@@ -56,14 +56,14 @@ describe('MarkdownView', () => {
it('applies `textClass` class to container', () => { it('applies `textClass` class to container', () => {
const wrapper = mount( const wrapper = mount(
<MarkdownView markdown="foo" textClass={{ 'fancy-effect': true }} /> <MarkdownView markdown="foo" classes={'fancy-effect'} />
); );
assert.isTrue(wrapper.find('.fancy-effect').exists()); assert.isTrue(wrapper.find('.fancy-effect').exists());
}); });
it('applies `textStyle` style to container', () => { it('applies `textStyle` style to container', () => {
const wrapper = mount( const wrapper = mount(
<MarkdownView markdown="foo" textStyle={{ fontFamily: 'serif' }} /> <MarkdownView markdown="foo" style={{ fontFamily: 'serif' }} />
); );
assert.deepEqual(wrapper.find(markdownSelector).prop('style'), { assert.deepEqual(wrapper.find(markdownSelector).prop('style'), {
fontFamily: 'serif', fontFamily: 'serif',
......
src/styles/sidebar/components/styled-text.scss → src/styles/sidebar/components/StyledText.scss
View file @ 5baa223b
...@@ -6,7 +6,7 @@ ...@@ -6,7 +6,7 @@
* https://github.com/hypothesis/client/pull/4295 * https://github.com/hypothesis/client/pull/4295
*/ */
@layer components { @layer components {
.styled-text { .StyledText {
@apply font-sans font-normal leading-snug; @apply font-sans font-normal leading-snug;
img, img,
......
src/styles/sidebar/components/_index.scss
View file @ 5baa223b
...@@ -29,6 +29,7 @@ ...@@ -29,6 +29,7 @@
@use './PaginationNavigation'; @use './PaginationNavigation';
@use './SelectionTabs'; @use './SelectionTabs';
@use './SearchInput'; @use './SearchInput';
@use './StyledText';
@use './TagEditor'; @use './TagEditor';
@use './Thread'; @use './Thread';
@use './ThreadCard'; @use './ThreadCard';
...@@ -36,5 +37,3 @@ ...@@ -36,5 +37,3 @@
@use './ToastMessages'; @use './ToastMessages';
@use './TopBar'; @use './TopBar';
@use './VersionInfo'; @use './VersionInfo';
@use './styled-text';
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