fix: Input and List Group components not properly forwarding refs

Migrate Input and List Group components to properly forward passed in refs to the underlying component. Also, replace HOC functionality with custom hooks. Relevant to #4012 and #4031.
react-bootstrap · Jul 27, 2019 · 8c0cf4a · 8c0cf4a
1 parent fce0e8b
commit 8c0cf4a
Show file tree

Hide file tree

Showing 3 changed files with 156 additions and 136 deletions.
diff --git a/src/InputGroup.js b/src/InputGroup.js
@@ -4,7 +4,21 @@ import PropTypes from 'prop-types';
 import React from 'react';
 
 import createWithBsPrefix from './utils/createWithBsPrefix';
-import { createBootstrapComponent } from './ThemeProvider';
+import { useBootstrapPrefix } from './ThemeProvider';
+
+const propTypes = {
+  /** @default 'input-group' */
+  bsPrefix: PropTypes.string,
+
+  /**
+   * Control the size of buttons and form elements from the top-level .
+   *
+   * @type {('sm'|'lg')}
+   */
+  size: PropTypes.string,
+
+  as: PropTypes.elementType,
+};
 
 /**
  *
@@ -14,33 +28,23 @@ import { createBootstrapComponent } from './ThemeProvider';
  * @property {InputGroupRadio} Radio
  * @property {InputGroupCheckbox} Checkbox
  */
