Skip to content
This repository has been archived by the owner on Oct 6, 2020. It is now read-only.

Commit

Permalink
fix(Icon): Prevent passed className from being overwritten (#35)
Browse files Browse the repository at this point in the history
## Summary 
When the `Icon` component was being provided with a `className` prop, this prop was overwriting the default class name generated for the icon, namely `mdi mdi-X`. In the web-app several icons disappeared mysteriously and this was because those component had this structure...
```js
<Icon className="blah" name="message" />
```

...which is explained by this snippet:
```js
<Icon className="bagels" name="message" /> //=> render Icon with className prop

const Icon = createComponent({
  props: () => ({
    className: 'mdi mdi-message', //=> generated by `Icon.getClassName`
  });

// and then inside of `utils#createComponent`
const resolvedProps = {
  ...baseProps(props), //=> className: mdi mdi-message
  ...props, //=> className: overwritten by bagels!!!   
};
```
  • Loading branch information
erikshestopal authored and kylealwyn committed Apr 1, 2019
1 parent b6b0d9b commit ebfca7a
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/Card/Card.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ Card.Header = createComponent({
as: Box,
style: ({ theme }) => css`
padding: 1rem;
border-bottom: 1px solid ${theme.colors.grayLight}};
border-bottom: 1px solid ${theme.colors.grayLight};
`,
});

Expand Down
4 changes: 2 additions & 2 deletions src/Icon/Icon.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import { createComponent } from '../utils';
const Icon = createComponent({
name: 'Icon',
tag: 'i',
props: ({ name, className }) => ({
className: `${Icon.getClassName(name)} ${className || ''}`,
props: ({ name }) => ({
className: Icon.getClassName(name),
}),
style: ({ theme, size, color, disabled }) => {
const colorFromTheme = theme.colors[color];
Expand Down
17 changes: 17 additions & 0 deletions src/Icon/Icon.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import React from 'react';
import { renderWithTheme } from '../../test/utils';
import Icon from './Icon';

describe('<Icon />', () => {
test('renders', () => {
const { asFragment } = renderWithTheme(<Icon name="message" />);

expect(asFragment()).toMatchSnapshot();
});

test('renders with passed class name', () => {
const { asFragment } = renderWithTheme(<Icon className="bagels" name="message" />);

expect(asFragment()).toMatchSnapshot();
});
});
29 changes: 29 additions & 0 deletions src/Icon/__snapshots__/Icon.spec.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`<Icon /> renders 1`] = `
<DocumentFragment>
.c0 {
color: inherit;
font-size: inherit;
}
<i
class="mdi mdi-message re-icon c0"
name="message"
/>
</DocumentFragment>
`;

exports[`<Icon /> renders with passed class name 1`] = `
<DocumentFragment>
.c0 {
color: inherit;
font-size: inherit;
}
<i
class="bagels mdi mdi-message re-icon c0"
name="message"
/>
</DocumentFragment>
`;
15 changes: 9 additions & 6 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,23 @@ export const getComponentVariant = (theme, componentName, variant) => {

export const getComponentStyle = componentName => themeGet(`components.${componentName}.style`);

export const getComponentClassName = ({ className, theme: { classPrefix }, variant }, name) =>
const getComponentClassName = ({ className, theme: { classPrefix }, variant }, name) =>
`${className || ''} ${classPrefix}-${name} ${variant ? `${classPrefix}-${name}-${variant}` : ''}`.trim();

export const createComponent = ({ name, tag = 'div', as, style, props: baseProps = () => ({}) }) => {
export const createComponent = ({ name, tag = 'div', as, style, props: getBaseProps = () => ({}) }) => {
const component = as ? styled(as) : styled[tag];

return component.attrs(props => {
const resolvedProps = {
...baseProps(props),
const baseProps = getBaseProps(props);
const finalProps = {
...baseProps,
...props,
className: baseProps.className || props.className,
};

return {
...resolvedProps,
className: getComponentClassName(resolvedProps, kebabCase(name)),
...finalProps,
className: getComponentClassName(finalProps, kebabCase(name)),
};
})`
${style}
Expand Down

0 comments on commit ebfca7a

Please sign in to comment.