feat: add typings for <Button> (#3080)

* feat: add typings for <Button>

* chore: bump @types/react and @types/react-dom

* feat: re-export bootstrap helpers as ComponentWithAsProp, BsPropsWithAs

package.json

@@ -123,8 +123,8 @@
     "@testing-library/react-hooks": "^8.0.1",
     "@testing-library/user-event": "^13.5.0",
     "@types/jest": "^29.5.10",
-    "@types/react": "17.0.0",
-    "@types/react-dom": "17.0.11",
+    "@types/react": "^17.0.80",
+    "@types/react-dom": "^17.0.11",
     "@types/react-responsive": "^8.0.8",
     "@types/react-table": "^7.7.19",
     "@types/react-test-renderer": "^18.0.0",

src/Button/Button.test.jsx → src/Button/Button.test.tsx

@@ -30,7 +30,7 @@ describe('<Button />', () => {
 
     it('renders with props iconAfter and size', () => {
       const tree = renderer.create((
-        <Button iconAfter={Close} size="md">Button</Button>
+        <Button iconAfter={Close} size="sm">Button</Button>
       )).toJSON();
       expect(tree).toMatchSnapshot();
     });
@@ -94,9 +94,21 @@ describe('<Button />', () => {
       });
 
       test('test button as hyperlink', () => {
-        render(<Button as={Hyperlink} destination="https://www.poop.com/💩">Button</Button>);
+        // eslint-disable-next-line @typescript-eslint/no-unused-vars
+        const ref = (_current: HTMLAnchorElement) => {}; // Check typing of a ref - should not show type errors.
+        render(<Button as={Hyperlink} ref={ref} destination="https://www.poop.com/💩">Button</Button>);
         expect(screen.getByRole('link').getAttribute('href')).toEqual('https://www.poop.com/💩');
       });
     });
+
+    test('with size="inline"', () => {
+      const tree = renderer.create((
+        <p>
+          <span className="mr-1">2 items selected.</span>
+          <Button variant="link" size="inline">Clear</Button>
+        </p>
+      )).toJSON();
+      expect(tree).toMatchSnapshot();
+    });
   });
 });

src/Button/__snapshots__/Button.test.jsx.snap → src/Button/__snapshots__/Button.test.tsx.snap

@@ -55,13 +55,13 @@ exports[`<Button /> correct rendering renders with props iconAfter 1`] = `
 
 exports[`<Button /> correct rendering renders with props iconAfter and size 1`] = `
 <button
-  className="btn btn-primary btn-md"
+  className="btn btn-primary btn-sm"
   disabled={false}
   type="button"
 >
   Button
   <span
-    className="pgn__icon pgn__icon__md btn-icon-after"
+    className="pgn__icon pgn__icon__sm btn-icon-after"
   >
     <svg
       aria-hidden={true}
@@ -197,3 +197,20 @@ exports[`<Button /> correct rendering renders without props 1`] = `
   Button
 </button>
 `;
+
+exports[`<Button /> correct rendering with size="inline" 1`] = `
+<p>
+  <span
+    className="mr-1"
+  >
+    2 items selected.
+  </span>
+  <button
+    className="btn btn-link btn-inline"
+    disabled={false}
+    type="button"
+  >
+    Clear
+  </button>
+</p>
+`;

src/Button/index.jsx → src/Button/index.tsx

@@ -1,31 +1,55 @@
 import React from 'react';
-import PropTypes from 'prop-types';
+import PropTypes, { type Requireable } from 'prop-types';
 import classNames from 'classnames';
-import BaseButton from 'react-bootstrap/Button';
-import BaseButtonGroup from 'react-bootstrap/ButtonGroup';
-import BaseButtonToolbar from 'react-bootstrap/ButtonToolbar';
+import BaseButton, { type ButtonProps as BaseButtonProps } from 'react-bootstrap/Button';
+import BaseButtonGroup, { type ButtonGroupProps as BaseButtonGroupProps } from 'react-bootstrap/ButtonGroup';
+import BaseButtonToolbar, { type ButtonToolbarProps } from 'react-bootstrap/ButtonToolbar';
+import type { ComponentWithAsProp } from '../utils/types/bootstrap';
 
 import Icon from '../Icon';
 