-class InputGroup extends React.Component {
-  static propTypes = {
-    /** @default 'input-group' */
-    bsPrefix: PropTypes.string.isRequired,
-
-    /**
-     * Control the size of buttons and form elements from the top-level .
-     *
-     * @type {('sm'|'lg')}
-     */
-    size: PropTypes.string,
-
-    as: PropTypes.elementType,
-  };
-
-  render() {
-    const {
+const InputGroup = React.forwardRef(
+  (
+    {
       bsPrefix,
       size,
       className,
       // Need to define the default "as" during prop destructuring to be compatible with styled-components github.com/react-bootstrap/react-bootstrap/issues/3595
       as: Component = 'div',
       ...props
-    } = this.props;
+    },
+    ref,
+  ) => {
+    bsPrefix = useBootstrapPrefix(bsPrefix, 'input-group');
 
     return (
       <Component
+        ref={ref}
         {...props}
         className={classNames(
           className,
@@ -49,8 +53,8 @@ class InputGroup extends React.Component {
         )}
       />
     );
-  }
-}
+  },
+);
 
 const InputGroupAppend = createWithBsPrefix('input-group-append');
 
@@ -72,12 +76,12 @@ const InputGroupRadio = props => (
   </InputGroupText>
 );
 
-const DecoratedInputGroup = createBootstrapComponent(InputGroup, 'input-group');
+InputGroup.propTypes = propTypes;
 
-DecoratedInputGroup.Text = InputGroupText;
-DecoratedInputGroup.Radio = InputGroupRadio;
-DecoratedInputGroup.Checkbox = InputGroupCheckbox;
-DecoratedInputGroup.Append = InputGroupAppend;
-DecoratedInputGroup.Prepend = InputGroupPrepend;
+InputGroup.Text = InputGroupText;
+InputGroup.Radio = InputGroupRadio;
+InputGroup.Checkbox = InputGroupCheckbox;
+InputGroup.Append = InputGroupAppend;
+InputGroup.Prepend = InputGroupPrepend;
 
-export default DecoratedInputGroup;
+export default InputGroup;
diff --git a/src/ListGroup.js b/src/ListGroup.js
@@ -2,59 +2,66 @@ import classNames from 'classnames';
 import React from 'react';
 import PropTypes from 'prop-types';
 
-import { uncontrollable } from 'uncontrollable';
+import { useUncontrolled } from 'uncontrollable';
 
-import { createBootstrapComponent } from './ThemeProvider';
+import { useBootstrapPrefix } from './ThemeProvider';
 import AbstractNav from './AbstractNav';
 import ListGroupItem from './ListGroupItem';
 
-class ListGroup extends React.Component {
-  static propTypes = {
-    /**
-     * @default 'list-group'
-     */
-    bsPrefix: PropTypes.string.isRequired,
-
-    /**
-     * Adds a variant to the list-group
-     *
-     * @type {('flush')}
-     */
-    variant: PropTypes.oneOf(['flush', null]),
-
-    /**
-     * You can use a custom element type for this component.
-     */
-    as: PropTypes.elementType,
-  };
-
-  static defaultProps = {
-    variant: null,
-  };
-
-  render() {
-    const { className, bsPrefix, variant, as = 'div', ...props } = this.props;
-
-    return (
-      <AbstractNav
-        {...props}
-        as={as}
-        className={classNames(
-          className,
-          bsPrefix,
-          variant && `${bsPrefix}-${variant}`,
-        )}
-      />
-    );
-  }
-}
-
-const DecoratedListGroup = uncontrollable(
-  createBootstrapComponent(ListGroup, 'list-group'),
-  {
+const propTypes = {
+  /**
+   * @default 'list-group'
+   */
+  bsPrefix: PropTypes.string,
+
+  /**
+   * Adds a variant to the list-group
+   *
+   * @type {('flush')}
+   */
+  variant: PropTypes.oneOf(['flush', null]),
+
+  /**
+   * You can use a custom element type for this component.
+   */
+  as: PropTypes.elementType,
+};
+
+const defaultProps = {
+  variant: null,
+};
+
+const ListGroup = React.forwardRef((props, ref) => {
+  let {
+    className,
+    bsPrefix,
+    variant,
+    // Need to define the default "as" during prop destructuring to be compatible with styled-components github.com/react-bootstrap/react-bootstrap/issues/3595
+    as = 'div',
+    ...controlledProps
+  } = useUncontrolled(props, {
     activeKey: 'onSelect',
-  },
-);
-DecoratedListGroup.Item = ListGroupItem;
+  });
+
+  bsPrefix = useBootstrapPrefix(bsPrefix, 'list-group');
+
+  return (
+    <AbstractNav
+      ref={ref}
+      {...controlledProps}
+      as={as}
+      className={classNames(
+        className,
+        bsPrefix,
+        variant && `${bsPrefix}-${variant}`,
+      )}
+    />
+  );
+});
+
+ListGroup.propTypes = propTypes;
+ListGroup.defaultProps = defaultProps;
+
+ListGroup.Item = ListGroupItem;
 
-export default DecoratedListGroup;
+export default ListGroup;
diff --git a/src/ListGroupItem.js b/src/ListGroupItem.js
@@ -4,67 +4,58 @@ import PropTypes from 'prop-types';
 
 import AbstractNavItem from './AbstractNavItem';
 import { makeEventKey } from './SelectableContext';
-import { createBootstrapComponent } from './ThemeProvider';
+import { useBootstrapPrefix } from './ThemeProvider';
 
-class ListGroupItem extends React.Component {
-  static propTypes = {
-    /**
-     * @default 'list-group-item'
-     */
-    bsPrefix: PropTypes.string.isRequired,
+const propTypes = {
+  /**
+   * @default 'list-group-item'
+   */
+  bsPrefix: PropTypes.string,
 
-    /**
-     * Sets contextual classes for list item
-     * @type {('primary'|'secondary'|'success'|'danger'|'warning'|'info'|'dark'|'light')}
-     */
-    variant: PropTypes.string,
-    /**
-     * Marks a ListGroupItem as actionable, applying additional hover, active and disabled styles
-     * for links and buttons.
-     */
-    action: PropTypes.bool,
-    /**
-     * Sets list item as active
-     */
-    active: PropTypes.bool,
+  /**
+   * Sets contextual classes for list item
+   * @type {('primary'|'secondary'|'success'|'danger'|'warning'|'info'|'dark'|'light')}
+   */
+  variant: PropTypes.string,
+  /**
+   * Marks a ListGroupItem as actionable, applying additional hover, active and disabled styles
+   * for links and buttons.
+   */
+  action: PropTypes.bool,
+  /**
+   * Sets list item as active
+   */
+  active: PropTypes.bool,
 
-    /**
-     * Sets list item state as disabled
-     */
-    disabled: PropTypes.bool,
+  /**
+   * Sets list item state as disabled
+   */
+  disabled: PropTypes.bool,
 
-    eventKey: PropTypes.string,
+  eventKey: PropTypes.string,
 
-    onClick: PropTypes.func,
+  onClick: PropTypes.func,
 
-    /**
-     * You can use a custom element type for this component. For none `action` items, items render as `li`.
-     * For actions the default is an achor or button element depending on whether a `href` is provided.
-     *
-     * @default {'div' | 'a' | 'button'}
-     */
-    as: PropTypes.elementType,
-  };
+  href: PropTypes.string,
 
-  static defaultProps = {
-    variant: null,
-    active: false,
-    disabled: false,
-  };
+  /**
+   * You can use a custom element type for this component. For none `action` items, items render as `li`.
+   * For actions the default is an achor or button element depending on whether a `href` is provided.
+   *
+   * @default {'div' | 'a' | 'button'}
+   */
+  as: PropTypes.elementType,
+};
 
-  handleClick = event => {
-    const { onClick, disabled } = this.props;
-    if (disabled) {
-      event.preventDefault();
-      event.stopPropagation();
-      return;
-    }
+const defaultProps = {
+  variant: null,
+  active: false,
+  disabled: false,
+};
 
-    if (onClick) onClick(event);
-  };
-
-  render() {
-    const {
+const ListGroupItem = React.forwardRef(
+  (
+    {
       bsPrefix,
       active,
       disabled,
@@ -73,16 +64,31 @@ class ListGroupItem extends React.Component {
       action,
       as,
       eventKey,
+      onClick,
       ...props
-    } = this.props;
+    },
+    ref,
+  ) => {
+    bsPrefix = useBootstrapPrefix(bsPrefix, 'list-group-item');
+
+    const handleClick = event => {
+      if (disabled) {
+        event.preventDefault();
+        event.stopPropagation();
+        return;
+      }
+
+      if (onClick) onClick(event);
+    };
 
     return (
       <AbstractNavItem
+        ref={ref}
         {...props}
         eventKey={makeEventKey(eventKey, props.href)}
         // eslint-disable-next-line
-        as={as || (action ? (props.href ? 'a' : 'button') : 'div')}
-        onClick={this.handleClick}
+      as={as || (action ? (props.href ? 'a' : 'button') : 'div')}
+        onClick={handleClick}
         className={classNames(
           className,
           bsPrefix,
@@ -93,7 +99,10 @@ class ListGroupItem extends React.Component {
         )}
       />
     );
-  }
-}
+  },
+);
+
+ListGroupItem.propTypes = propTypes;
+ListGroupItem.defaultProps = defaultProps;
 
-export default createBootstrapComponent(ListGroupItem, 'list-group-item');
+export default ListGroupItem;