Skip to content

Commit

Permalink
Simplify usage without cloneElement to be more straightforward
Browse files Browse the repository at this point in the history
  • Loading branch information
michaeljaltamirano committed Nov 19, 2020
1 parent e403469 commit dc2391d
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 40 deletions.
24 changes: 12 additions & 12 deletions .size-snapshot.json
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
{
"bundle.js": {
"bundled": 824462,
"minified": 754465,
"gzipped": 90210
"bundled": 823765,
"minified": 754362,
"gzipped": 90168
},
"bundle.umd.js": {
"bundled": 834162,
"minified": 730280,
"gzipped": 86356
"bundled": 833441,
"minified": 730202,
"gzipped": 86312
},
"icons/icons/crossIcon.js": {
"bundled": 238,
Expand Down Expand Up @@ -1004,16 +1004,16 @@
}
},
"shared-components/icon/index.js": {
"bundled": 2619,
"minified": 1165,
"gzipped": 559,
"bundled": 1919,
"minified": 1056,
"gzipped": 516,
"treeshaked": {
"rollup": {
"code": 341,
"import_statements": 220
"code": 342,
"import_statements": 221
},
"webpack": {
"code": 2024
"code": 1924
}
}
},
Expand Down
39 changes: 11 additions & 28 deletions src/shared-components/icon/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { useTheme } from 'emotion-theming';

import Style from './style';

type SVGComponent = React.ComponentType<React.SVGProps<SVGSVGElement>>;
export interface IconProps extends React.SVGProps<SVGSVGElement> {
className?: string;
fill?: string;
Expand All @@ -26,28 +27,20 @@ export interface IconProps extends React.SVGProps<SVGSVGElement> {
* **This component should not be used directly**, and so is not included in the `shared-components` export.
*/
export const Icon = ({
children,
className,
displayInline = false,
fill,
height = 16,
IconComponent,
rotate = 0,
width = 16,
...rest
}: IconProps & { children: JSX.Element }) => {
// Many of our SVG have fill="none". When passing fill down as a prop to cloneElement, an
// undefined fill value will overwrite the "none" value. This mechanism prevents unintended regression.
const safeFill = fill ? { fill } : {};

return React.cloneElement(children, {
className,
css: Style.iconStyles({ displayInline, fill, rotate }),
height,
width,
...safeFill,
...rest,
});
};
}: IconProps & { IconComponent: SVGComponent }) => (
<IconComponent
css={Style.iconStyles({ displayInline, fill: rest.fill, rotate })}
height={height}
width={width}
{...rest}
/>
);

Icon.propTypes = {
children: PropTypes.element.isRequired,
Expand All @@ -59,8 +52,6 @@ Icon.propTypes = {
width: PropTypes.oneOfType([PropTypes.number, PropTypes.string]),
};

type SVGComponent = React.ComponentType<React.SVGProps<SVGSVGElement>>;

export const useIcon = (
PrimaryIcon: SVGComponent,
SecondaryIcon: SVGComponent,
Expand All @@ -70,13 +61,5 @@ export const useIcon = (

const ThemeIcon = theme.__type === 'primary' ? PrimaryIcon : SecondaryIcon;

/**
* React.cloneElement not fully compatible with css prop, so we apply it here (with undefined) to have it available in React.cloneElement
* @see: https://github.com/emotion-js/emotion/pull/1985/files#diff-6e9f0af93add92299d604a909f5b4a3c366a28c819127d9b7f33f3694cdfcffcR251
*/
return (
<Icon {...props}>
<ThemeIcon css={undefined} />
</Icon>
);
return <Icon IconComponent={ThemeIcon} {...props} />;
};

0 comments on commit dc2391d

Please sign in to comment.