-const Button = React.forwardRef(({
+interface ButtonProps extends Omit<BaseButtonProps, 'size'> {
+  /**
+   * An icon component to render. Example:
+   * ```
+   * import { Close } from '@openedx/paragon/icons';
+   * <Button iconBefore={Close}>Close</Button>
+   * ```
+   */
+  iconBefore?: React.ComponentType;
+  /**
+   * An icon component to render. Example:
+   * ```
+   * import { Close } from '@openedx/paragon/icons';
+   * <Button iconAfter={Close}>Close</Button>
+   * ```
+   */
+  iconAfter?: React.ComponentType;
+  size?: 'sm' | 'md' | 'lg' | 'inline';
+}
+
+type ButtonType = ComponentWithAsProp<'button', ButtonProps> & { Deprecated?: any };
+
+const Button: ButtonType = React.forwardRef<HTMLButtonElement, ButtonProps>(({
   children,
   iconAfter,
   iconBefore,
+  size,
   ...props
 }, ref) => (
   <BaseButton
+    size={size as 'sm' | 'lg' | undefined} // Bootstrap's <Button> types do not allow 'md' or 'inline', but we do.
     {...props}
     className={classNames(props.className)}
     ref={ref}
   >
-    {iconBefore && <Icon className="btn-icon-before" size={props.size} src={iconBefore} />}
+    {iconBefore && <Icon className="btn-icon-before" size={size} src={iconBefore} />}
     {children}
-    {iconAfter && <Icon className="btn-icon-after" size={props.size} src={iconAfter} />}
+    {iconAfter && <Icon className="btn-icon-after" size={size} src={iconAfter} />}
   </BaseButton>
 ));
 
 Button.propTypes = {
-  ...Button.propTypes,
   /** Specifies class name to apply to the button */
   className: PropTypes.string,
   /** Disables the Button, preventing mouse events, even if the underlying component is an `<a>` element */
@@ -51,10 +75,13 @@ Button.propTypes = {
   variant: PropTypes.string,
   /** An icon component to render.
   * Example import of a Paragon icon component: `import { Check } from '@openedx/paragon/icons';` */
-  iconBefore: PropTypes.oneOfType([PropTypes.elementType, PropTypes.node]),
+  iconBefore: PropTypes.elementType as Requireable<React.ComponentType>,
   /** An icon component to render.
   * Example import of a Paragon icon component: `import { Check } from '@openedx/paragon/icons';` */
-  iconAfter: PropTypes.oneOfType([PropTypes.elementType, PropTypes.node]),
+  iconAfter: PropTypes.elementType as Requireable<React.ComponentType>,
+  // The 'as' type casting above is required for TypeScript checking, because the 'PropTypes.elementType' type normally
+  // allows strings as a value (for use cases like 'div') but we don't support that for <Icon />/iconBefore/iconAfter.
+  // The React TypeScript type definitions are more specific (React.ComponentType vs React.ElementType).
 };
 
 Button.defaultProps = {
@@ -66,20 +93,29 @@ Button.defaultProps = {
   disabled: false,
 };
 
-function ButtonGroup(props) {
-  return <BaseButtonGroup {...props} />;
-}
-function ButtonToolbar(props) {
-  return <BaseButtonToolbar {...props} />;
+// We could just re-export 'ButtonGroup' and 'ButtonToolbar', but we currently
+// override them to add propTypes validation at runtime, since most Paragon
+// consumers aren't using TypeScript yet. We also force ButtonGroup's 'size'
+// prop to accept our custom values of 'md' and 'inline' which are used in
+// Paragon but not used in the base Bootstrap classes.
+
+interface ButtonGroupProps extends Omit<BaseButtonGroupProps, 'size'> {
+  size?: 'sm' | 'md' | 'lg' | 'inline';
 }
 
+const ButtonGroup: ComponentWithAsProp<'div', ButtonGroupProps> = (
+  React.forwardRef<HTMLButtonElement, ButtonGroupProps>(({ size, ...props }, ref) => (
+    <BaseButtonGroup size={size as 'sm' | 'lg'} {...props} ref={ref} />
+  ))
+);
+
 ButtonGroup.propTypes = {
   /** Specifies element type for this component. */
   as: PropTypes.elementType,
   /** An ARIA role describing the button group. */
   role: PropTypes.string,
   /** Specifies the size for all Buttons in the group. */
-  size: PropTypes.oneOf(['sm', 'md', 'lg']),
+  size: PropTypes.oneOf(['sm', 'md', 'lg', 'inline']),
   /** Display as a button toggle group. */
   toggle: PropTypes.bool,
   /** Specifies if the set of Buttons should appear vertically stacked. */
@@ -97,6 +133,12 @@ ButtonGroup.defaultProps = {
   size: 'md',
 };
 
+const ButtonToolbar: ComponentWithAsProp<'div', ButtonToolbarProps> = (
+  React.forwardRef<HTMLButtonElement, ButtonToolbarProps>((props, ref) => (
+    <BaseButtonToolbar {...props} ref={ref} />
+  ))
+);
+
 ButtonToolbar.propTypes = {
   /** An ARIA role describing the button group. */
   role: PropTypes.string,

src/Chip/ChipIcon.tsx

@@ -8,7 +8,7 @@ import { STYLE_VARIANTS } from './constants';
 
 export interface ChipIconProps {
   className: string,
-  src: React.ReactElement | Function,
+  src: React.ComponentType,
   onClick?: KeyboardEventHandler & MouseEventHandler,
   alt?: string,
   variant: string,

src/Chip/index.tsx

@@ -1,5 +1,5 @@
 import React, { ForwardedRef, KeyboardEventHandler, MouseEventHandler } from 'react';
-import PropTypes from 'prop-types';
+import PropTypes, { type Requireable } from 'prop-types';
 import classNames from 'classnames';
 // @ts-ignore
 import { requiredWhen } from '../utils/propTypes';
@@ -15,9 +15,9 @@ export interface IChip {
   onClick?: KeyboardEventHandler & MouseEventHandler,
   className?: string,
   variant?: string,
-  iconBefore?: React.ReactElement | Function,
+  iconBefore?: React.ComponentType,
   iconBeforeAlt?: string,
-  iconAfter?: React.ReactElement | Function,
+  iconAfter?: React.ComponentType,
   iconAfterAlt?: string,
   onIconBeforeClick?: KeyboardEventHandler & MouseEventHandler,
   onIconAfterClick?: KeyboardEventHandler & MouseEventHandler,
@@ -111,7 +111,7 @@ Chip.propTypes = {
    *
    * `import { Check } from '@openedx/paragon/icons';`
    */
-  iconBefore: PropTypes.oneOfType([PropTypes.element, PropTypes.func]),
+  iconBefore: PropTypes.elementType as Requireable<React.ComponentType>,
   /** Specifies icon alt text. */
   iconBeforeAlt: requiredWhen(PropTypes.string, ['iconBefore', 'onIconBeforeClick']),
   /** A click handler for the `Chip` icon before. */
@@ -122,7 +122,7 @@ Chip.propTypes = {
    *
    * `import { Check } from '@openedx/paragon/icons';`
    */
-  iconAfter: PropTypes.oneOfType([PropTypes.element, PropTypes.func]),
+  iconAfter: PropTypes.elementType as Requireable<React.ComponentType>,
   /** Specifies icon alt text. */
   iconAfterAlt: requiredWhen(PropTypes.string, ['iconAfter', 'onIconAfterClick']),
   /** A click handler for the `Chip` icon after. */

src/Icon/index.d.ts

@@ -1,13 +1,15 @@
 import React from 'react';
 
 export interface IconProps extends React.ComponentPropsWithoutRef<'span'> {
-  src?: React.ReactElement | Function;
+  // Note: React.ComponentType is what we want here. React.ElementType would allow some element type strings like "div",
+  // but we only want to allow components like 'Add' (a specific icon component function/class)
+  src?: React.ComponentType;
   svgAttrs?: {
     'aria-label'?: string;
     'aria-labelledby'?: string;
   };
   id?: string | null;
-  size?: 'xs' | 'sm' | 'md' | 'lg';
+  size?: 'xs' | 'sm' | 'md' | 'lg' | 'inline';
   className?: string | string[];
   hidden?: boolean;
   screenReaderText?: React.ReactNode;

src/Icon/index.jsx

@@ -74,7 +74,7 @@ Icon.propTypes = {
    * An icon component to render.
    * Example import of a Paragon icon component: `import { Check } from '@openedx/paragon/icons';`
    */
-  src: PropTypes.oneOfType([PropTypes.element, PropTypes.elementType]),
+  src: PropTypes.elementType,
   /** HTML element attributes to pass through to the underlying svg element */
   svgAttrs: PropTypes.shape({
     'aria-label': PropTypes.string,

src/index.d.ts

@@ -5,6 +5,7 @@
 // Things that have types
 // // // // // // // // // // // // // // // // // // // // // // // // // // //
 export { default as Bubble } from './Bubble';
+export { default as Button, ButtonGroup, ButtonToolbar } from './Button';
 export { default as Chip, CHIP_PGN_CLASS } from './Chip';
 export { default as ChipCarousel } from './ChipCarousel';
 export { default as Hyperlink, HYPER_LINK_EXTERNAL_LINK_ALT_TEXT, HYPER_LINK_EXTERNAL_LINK_TITLE } from './Hyperlink';
@@ -21,7 +22,6 @@ export const Avatar: any; // from './Avatar';
 export const AvatarButton: any; // from './AvatarButton';
 export const Badge: any; // from './Badge';
 export const Breadcrumb: any; // from './Breadcrumb';
-export const Button: any, ButtonGroup: any, ButtonToolbar: any; // from './Button';
 export const
   Card: any,
   CardColumns: any,

src/index.js

@@ -1,10 +1,11 @@
-// To keep this file in sync with the .d.ts file, it's in the same order
-// and each line number is the same
+// Keep this file in sync with the .d.ts file (manually). It's in the same order
+// and each line number is the same, to make it easier.
 
 // // // // // // // // // // // // // // // // // // // // // // // // // // //
 // Things that have types
 // // // // // // // // // // // // // // // // // // // // // // // // // // //
 export { default as Bubble } from './Bubble';
+export { default as Button, ButtonGroup, ButtonToolbar } from './Button';
 export { default as Chip, CHIP_PGN_CLASS } from './Chip';
 export { default as ChipCarousel } from './ChipCarousel';
 export { default as Hyperlink, HYPER_LINK_EXTERNAL_LINK_ALT_TEXT, HYPER_LINK_EXTERNAL_LINK_TITLE } from './Hyperlink';
@@ -21,7 +22,6 @@ export { default as Avatar } from './Avatar';
 export { default as AvatarButton } from './AvatarButton';
 export { default as Badge } from './Badge';
 export { default as Breadcrumb } from './Breadcrumb';
-export { default as Button, ButtonGroup, ButtonToolbar } from './Button';
 export {
   default as Card,
   CardColumns,