Make IconButton able to be referenced. (#14163)

* Make IconButton able to get referenced. * Components: Forward IconButton ref as stateless function * Components: Restore IconButton aria-pressed prop pass-through
WordPress · Mar 6, 2019 · e4fcb0f · e4fcb0f
1 parent 829927a
commit e4fcb0f
Show file tree

Hide file tree

Showing 14 changed files with 96 additions and 71 deletions.
diff --git a/packages/components/src/form-file-upload/test/index.js b/packages/components/src/form-file-upload/test/index.js
@@ -22,7 +22,7 @@ describe( 'InserterMenu', () => {
 			</FormFileUpload>
 		);
 
-		const iconButton = wrapper.find( 'IconButton' );
+		const iconButton = wrapper.find( 'ForwardRef(IconButton)' );
 		const input = wrapper.find( 'input' );
 		expect( iconButton.prop( 'children' ) ).toBe( 'My Upload Button' );
 		expect( input.prop( 'style' ).display ).toBe( 'none' );

diff --git a/packages/components/src/icon-button/index.js b/packages/components/src/icon-button/index.js
@@ -7,7 +7,7 @@ import { isArray, isString } from 'lodash';
 /**
  * WordPress dependencies
  */
-import { Component } from '@wordpress/element';
+import { forwardRef } from '@wordpress/element';
 
 /**
  * Internal dependencies
@@ -16,50 +16,60 @@ import Tooltip from '../tooltip';
 import Button from '../button';
 import Dashicon from '../dashicon';
 
-// This is intentionally a Component class, not a function component because it
-// is common to apply a ref to the button element (only supported in class)
-class IconButton extends Component {
-	render() {
-		const { icon, children, label, className, tooltip, shortcut, labelPosition, ...additionalProps } = this.props;
-		const { 'aria-pressed': ariaPressed } = this.props;
-		const classes = classnames( 'components-icon-button', className, {
-			'has-text': children,
-		} );
-		const tooltipText = tooltip || label;
+function IconButton( props, ref ) {
+	const {
+		icon,
+		children,
+		label,
+		className,
+		tooltip,
+		shortcut,
+		labelPosition,
+		...additionalProps
+	} = props;
+	const { 'aria-pressed': ariaPressed } = additionalProps;
+	const classes = classnames( 'components-icon-button', className, {
+		'has-text': children,
+	} );
+	const tooltipText = tooltip || label;
 
-		// Should show the tooltip if...
-		const showTooltip = ! additionalProps.disabled && (
-			// an explicit tooltip is passed or...
-			tooltip ||
-			// there's a shortcut or...
-			shortcut ||
-			(
-				// there's a label and...
-				!! label &&
-				// the children are empty and...
-				( ! children || ( isArray( children ) && ! children.length ) ) &&
-				// the tooltip is not explicitly disabled.
-				false !== tooltip
-			)
-		);
-
-		let element = (
-			<Button aria-label={ label } { ...additionalProps } className={ classes }>
-				{ isString( icon ) ? <Dashicon icon={ icon } ariaPressed={ ariaPressed } /> : icon }
-				{ children }
-			</Button>
-		);
+	// Should show the tooltip if...
+	const showTooltip = ! additionalProps.disabled && (
+		// an explicit tooltip is passed or...
+		tooltip ||
+		// there's a shortcut or...
+		shortcut ||
+		(
+			// there's a label and...
+			!! label &&
+			// the children are empty and...
+			( ! children || ( isArray( children ) && ! children.length ) ) &&
+			// the tooltip is not explicitly disabled.
+			false !== tooltip
+		)
+	);
 
-		if ( showTooltip ) {
-			element = (
-				<Tooltip text={ tooltipText } shortcut={ shortcut } position={ labelPosition }>
-					{ element }
-				</Tooltip>
-			);
-		}
+	let element = (
+		<Button
+			aria-label={ label }
+			{ ...additionalProps }
+			className={ classes }
+			ref={ ref }
+		>
+			{ isString( icon ) ? <Dashicon icon={ icon } ariaPressed={ ariaPressed } /> : icon }
+			{ children }
+		</Button>
+	);
 
-		return element;
+	if ( showTooltip ) {
+		element = (
+			<Tooltip text={ tooltipText } shortcut={ shortcut } position={ labelPosition }>
+				{ element }
+			</Tooltip>
+		);
 	}
+
+	return element;
 }
 
-export default IconButton;
+export default forwardRef( IconButton );
diff --git a/packages/components/src/icon-button/test/index.js b/packages/components/src/icon-button/test/index.js
@@ -2,6 +2,12 @@
  * External dependencies
  */
 import { shallow } from 'enzyme';
+import TestUtils from 'react-dom/test-utils';
+
+/**
+ * WordPress dependencies
+ */
+import { createRef } from '@wordpress/element';
 
 /**
  * Internal dependencies
@@ -49,9 +55,11 @@ describe( 'IconButton', () => {
 			expect( iconButton.hasClass( 'test' ) ).toBe( true );
 		} );
 
-		it( 'should add an additonal prop to the IconButton element', () => {
-			const iconButton = shallow( <IconButton test="test" /> );
-			expect( iconButton.props().test ).toBe( 'test' );
+		it( 'should pass additional props to the underlying button', () => {
+			const iconButton = shallow( <IconButton disabled aria-pressed="true" /> );
+
+			expect( iconButton.find( 'ForwardRef(Button)' ).prop( 'aria-pressed' ) ).toBe( 'true' );
+			expect( iconButton.find( 'ForwardRef(Button)' ).prop( 'disabled' ) ).toBe( true );
 		} );
 
 		it( 'should allow custom tooltip text', () => {
@@ -72,5 +80,12 @@ describe( 'IconButton', () => {
 			expect( iconButton.name() ).toBe( 'Tooltip' );
 			expect( iconButton.prop( 'text' ) ).toBe( 'WordPress' );
 		} );
+
+		it( 'forwards ref', () => {
+			const ref = createRef();
+
+			TestUtils.renderIntoDocument( <IconButton ref={ ref } /> );
+			expect( ref.current.type ).toBe( 'button' );
+		} );
 	} );
 } );
diff --git a/packages/components/src/menu-item/test/__snapshots__/index.js.snap b/packages/components/src/menu-item/test/__snapshots__/index.js.snap
@@ -1,7 +1,7 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`MenuItem should match snapshot when all props provided 1`] = `
-<IconButton
+<ForwardRef(IconButton)
   aria-checked={true}
   className="components-menu-item__button my-class has-icon"
   icon="wordpress"
@@ -13,7 +13,7 @@ exports[`MenuItem should match snapshot when all props provided 1`] = `
     className="components-menu-item__shortcut"
     shortcut="mod+shift+alt+w"
   />
-</IconButton>
+</ForwardRef(IconButton)>
 `;
 
 exports[`MenuItem should match snapshot when info is provided 1`] = `
@@ -40,7 +40,7 @@ exports[`MenuItem should match snapshot when info is provided 1`] = `
 `;
 
 exports[`MenuItem should match snapshot when isSelected and role are optionally provided 1`] = `
-<IconButton
+<ForwardRef(IconButton)
   className="components-menu-item__button my-class has-icon"
   icon="wordpress"
   onClick={[Function]}
@@ -51,7 +51,7 @@ exports[`MenuItem should match snapshot when isSelected and role are optionally
     className="components-menu-item__shortcut"
     shortcut="mod+shift+alt+w"
   />
-</IconButton>
+</ForwardRef(IconButton)>
 `;
 
 exports[`MenuItem should match snapshot when only label provided 1`] = `

diff --git a/packages/components/src/notice/test/__snapshots__/index.js.snap b/packages/components/src/notice/test/__snapshots__/index.js.snap
@@ -17,7 +17,7 @@ exports[`Notice should match snapshot 1`] = `
       View
     </ForwardRef(Button)>
   </div>
-  <IconButton
+  <ForwardRef(IconButton)
     className="components-notice__dismiss"
     icon="no"
     label="Dismiss this notice"

diff --git a/packages/edit-post/src/components/header/more-menu/test/__snapshots__/index.js.snap b/packages/edit-post/src/components/header/more-menu/test/__snapshots__/index.js.snap
@@ -12,7 +12,7 @@ exports[`MoreMenu should match snapshot 1`] = `
     <div
       className="edit-post-more-menu"
     >
-      <IconButton
+      <ForwardRef(IconButton)
         aria-expanded={false}
         icon="ellipsis"
         label="Show more tools & options"
@@ -81,7 +81,7 @@ exports[`MoreMenu should match snapshot 1`] = `
             </button>
           </ForwardRef(Button)>
         </Tooltip>
-      </IconButton>
+      </ForwardRef(IconButton)>
     </div>
   </Dropdown>
 </MoreMenu>

diff --git a/packages/editor/src/components/block-mover/test/index.js b/packages/editor/src/components/block-mover/test/index.js
@@ -37,9 +37,9 @@ describe( 'BlockMover', () => {
 			const moveDown = blockMover.childAt( 2 );
 			const moveUpDesc = blockMover.childAt( 3 );
 			const moveDownDesc = blockMover.childAt( 4 );
-			expect( moveUp.type().name ).toBe( 'IconButton' );
+			expect( moveUp.name() ).toBe( 'ForwardRef(IconButton)' );
 			expect( drag.type().name ).toBe( 'IconDragHandle' );
-			expect( moveDown.type().name ).toBe( 'IconButton' );
+			expect( moveDown.name() ).toBe( 'ForwardRef(IconButton)' );
 			expect( moveUp.props() ).toMatchObject( {
 				className: 'editor-block-mover__control',
 				onClick: undefined,

diff --git a/packages/editor/src/components/block-switcher/test/__snapshots__/index.js.snap b/packages/editor/src/components/block-switcher/test/__snapshots__/index.js.snap
@@ -2,7 +2,7 @@
 
 exports[`BlockSwitcher should render disabled block switcher with multi block of different types when no transforms 1`] = `
 <Toolbar>
-  <IconButton
+  <ForwardRef(IconButton)
     className="editor-block-switcher__no-switcher-icon"
     disabled={true}
     label="Block icon"
@@ -11,7 +11,7 @@ exports[`BlockSwitcher should render disabled block switcher with multi block of
       icon="layout"
       showColors={true}
     />
-  </IconButton>
+  </ForwardRef(IconButton)>
 </Toolbar>
 `;
 

diff --git a/packages/editor/src/components/block-switcher/test/index.js b/packages/editor/src/components/block-switcher/test/index.js
@@ -171,7 +171,7 @@ describe( 'BlockSwitcher', () => {
 
 			test( 'should simulate a keydown event, which should call onToggle and open transform toggle.', () => {
 				const toggleClosed = shallow( getDropdown().props().renderToggle( { onToggle: onToggleStub, isOpen: false } ) );
-				const iconButtonClosed = toggleClosed.find( 'IconButton' );
+				const iconButtonClosed = toggleClosed.find( 'ForwardRef(IconButton)' );
 
 				iconButtonClosed.simulate( 'keydown', mockKeyDown );
 
@@ -180,7 +180,7 @@ describe( 'BlockSwitcher', () => {
 
 			test( 'should simulate a click event, which should call onToggle.', () => {
 				const toggleOpen = shallow( getDropdown().props().renderToggle( { onToggle: onToggleStub, isOpen: true } ) );
-				const iconButtonOpen = toggleOpen.find( 'IconButton' );
+				const iconButtonOpen = toggleOpen.find( 'ForwardRef(IconButton)' );
 
 				iconButtonOpen.simulate( 'keydown', mockKeyDown );
 

diff --git a/packages/editor/src/components/post-publish-panel/test/__snapshots__/index.js.snap b/packages/editor/src/components/post-publish-panel/test/__snapshots__/index.js.snap
@@ -12,7 +12,7 @@ exports[`PostPublishPanel should render the post-publish panel if the post is pu
     >
       Published
     </div>
-    <IconButton
+    <ForwardRef(IconButton)
       aria-expanded={true}
       icon="no-alt"
       label="Close panel"
@@ -47,7 +47,7 @@ exports[`PostPublishPanel should render the post-publish panel if the post is sc
     >
       Scheduled
     </div>
-    <IconButton
+    <ForwardRef(IconButton)
       aria-expanded={true}
       icon="no-alt"
       label="Close panel"
@@ -88,7 +88,7 @@ exports[`PostPublishPanel should render the pre-publish panel if post status is
         className="editor-post-publish-panel__spacer"
       />
     </div>
-    <IconButton
+    <ForwardRef(IconButton)
       aria-expanded={true}
       icon="no-alt"
       label="Close panel"
@@ -127,7 +127,7 @@ exports[`PostPublishPanel should render the pre-publish panel if the post is not
         className="editor-post-publish-panel__spacer"
       />
     </div>
-    <IconButton
+    <ForwardRef(IconButton)
       aria-expanded={true}
       icon="no-alt"
       label="Close panel"
@@ -166,7 +166,7 @@ exports[`PostPublishPanel should render the spinner if the post is being saved 1
         className="editor-post-publish-panel__spacer"
       />
     </div>
-    <IconButton
+    <ForwardRef(IconButton)
       aria-expanded={true}
       icon="no-alt"
       label="Close panel"

diff --git a/packages/editor/src/components/url-input/test/button.js b/packages/editor/src/components/url-input/test/button.js
@@ -12,19 +12,19 @@ import URLInput from '../';
 import URLInputButton from '../button';
 
 describe( 'URLInputButton', () => {
-	const clickEditLink = ( wrapper ) => wrapper.find( 'IconButton.components-toolbar__control' ).simulate( 'click' );
+	const clickEditLink = ( wrapper ) => wrapper.find( 'ForwardRef(IconButton).components-toolbar__control' ).simulate( 'click' );
 
 	it( 'should have a valid class name in the wrapper tag', () => {
 		const wrapper = shallow( <URLInputButton /> );
 		expect( wrapper.hasClass( 'editor-url-input__button' ) ).toBe( true );
 	} );
 	it( 'should not have is-active class when url prop not defined', () => {
 		const wrapper = shallow( <URLInputButton /> );
-		expect( wrapper.find( 'IconButton' ).hasClass( 'is-active' ) ).toBe( false );
+		expect( wrapper.find( 'ForwardRef(IconButton)' ).hasClass( 'is-active' ) ).toBe( false );
 	} );
 	it( 'should have is-active class name if url prop defined', () => {
 		const wrapper = shallow( <URLInputButton url="https://example.com" /> );
-		expect( wrapper.find( 'IconButton' ).hasClass( 'is-active' ) ).toBe( true );
+		expect( wrapper.find( 'ForwardRef(IconButton)' ).hasClass( 'is-active' ) ).toBe( true );
 	} );
 	it( 'should have hidden form by default', () => {
 		const wrapper = shallow( <URLInputButton /> );

diff --git a/packages/editor/src/components/url-popover/test/__snapshots__/index.js.snap b/packages/editor/src/components/url-popover/test/__snapshots__/index.js.snap
@@ -13,7 +13,7 @@ exports[`URLPopover matches the snapshot in its default state 1`] = `
     <div>
       Editor
     </div>
-    <IconButton
+    <ForwardRef(IconButton)
       aria-expanded={false}
       className="editor-url-popover__settings-toggle"
       icon="arrow-down-alt2"
@@ -37,7 +37,7 @@ exports[`URLPopover matches the snapshot when the settings are toggled open 1`]
     <div>
       Editor
     </div>
-    <IconButton
+    <ForwardRef(IconButton)
       aria-expanded={true}
       className="editor-url-popover__settings-toggle"
       icon="arrow-down-alt2"

diff --git a/packages/nux/src/components/dot-tip/test/__snapshots__/index.js.snap b/packages/nux/src/components/dot-tip/test/__snapshots__/index.js.snap
@@ -21,7 +21,7 @@ exports[`DotTip should render correctly 1`] = `
       Got it
     </ForwardRef(Button)>
   </p>
-  <IconButton
+  <ForwardRef(IconButton)
     className="nux-dot-tip__disable"
     icon="no-alt"
     label="Disable tips"

diff --git a/packages/nux/src/components/dot-tip/test/index.js b/packages/nux/src/components/dot-tip/test/index.js
@@ -46,7 +46,7 @@ describe( 'DotTip', () => {
 				It looks like you’re writing a letter. Would you like help?
 			</DotTip>
 		);
-		wrapper.find( 'IconButton[label="Disable tips"]' ).first().simulate( 'click' );
+		wrapper.find( 'ForwardRef(IconButton)[label="Disable tips"]' ).first().simulate( 'click' );
 		expect( onDisable ).toHaveBeenCalled();
 	} );
 } );