Skip to content

Commit

Permalink
feat(icon): Add fixes to render icons using new css classes
Browse files Browse the repository at this point in the history
  • Loading branch information
lzcabrera committed Sep 21, 2017
1 parent 9a9f0c8 commit a355ae8
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 21 deletions.
10 changes: 6 additions & 4 deletions src/components/Icon/Icon.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React from 'react'
import PropTypes from 'prop-types'

import { deprecate } from '../../warn'
import capitalize from './capitalize'

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

Expand All @@ -22,11 +23,12 @@ const Icon = ({ glyph, variant, label, fixedWidth, size, className, children, ..
deprecate('Icon', '\'fixedWidth\' prop is deprecated.')
}

const classes = `${styles.icon} ${styles[`iconCore${glyph.charAt(0).toUpperCase()}${glyph.slice(1)}`]}`
const classes = `${styles.icon}`
+ ` ${styles[`iconCore${capitalize(glyph)}`]}`
+ ` ${variant ? styles[variant] : ''}`
+ `${fixedWidth ? ` ${styles.fw}` : ''}`
+ `${size ? ` ${styles[size]}` : ''}`
+ `${className ? ` ${className}` : ''}`
+ ` ${fixedWidth ? `${styles.fw}` : ''}`
+ ` ${size ? `${styles[size]}` : ''}`
+ ` ${className ? `${className}` : ''}`

return (
<i
Expand Down
32 changes: 16 additions & 16 deletions src/components/Icon/Icon.modules.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,21 @@

// Unicode points of each icon's location within the font files.
$core-icon-codepoints: (
caretDown: "\f105",
caretUp: "\f106",
checkmark: "\f101",
chevron: "\f107",
leftChevron: "\f107",
exclamationPointCircle: "\f103",
expander: "\f113",
hamburger: "\f112",
incomplete: "\f104",
location: "\f110",
minus: "\f109",
plus: "\f108",
questionMarkCircle: "\f102",
spyglass: "\f111",
times: "\f104"
CaretDown: "\f105",
CaretUp: "\f106",
Checkmark: "\f101",
Chevron: "\f107",
LeftChevron: "\f107",
ExclamationPointCircle: "\f103",
Expander: "\f113",
Hamburger: "\f112",
Incomplete: "\f104",
Location: "\f110",
Minus: "\f109",
Plus: "\f108",
QuestionMarkCircle: "\f102",
Spyglass: "\f111",
Times: "\f104"
);

.icon {
Expand Down Expand Up @@ -65,7 +65,7 @@ $core-icon-codepoints: (
}

@each $name, $codepoint in $core-icon-codepoints {
.iconCore#{to-upper-case($name)}::before {
.iconCore#{$name}::before {
// composes: icon; TODO: can't use composes inside sass @each and
// composes doesn't quite make sense on the color modifiers
content: $codepoint;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
exports[`<Icon /> renders 1`] = `
<i
aria-hidden="true"
className="icon iconCoreCheckmark inherit medium"
className="icon iconCoreCheckmark inherit medium "
/>
`;
9 changes: 9 additions & 0 deletions src/components/Icon/__tests__/capitalize.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import capitalize from '../capitalize'

describe.only('capitalize', () => {
it('capitalizes a string', () => {
const string = capitalize('test')

expect(string).toBe('Test')
})
})
5 changes: 5 additions & 0 deletions src/components/Icon/capitalize.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const capitalize = (string) => {
return `${string.charAt(0).toUpperCase()}${string.slice(1)}`
}

export default capitalize

This comment has been minimized.

Copy link
@ryanoglesby08

ryanoglesby08 Sep 21, 2017

Contributor

Nice! :)

Small suggestion: Get rid of the curly braces and the return statement because its only a single expression.

I made pretty much the same exact function in the WithSpacing component. Please get rid of the duplication and put the "capitalize" in a place that can be shared by both components.

This comment has been minimized.

Copy link
@lzcabrera

lzcabrera Sep 21, 2017

Author Member

I had already created a capitalize.js file. I moved it to src/ and now both WithSpacing and Icon are sharing it.

0 comments on commit a355ae8

Please sign in to comment.