Skip to content

Commit

Permalink
feat(text): Code Review updates
Browse files Browse the repository at this point in the history
Let Text inherit font-size and weight from Paragraph and Remove Paragraph.Sup and Paragraph.Sub
  • Loading branch information
lzcabrera committed Aug 31, 2017
1 parent 64c504e commit 66c5dfb
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 44 deletions.
34 changes: 12 additions & 22 deletions src/components/Typography/Paragraph/Paragraph.jsx
Original file line number Diff line number Diff line change
@@ -1,28 +1,22 @@
import React from 'react'
import PropTypes from 'prop-types'
import classnames from 'classnames'

import safeRest from '../../../safeRest'
import TextSup from '../Text/TextSup/TextSup'
import TextSub from '../Text/TextSub/TextSub'

import spacingStyles from '../../Spacing.modules.scss'
import styles from './Paragraph.modules.scss'
import textStyles from '../Text/Text.modules.scss'

/**
* Used for paragraphs of text.
*/
const Paragraph = ({ bold, size, align, invert, children, ...rest }, context) => {
const paragraphColor = invert ? textStyles.colorInverted : textStyles.color

const classes = classnames(
spacingStyles.noSpacing,
context.inheritColor ? styles.inheritColor : paragraphColor,
textStyles[size],
bold ? textStyles.boldFont : textStyles[`${size}Font`],
styles[`${align}Align`]
)
const classes = `
${spacingStyles.noSpacing}
${context.inheritColor ? styles.inheritColor : paragraphColor}
${textStyles[size]}
${bold ? textStyles.boldFont : textStyles[`${size}Font`]}
${styles[`${align}Align`]}
`

return (
<p {...safeRest(rest)} className={classes}>
Expand All @@ -33,32 +27,31 @@ const Paragraph = ({ bold, size, align, invert, children, ...rest }, context) =>

Paragraph.propTypes = {
/**
* Set a paragraph of text stylistically different from normal text by using the boldface,
* without conveying any special importance or relevance.
* Embolden paragraph text without conveying any special importance or relevance.
*/
bold: PropTypes.bool,
/**
* Font size (default is `medium`)
* Font size.
*/
size: PropTypes.oneOf([
'small',
'medium',
'large'
]),
/**
* Align paragraph content.
* Align content.
*/
align: PropTypes.oneOf([
'left',
'center',
'right'
]),
/**
* Invert paragraph style to appear light on dark backgrounds.
* Invert to appear light on dark backgrounds.
*/
invert: PropTypes.bool,
/**
* Paragraph text.
* The content. Can be text, other components, or HTML elements.
*/
children: PropTypes.node.isRequired
}
Expand All @@ -74,7 +67,4 @@ Paragraph.contextTypes = {
inheritColor: PropTypes.bool
}

Paragraph.Sup = TextSup
Paragraph.Sub = TextSub

export default Paragraph
2 changes: 1 addition & 1 deletion src/components/Typography/Paragraph/Paragraph.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

```
<Paragraph>Enjoy your choice of <Link href="#">5 channels</Link>, and up to <Link href="#">23 local and regional channels</Link>. Plus, add Premium channel packs like HBO, TMN &amp; Crave TV for just $20/mo. All delivered via TELUS Internet through the Pik TV media box and Pik TV app.<Paragraph.Sup>2</Paragraph.Sup>
<Paragraph>Enjoy your choice of <Link href="#">5 channels</Link>, and up to <Link href="#">23 local and regional channels</Link>. Plus, add Premium channel packs like HBO, TMN &amp; Crave TV for just $20/mo. All delivered via TELUS Internet through the Pik TV media box and Pik TV app.<Text.Sup>2</Text.Sup>
</Paragraph>
```
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@

exports[`Paragraph renders 1`] = `
<p
className="noSpacing color medium mediumFont leftAlign"
className="
noSpacing
color
medium
mediumFont
leftAlign
"
>
Some content
</p>
Expand Down
25 changes: 10 additions & 15 deletions src/components/Typography/Text/Text.jsx
Original file line number Diff line number Diff line change
@@ -1,23 +1,18 @@
import React from 'react'
import PropTypes from 'prop-types'
import classnames from 'classnames'

import safeRest from '../../../safeRest'
import TextSup from './TextSup/TextSup'
import TextSub from './TextSub/TextSub'

import styles from './Text.modules.scss'


/**
* Used for inline text.
*/
const Text = ({ bold, size, invert, children, ...rest }) => {
const classes = classnames(
styles[size],
bold ? styles.boldFont : styles[`${size}Font`],
invert ? styles.colorInverted : styles.color
)
const classes = `
${styles[size]}
${bold ? styles.boldFont : styles[`${size}Font`]}
${invert ? styles.colorInverted : styles.color}
`

return (
<span {...safeRest(rest)} className={classes}>
Expand All @@ -28,14 +23,14 @@ const Text = ({ bold, size, invert, children, ...rest }) => {

Text.propTypes = {
/**
* Set a span of text stylistically different from normal text by using the boldface,
* without conveying any special importance or relevance.
* Embolden text without conveying any special importance or relevance.
*/
bold: PropTypes.bool,
/**
* Font size (default is `medium`)
* Font size.
*/
size: PropTypes.oneOf([
'base',
'small',
'medium',
'large'
Expand All @@ -45,14 +40,14 @@ Text.propTypes = {
*/
invert: PropTypes.bool,
/**
* Text
* The text you wish to apply special styles.
*/
children: PropTypes.node.isRequired
}

Text.defaultProps = {
bold: false,
size: 'medium',
size: 'base',
invert: false
}

Expand Down
8 changes: 8 additions & 0 deletions src/components/Typography/Text/Text.modules.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
@import '../../../scss/settings/colours';
@import '../../../scss/settings/typography';

.base {
font-size: inherit;
}

.baseFont {
font-weight: inherit;
}

.small {
font-size: .875rem;
letter-spacing: -0.6px;
Expand Down
2 changes: 1 addition & 1 deletion src/components/Typography/Text/TextSup/TextSup.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

```
<Paragraph>Enjoy your choice of <Link href="#">5 channels</Link>, and up to <Link href="#">23 local and regional channels</Link>. Plus, add Premium channel packs like HBO, TMN &amp; Crave TV for just $20/mo. All delivered via TELUS Internet through the Pik TV media box and Pik TV app.<Paragraph.Sup>2</Paragraph.Sup>
<Paragraph>Enjoy your choice of <Link href="#">5 channels</Link>, and up to <Link href="#">23 local and regional channels</Link>. Plus, add Premium channel packs like HBO, TMN &amp; Crave TV for just $20/mo. All delivered via TELUS Internet through the Pik TV media box and Pik TV app.<Text.Sup>2</Text.Sup>
</Paragraph>
```

Expand Down
4 changes: 1 addition & 3 deletions src/components/Typography/Text/__tests__/Text.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,9 @@ describe('Text', () => {
it('can be bold', () => {
let text = doShallow()
expect(text).not.toHaveClassName('boldFont')
expect(text).toHaveClassName('mediumFont')

text = doShallow({ bold: true })
expect(text).toHaveClassName('boldFont')
expect(text).not.toHaveClassName('mediumFont')
})

it('can be inverted', () => {
Expand All @@ -43,7 +41,7 @@ describe('Text', () => {

it('can be sized', () => {
let text = doShallow()
expect(text).toHaveClassName('medium mediumFont')
expect(text).toHaveClassName('base baseFont')

text = doShallow({ size: 'small' })
expect(text).toHaveClassName('small smallFont')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@

exports[`Text renders 1`] = `
<span
className="medium mediumFont color"
className="
base
baseFont
color
"
>
Some content
</span>
Expand Down

0 comments on commit 66c5dfb

Please sign in to comment.