From 3df38d4e63831a725de522156d52e8e27746f0f9 Mon Sep 17 00:00:00 2001 From: Ryan Oglesby Date: Sun, 10 Dec 2017 13:52:56 -0600 Subject: [PATCH] Fix: Use Text component for rendering text blocks, em and strong in Markdown (#731) --- src/rsg-components/Markdown/Markdown.js | 37 ++++--- .../__snapshots__/Markdown.spec.js.snap | 100 +++++++++--------- src/rsg-components/Para/Para.spec.js | 20 +++- src/rsg-components/Para/ParaRenderer.js | 9 +- .../Para/__snapshots__/Para.spec.js.snap | 14 ++- src/rsg-components/Props/PropsRenderer.js | 17 ++- .../Props/__snapshots__/Props.spec.js.snap | 36 +++++-- src/rsg-components/Text/Text.spec.js | 70 ++++++++---- src/rsg-components/Text/TextRenderer.js | 44 +++++++- .../Text/__snapshots__/Text.spec.js.snap | 36 +++++-- 10 files changed, 269 insertions(+), 114 deletions(-) diff --git a/src/rsg-components/Markdown/Markdown.js b/src/rsg-components/Markdown/Markdown.js index 0180d3a6e..544438cc9 100644 --- a/src/rsg-components/Markdown/Markdown.js +++ b/src/rsg-components/Markdown/Markdown.js @@ -5,7 +5,8 @@ import mapValues from 'lodash/mapValues'; // import memoize from 'lodash/memoize'; import Styled from 'rsg-components/Styled'; import Link from 'rsg-components/Link'; -import { styles as paraStyles } from 'rsg-components/Para'; +import Text from 'rsg-components/Text'; +import Para, { styles as paraStyles } from 'rsg-components/Para'; import MarkdownHeading from 'rsg-components/Markdown/MarkdownHeading'; // We’re explicitly specifying Webpack loaders here so we could skip specifying them in Webpack configuration. @@ -79,6 +80,24 @@ const getBaseOverrides = memoize(classes => { level: 6, }, }, + p: { + component: Para, + props: { + semantic: 'p', + }, + }, + em: { + component: Text, + props: { + semantic: 'em', + }, + }, + strong: { + component: Text, + props: { + semantic: 'strong', + }, + }, code: { component: Code, props: { @@ -95,10 +114,7 @@ const getInlineOverrides = memoize(classes => { return { ...overrides, p: { - component: 'span', - props: { - className: classes.base, - }, + component: Text, }, }; }, () => 'getInlineOverrides'); @@ -110,9 +126,6 @@ const styles = ({ space, fontFamily, fontSize, color, borderRadius }) => ({ fontSize: 'inherit', }, para: paraStyles({ space, color, fontFamily }).para, - p: { - composes: '$para', - }, ul: { composes: '$para', paddingLeft: space[3], @@ -144,14 +157,6 @@ const styles = ({ space, fontFamily, fontSize, color, borderRadius }) => ({ borderColor: color.border, borderStyle: 'solid', }, - em: { - composes: '$base', - fontStyle: 'italic', - }, - strong: { - composes: '$base', - fontWeight: 'bold', - }, code: { fontFamily: fontFamily.monospace, fontSize: 'inherit', diff --git a/src/rsg-components/Markdown/__snapshots__/Markdown.spec.js.snap b/src/rsg-components/Markdown/__snapshots__/Markdown.spec.js.snap index 8f2b7de24..6289dc1e1 100644 --- a/src/rsg-components/Markdown/__snapshots__/Markdown.spec.js.snap +++ b/src/rsg-components/Markdown/__snapshots__/Markdown.spec.js.snap @@ -2,9 +2,9 @@ exports[`Markdown should render Markdown in span in inline mode 1`] = ` - + Hello - + world ! @@ -15,28 +15,28 @@ exports[`Markdown should render Markdown in span in inline mode 1`] = ` exports[`Markdown should render Markdown with custom CSS classes 1`] = `
-
-

+
+

Header

-

+

Text with - + some - + formatting and a link .

-

+

Image @@ -47,24 +47,24 @@ exports[`Markdown should render Markdown with custom CSS classes 1`] = ` exports[`Markdown should render check-lists correctly 1`] = ` -

    -
  • +
      +
    • list 1
    • -
    • +
    • list 2
    • -
    • +
    • @@ -76,8 +76,8 @@ exports[`Markdown should render check-lists correctly 1`] = ` exports[`Markdown should render code blocks without escaping 1`] = ` -
      -  
      +
      +  
           
           
         
      @@ -88,33 +88,33 @@ exports[`Markdown should render code blocks without escaping 1`] = `
       exports[`Markdown should render headings correctly 1`] = `
       
       
      -
      -

      +
      +

      one

      -
      -

      +
      +

      two

      -
      -

      +
      +

      three

      -
      -

      +
      +

      four

      -
      -
      +
      +
      five
      -
      -
      +
      +
      six
      @@ -124,9 +124,9 @@ exports[`Markdown should render headings correctly 1`] = ` exports[`Markdown should render inline code with escaping 1`] = ` -

      +

      Foo - + <bar> baz @@ -136,10 +136,10 @@ exports[`Markdown should render inline code with escaping 1`] = ` exports[`Markdown should render links 1`] = ` -

      +

      a link @@ -149,27 +149,27 @@ exports[`Markdown should render links 1`] = ` exports[`Markdown should render mixed nested lists correctly 1`] = ` -

        -
      • +
          +
        • list 1
        • -
        • +
        • list 2 -
            -
          1. +
          2. Sub-list
          3. -
          4. +
          5. Sub-list
          6. -
          7. +
          8. Sub-list
        • -
        • +
        • list 3
        @@ -178,16 +178,16 @@ exports[`Markdown should render mixed nested lists correctly 1`] = ` exports[`Markdown should render ordered lists correctly 1`] = ` -
          -
        1. +
        2. list
        3. -
        4. +
        5. item
        6. -
        7. +
        8. three
        @@ -196,14 +196,14 @@ exports[`Markdown should render ordered lists correctly 1`] = ` exports[`Markdown should render unordered lists correctly 1`] = ` -
          -
        • +
            +
          • list
          • -
          • +
          • item
          • -
          • +
          • three
          diff --git a/src/rsg-components/Para/Para.spec.js b/src/rsg-components/Para/Para.spec.js index 379109953..0b188a7d0 100644 --- a/src/rsg-components/Para/Para.spec.js +++ b/src/rsg-components/Para/Para.spec.js @@ -1,8 +1,22 @@ import React from 'react'; -import { ParaRenderer } from './ParaRenderer'; +import { ParaRenderer, styles } from './ParaRenderer'; -it('should render paragraph', () => { - const actual = shallow(Pizza); +const props = { + classes: classes(styles), +}; + +it('should render paragraph as a
          ', () => { + const actual = shallow(Pizza); + + expect(actual).toMatchSnapshot(); +}); + +it('should render paragraph as a

          ', () => { + const actual = shallow( + + Pizza + + ); expect(actual).toMatchSnapshot(); }); diff --git a/src/rsg-components/Para/ParaRenderer.js b/src/rsg-components/Para/ParaRenderer.js index 18e562b5c..ed20e7f25 100644 --- a/src/rsg-components/Para/ParaRenderer.js +++ b/src/rsg-components/Para/ParaRenderer.js @@ -2,7 +2,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import Styled from 'rsg-components/Styled'; -export const styles = ({ space, fontFamily, color }) => ({ +export const styles = ({ space, color, fontFamily }) => ({ para: { marginTop: 0, marginBottom: space[2], @@ -13,12 +13,15 @@ export const styles = ({ space, fontFamily, color }) => ({ }, }); -export function ParaRenderer({ classes, children }) { - return

          {children}
          ; +export function ParaRenderer({ classes, semantic, children }) { + const Tag = semantic || 'div'; + + return {children}; } ParaRenderer.propTypes = { classes: PropTypes.object.isRequired, + semantic: PropTypes.oneOf(['p']), children: PropTypes.node.isRequired, }; diff --git a/src/rsg-components/Para/__snapshots__/Para.spec.js.snap b/src/rsg-components/Para/__snapshots__/Para.spec.js.snap index c4a88d19f..3cdc857ec 100644 --- a/src/rsg-components/Para/__snapshots__/Para.spec.js.snap +++ b/src/rsg-components/Para/__snapshots__/Para.spec.js.snap @@ -1,7 +1,17 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`should render paragraph 1`] = ` -
          +exports[`should render paragraph as a
          1`] = ` +
          Pizza
          `; + +exports[`should render paragraph as a

          1`] = ` +

          + Pizza +

          +`; diff --git a/src/rsg-components/Props/PropsRenderer.js b/src/rsg-components/Props/PropsRenderer.js index 12425e92f..0c1e4db70 100644 --- a/src/rsg-components/Props/PropsRenderer.js +++ b/src/rsg-components/Props/PropsRenderer.js @@ -77,7 +77,11 @@ const defaultValueBlacklist = ['null', 'undefined']; function renderDefault(prop) { if (prop.required) { - return Required; + return ( + + Required + + ); } else if (prop.defaultValue) { if (prop.type) { const propName = prop.type.name; @@ -86,7 +90,12 @@ function renderDefault(prop) { return {showSpaces(unquote(prop.defaultValue.value))}; } else if (propName === 'func') { return ( - + Function ); @@ -98,7 +107,7 @@ function renderDefault(prop) { // eslint-disable-next-line no-eval const object = eval(`(${prop.defaultValue.value})`); return ( - + Shape ); @@ -107,7 +116,7 @@ function renderDefault(prop) { // local scope. To avoid any breakage we fall back to rendering the // prop without any formatting return ( - + Shape ); diff --git a/src/rsg-components/Props/__snapshots__/Props.spec.js.snap b/src/rsg-components/Props/__snapshots__/Props.spec.js.snap index 450e3a7db..188fede2c 100644 --- a/src/rsg-components/Props/__snapshots__/Props.spec.js.snap +++ b/src/rsg-components/Props/__snapshots__/Props.spec.js.snap @@ -242,7 +242,10 @@ exports[`props columns should render PropTypes.objectOf(PropTypes.shape) 1`] = ` number — - + Required
          @@ -422,7 +425,10 @@ exports[`props columns should render PropTypes.shape 1`] = ` number — - + Required
          @@ -470,6 +476,8 @@ exports[`props columns should render PropTypes.shape defaultProps, falling back key="2" > @@ -492,7 +500,10 @@ exports[`props columns should render PropTypes.shape defaultProps, falling back number — - + Required
          @@ -555,7 +566,10 @@ exports[`props columns should render PropTypes.shape with description 1`] = ` number — - + Required — @@ -613,6 +627,8 @@ exports[`props columns should render PropTypes.shape with formatted defaultProps key="2" > Required
      @@ -768,7 +787,10 @@ exports[`props columns should render PropTypes.string.isRequired 1`] = `
      - + Required
      @@ -920,6 +942,8 @@ exports[`props columns should render function body in tooltip 1`] = ` key="2" > diff --git a/src/rsg-components/Text/Text.spec.js b/src/rsg-components/Text/Text.spec.js index 9a929f7fa..e4293dabb 100644 --- a/src/rsg-components/Text/Text.spec.js +++ b/src/rsg-components/Text/Text.spec.js @@ -5,28 +5,60 @@ const props = { classes: classes(styles), }; -it('should render text', () => { - const actual = shallow(Pizza); +describe('Text', () => { + it('should render text', () => { + const actual = shallow(Pizza); - expect(actual).toMatchSnapshot(); -}); + expect(actual).toMatchSnapshot(); + }); -it('should render underlined text', () => { - const actual = shallow( - - Pizza - - ); + it('should render underlined text', () => { + const actual = shallow( + + Pizza + + ); - expect(actual).toMatchSnapshot(); -}); + expect(actual).toMatchSnapshot(); + }); + + it('should render sized text', () => { + const actual = shallow( + + Pizza + + ); + + expect(actual).toMatchSnapshot(); + }); + + it('should render colored text', () => { + const actual = shallow( + + Pizza + + ); + + expect(actual).toMatchSnapshot(); + }); + + it('should render text with a semantic tag and styles', () => { + const actual = shallow( + + Pizza + + ); + + expect(actual).toMatchSnapshot(); + }); -it('should render text with a title', () => { - const actual = shallow( - - Pizza - - ); + it('should render text with a title', () => { + const actual = shallow( + + Pizza + + ); - expect(actual).toMatchSnapshot(); + expect(actual).toMatchSnapshot(); + }); }); diff --git a/src/rsg-components/Text/TextRenderer.js b/src/rsg-components/Text/TextRenderer.js index f9d6ca684..7a1815533 100644 --- a/src/rsg-components/Text/TextRenderer.js +++ b/src/rsg-components/Text/TextRenderer.js @@ -6,29 +6,63 @@ import cx from 'classnames'; export const styles = ({ fontFamily, fontSize, color }) => ({ text: { fontFamily: fontFamily.base, + }, + inheritSize: { + fontSize: 'inherit', + }, + smallSize: { fontSize: fontSize.small, + }, + baseSize: { + fontSize: fontSize.base, + }, + textSize: { + fontSize: fontSize.text, + }, + baseColor: { + color: color.base, + }, + lightColor: { color: color.light, }, + em: { + fontStyle: 'italic', + }, + strong: { + fontWeight: 'bold', + }, isUnderlined: { borderBottom: [[1, 'dotted', color.lightest]], }, }); -export function TextRenderer({ classes, children, underlined, ...other }) { - const classNames = cx(classes.text, { +export function TextRenderer({ classes, semantic, size, color, underlined, children, ...props }) { + const Tag = semantic || 'span'; + const classNames = cx(classes.text, classes[`${size}Size`], classes[`${color}Color`], { + [classes[semantic]]: semantic, [classes.isUnderlined]: underlined, }); + return ( - + {children} - + ); } TextRenderer.propTypes = { classes: PropTypes.object.isRequired, - children: PropTypes.node.isRequired, + semantic: PropTypes.oneOf(['em', 'strong']), + size: PropTypes.oneOf(['inherit', 'small', 'base', 'text']), + color: PropTypes.oneOf(['base', 'light']), underlined: PropTypes.bool, + children: PropTypes.node.isRequired, +}; + +TextRenderer.defaultProps = { + size: 'inherit', + color: 'base', + underlined: false, }; export default Styled(styles)(TextRenderer); diff --git a/src/rsg-components/Text/__snapshots__/Text.spec.js.snap b/src/rsg-components/Text/__snapshots__/Text.spec.js.snap index 61400dc87..bc79a4202 100644 --- a/src/rsg-components/Text/__snapshots__/Text.spec.js.snap +++ b/src/rsg-components/Text/__snapshots__/Text.spec.js.snap @@ -1,25 +1,49 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`should render text 1`] = ` +exports[`Text should render colored text 1`] = ` Pizza `; -exports[`should render text with a title 1`] = ` +exports[`Text should render sized text 1`] = ` + Pizza + +`; + +exports[`Text should render text 1`] = ` + + Pizza + +`; + +exports[`Text should render text with a semantic tag and styles 1`] = ` + + Pizza + +`; + +exports[`Text should render text with a title 1`] = ` + Pizza `; -exports[`should render underlined text 1`] = ` +exports[`Text should render underlined text 1`] = ` Pizza