Skip to content

Commit

Permalink
refactor(spacing): Removed all usages of the spacing Sass variables
Browse files Browse the repository at this point in the history
Change ChevronLink and Lists to use Box
  • Loading branch information
ryanoglesby08 committed Oct 3, 2017
1 parent 611e0d7 commit 281264b
Show file tree
Hide file tree
Showing 18 changed files with 112 additions and 106 deletions.
4 changes: 1 addition & 3 deletions docs/components/Hero/Hero.modules.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
@import '../../../src/scss/settings/spacing';

.hero {
display: flex;
flex-direction: column;
Expand All @@ -8,5 +6,5 @@
background-image: url("./hero-background.jpg");
background-size: cover;
height: 200px;
padding: $spacing-wide;
padding: 2rem;
}
3 changes: 1 addition & 2 deletions docs/components/PurpleBlock/PurpleBlock.modules.scss
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
@import '../../../src/scss/settings/colours';
@import '../../../src/scss/settings/spacing';

.purpleBlock {
background-color: $color-secondary;
padding: $spacing-wide;
padding: 2rem;
}
4 changes: 1 addition & 3 deletions docs/scss/_layout.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
@import '../../src/scss/settings/spacing';

.docs--layout-vertically {
display: flex;
flex-direction: column;
Expand All @@ -12,6 +10,6 @@

.docs--horizontal-spacing {
> * {
margin-right: $spacing-base;
margin-right: 1rem;
}
}
40 changes: 22 additions & 18 deletions src/components/Box/Box.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ const getClasses = (
)

const Box = ({
inline,
spacing,
all,
vertical,
Expand All @@ -49,26 +50,28 @@ const Box = ({
dangerouslyAddClassName,
children,
...rest
}) => (
<div
{...safeRest(rest)}
className={getClasses(
spacing,
all,
vertical,
horizontal,
top,
right,
bottom,
left,
dangerouslyAddClassName
)}
>
{children}
</div>
)
}) =>
React.createElement(
inline ? 'span' : 'div',
{
...safeRest(rest),
className: getClasses(
spacing,
all,
vertical,
horizontal,
top,
right,
bottom,
left,
dangerouslyAddClassName
),
},
children
)

Box.propTypes = {
inline: PropTypes.bool,
spacing: PropTypes.oneOf(['margin', 'padding']).isRequired,
all: PropTypes.oneOf([1, 2, 3, 4, 6]),
vertical: PropTypes.oneOf([1, 2, 3, 4, 6]),
Expand All @@ -82,6 +85,7 @@ Box.propTypes = {
}

Box.defaultProps = {
inline: false,
all: undefined,
vertical: undefined,
horizontal: undefined,
Expand Down
10 changes: 9 additions & 1 deletion src/components/Box/__tests__/Box.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import Box from '../Box'
describe('Box', () => {
const defaultProps = {
spacing: 'padding',
vertical: 2
vertical: 2,
}

const doShallow = (props = {}) =>
Expand Down Expand Up @@ -66,6 +66,14 @@ describe('Box', () => {
expect(box).toHaveClassName('topPadding-1 rightPadding-2 bottomPadding-3 leftPadding-4')
})

it('can be either inline or block', () => {
let box = doShallow()
expect(box).toHaveTagName('div')

box = doShallow({ inline: true })
expect(box).toHaveTagName('span')
})

it('will add additional arbitrary class names', () => {
const box = doShallow({ spacing: 'margin', right: 3, dangerouslyAddClassName: 'a-class' })

Expand Down
5 changes: 2 additions & 3 deletions src/components/Button/Button.modules.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
@import '../../scss/settings/colours';
@import '../../scss/settings/spacing';
@import '../../scss/settings/responsive-grid';
@import '../../scss/utility/responsive';

Expand All @@ -22,7 +21,7 @@ $button-text-color: $color-white;
align-items: center;
justify-content: center;

padding: 0 $spacing-wide;
padding: 0 2rem;

transition: background 0.2s;

Expand All @@ -41,7 +40,7 @@ $button-text-color: $color-white;
.button + .button {
// Using < 20px to compensate for the extra margin which appears between inline-block
// elements due to whitespace in the actual markup.
margin-left: 16px;
margin-left: 1rem;
}
}

Expand Down
5 changes: 3 additions & 2 deletions src/components/Input/Input.modules.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
@import '../../scss/settings/colours';
@import '../../scss/settings/typography';
@import '../../scss/settings/spacing';

// FIXME: Override the global styles in forms.scss. This can be removed when the global styles are purged.
.resetLabel {
Expand Down Expand Up @@ -75,7 +74,9 @@ input.input {
//composes: noMargin from '../Spacing/Spacing.modules.scss';
margin: 0;
// This top+bottom padding fixes an IE11 bug that was causing the input value to jump slightly and get cut off.
padding: $spacing-tight 0;
// FIXME: Will be able to use composes when removing the targeting of the <input> element above
// composes: verticalPadding-2 from '../Box/Box.modules.scss';
padding: 0.5rem 0;

outline: 0;

Expand Down
40 changes: 18 additions & 22 deletions src/components/Link/ChevronLink/ChevronLink.jsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import React from 'react'
import PropTypes from 'prop-types'

import safeRest from '../../../utils/safeRest'
import Box from '../../Box/Box'
import DecorativeIcon from '../../../components/Icons/DecorativeIcon/DecorativeIcon'
import safeRest from '../../../utils/safeRest'
import { warn } from '../../../utils/warn'

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

const getClassName = (variant) => {
const getClassName = variant => {
switch (variant) {
case 'secondary':
return styles.secondary
Expand All @@ -18,29 +19,31 @@ const getClassName = (variant) => {
}
}

const getIcon = (symbol, className) => (
<span className={className}>
<DecorativeIcon symbol={symbol} size={16} />
</span>
)
const getIcon = (symbol, className) => {
const direction = symbol === 'leftChevron' ? { right: 2 } : { left: 2 }

return (
<Box inline spacing="margin" {...direction} dangerouslyAddClassName={className}>
<DecorativeIcon symbol={symbol} size={16} />
</Box>
)
}

/**
* A call to action link.
*
* <span class="docs--badge__new">new!</span> <span class="docs--badge__version">v0.21.0</span>
*/
const ChevronLink = ({ reactRouterLinkComponent, variant, direction, children, ...rest }) => {
if ((reactRouterLinkComponent || rest.to) &&
!(reactRouterLinkComponent && rest.to)) {
if ((reactRouterLinkComponent || rest.to) && !(reactRouterLinkComponent && rest.to)) {
warn('Chevron Link', 'The props `reactRouterLinkComponent` and `to` must be used together.')
}

return React.createElement(

reactRouterLinkComponent || 'a',
{
...safeRest(rest),
className: getClassName(variant)
className: getClassName(variant),
},
direction === 'left' ? getIcon('leftChevron', styles.leftChevron) : undefined,
children,
Expand All @@ -52,18 +55,11 @@ ChevronLink.propTypes = {
/**
* The style.
*/
variant: PropTypes.oneOf([
'primary',
'secondary',
'inverted'
]),
variant: PropTypes.oneOf(['primary', 'secondary', 'inverted']),
/**
* The chevron's direction and placement.
*/
direction: PropTypes.oneOf([
'left',
'right'
]),
direction: PropTypes.oneOf(['left', 'right']),
/**
* React Router Link component.
*/
Expand All @@ -79,14 +75,14 @@ ChevronLink.propTypes = {
/**
* The label.
*/
children: PropTypes.string.isRequired
children: PropTypes.string.isRequired,
}
ChevronLink.defaultProps = {
variant: 'primary',
direction: 'right',
reactRouterLinkComponent: null,
to: null,
href: null
href: null,
}

export default ChevronLink
2 changes: 0 additions & 2 deletions src/components/Link/ChevronLink/ChevronLink.modules.scss
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,8 @@

.rightChevron {
composes: chevron;
margin-left: 0.5rem;
}

.leftChevron {
composes: chevron;
margin-right: 0.5rem;
}
17 changes: 9 additions & 8 deletions src/components/Link/ChevronLink/__tests__/ChevronLink.spec.jsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import React from 'react'
import { shallow } from 'enzyme'
import { shallow, render } from 'enzyme'
import toJson from 'enzyme-to-json'
import { warn } from '../../../../utils/warn'

import ChevronLink from '../ChevronLink'
import DecorativeIcon from '../../../Icons/DecorativeIcon/DecorativeIcon'
import Box from '../../../Box/Box'

jest.mock('../../../../utils/warn', () => (
{ warn: jest.fn() }
Expand All @@ -20,7 +21,7 @@ describe('ChevronLink', () => {
})

it('renders', () => {
const link = doShallow()
const link = render(<ChevronLink href="test.com">Go home</ChevronLink>)

expect(toJson(link)).toMatchSnapshot()
})
Expand All @@ -41,13 +42,13 @@ describe('ChevronLink', () => {

it('must use `reactRouterLinkComponent` and `to` props together', () => {
const MyLink = () => <span />
let link = doShallow({ reactRouterLinkComponent: MyLink })
doShallow({ reactRouterLinkComponent: MyLink })

expect(warn).toHaveBeenCalled()

jest.clearAllMocks()

link = doShallow({ to: '/about' })
const link = doShallow({ to: '/about' })

expect(link).toHaveProp('to')
expect(warn).toHaveBeenCalled()
Expand All @@ -56,16 +57,16 @@ describe('ChevronLink', () => {
it('has a chevron icon', () => {
let link = doShallow({ href: 'https://telus.com' })
expect(link).toContainReact(
<span className="rightChevron">
<Box inline spacing="margin" left={2} dangerouslyAddClassName="rightChevron">
<DecorativeIcon symbol="chevron" size={16} />
</span>
</Box>
)

link = doShallow({ href: 'https://telus.com', direction: 'left' })
expect(link).toContainReact(
<span className="leftChevron">
<Box inline spacing="margin" right={2} dangerouslyAddClassName="leftChevron">
<DecorativeIcon symbol="leftChevron" size={16} />
</span>
</Box>
)
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,19 @@

exports[`ChevronLink renders 1`] = `
<a
className="primary"
href={null}
to={null}
class="primary"
href="test.com"
>
Go home
<span
className="rightChevron"
class="leftMargin-2 rightChevron"
>
<DecorativeIcon
size={16}
symbol="chevron"
/>
<i
aria-hidden="true"
class="iconChevron size16"
>
</i>
</span>
</a>
`;
5 changes: 0 additions & 5 deletions src/components/Lists/ListItem.modules.scss

This file was deleted.

12 changes: 7 additions & 5 deletions src/components/Lists/OrderedList/OrderedItem.jsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
import React from 'react'
import PropTypes from 'prop-types'

import safeRest from '../../../utils/safeRest'
import Box from '../../Box/Box'

import styles from '../ListItem.modules.scss'
import safeRest from '../../../utils/safeRest'

const OrderedItem = ({ children, ...rest }) => (
<li {...safeRest(rest)} className={styles.item}>
{children}
<li {...safeRest(rest)}>
<Box spacing="margin" bottom={2}>
{children}
</Box>
</li>
)

OrderedItem.propTypes = {
children: PropTypes.node.isRequired
children: PropTypes.node.isRequired,
}

OrderedItem.displayName = 'OrderedList.Item'
Expand Down
Loading

0 comments on commit 281264b

Please sign in to comment.