Skip to content

Commit

Permalink
Use Icon component to unify the way icons are handled
Browse files Browse the repository at this point in the history
  • Loading branch information
gziolo committed Nov 28, 2019
1 parent 1d3caa0 commit 13b6cae
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 22 deletions.
13 changes: 3 additions & 10 deletions packages/components/src/icon-button/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
* External dependencies
*/
import classnames from 'classnames';
import { isArray, isString } from 'lodash';
import { isArray } from 'lodash';

/**
* WordPress dependencies
*/
import { cloneElement, forwardRef } from '@wordpress/element';
import { forwardRef } from '@wordpress/element';

/**
* Internal dependencies
Expand Down Expand Up @@ -49,21 +49,14 @@ function IconButton( props, ref ) {
)
);

let buttonIcon = isString( icon ) ?
<Icon icon={ icon } /> :
icon;
if ( size ) {
buttonIcon = cloneElement( buttonIcon, { size } );
}

let element = (
<Button
aria-label={ label }
{ ...additionalProps }
className={ classes }
ref={ ref }
>
{ buttonIcon }
<Icon icon={ icon } size={ size } />
{ children }
</Button>
);
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/icon-button/stories/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export default { title: 'Components|IconButton', component: IconButton };
export const _default = () => {
const icon = text( 'Icon', 'ellipsis' );
const label = text( 'Label', 'More' );
const size = number( 'Size', 24 );
const size = number( 'Size' );

return (
<IconButton
Expand Down
10 changes: 2 additions & 8 deletions packages/components/src/placeholder/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,11 @@
* External dependencies
*/
import classnames from 'classnames';
import { isString } from 'lodash';

/**
* WordPress dependencies
*/
import { cloneElement } from '@wordpress/element';

/**
* Internal dependencies
*/
import Dashicon from '../dashicon';
import Icon from '../icon';

/**
* Renders a placeholder. Normally used by blocks to render their empty state.
Expand All @@ -32,7 +26,7 @@ function Placeholder( { icon, children, label, instructions, className, notices,
</div>
}
<div className="components-placeholder__label">
{ isString( icon ) ? <Dashicon icon={ icon } /> : cloneElement( icon, { height: 24, width: 24 } ) }
<Icon icon={ icon } />
{ label }
</div>
{ !! instructions && <div className="components-placeholder__instructions">{ instructions }</div> }
Expand Down
6 changes: 3 additions & 3 deletions packages/components/src/placeholder/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,19 @@ describe( 'Placeholder', () => {
expect( placeholderFieldset.exists() ).toBe( true );
} );

it( 'should render a Dashicon in the label section', () => {
it( 'should render an Icon in the label section', () => {
const placeholder = shallow( <Placeholder icon="wordpress" /> );
const placeholderLabel = placeholder.find( '.components-placeholder__label' );

expect( placeholderLabel.exists() ).toBe( true );
expect( placeholderLabel.find( 'Dashicon' ).exists() ).toBe( true );
expect( placeholderLabel.find( 'Icon' ).exists() ).toBe( true );
} );

it( 'should render a label section', () => {
const label = 'WordPress';
const placeholder = shallow( <Placeholder label={ label } /> );
const placeholderLabel = placeholder.find( '.components-placeholder__label' );
const child = placeholderLabel.childAt( 0 );
const child = placeholderLabel.childAt( 1 );

expect( child.text() ).toBe( label );
} );
Expand Down

0 comments on commit 13b6cae

Please sign in to comment.