From 00153d0013a92af27c535fbc6cf3592fd40ad400 Mon Sep 17 00:00:00 2001
From: Xavier Drdak <1198051+xdrdak@users.noreply.github.com>
Date: Fri, 12 Jun 2020 15:23:47 -0400
Subject: [PATCH] ForwardRef on Remaining Base Components (#75)

* Forward ref on remaining base components

* adjust changelog

Co-authored-by: Guillaume Lambert <24464151+glambert@users.noreply.github.com>
---
 packages/flame/CHANGELOG.md               |   4 +
 packages/flame/src/Button/Button.test.tsx |   6 ++
 packages/flame/src/Button/Button.tsx      | 100 ++++++++++++----------
 packages/flame/src/Switch/Switch.test.tsx |   6 ++
 packages/flame/src/Switch/Switch.tsx      |  34 +++++---
 5 files changed, 90 insertions(+), 60 deletions(-)

diff --git a/packages/flame/CHANGELOG.md b/packages/flame/CHANGELOG.md
index 2a528ca4..76cb349d 100644
--- a/packages/flame/CHANGELOG.md
+++ b/packages/flame/CHANGELOG.md
@@ -9,6 +9,10 @@ Refer to the [CONTRIBUTING guide](https://github.com/lightspeed/flame/blob/maste
 
 ## [Unreleased]
 
+### Breaking
+
+- Use React.forwardRef on `<Button>` and `<Switch>` components ([#75](https://github.com/lightspeed/flame/pull/75))
+
 ### Deprecation Warning
 
 - Alert icon prop will be removed in the next major feature release. Icons will be automatically assigned to an alert based on the variant used ([#82](https://github.com/lightspeed/flame/pull/82))
diff --git a/packages/flame/src/Button/Button.test.tsx b/packages/flame/src/Button/Button.test.tsx
index aa7329da..355a8f4a 100644
--- a/packages/flame/src/Button/Button.test.tsx
+++ b/packages/flame/src/Button/Button.test.tsx
@@ -103,4 +103,10 @@ describe('Button Component', () => {
     getByTitle('Loading...');
     expect(domNode).not.toBeVisible();
   });
+
+  it('should forward the ref properly', () => {
+    const ref = React.createRef<HTMLButtonElement>();
+    customRender(<Button ref={ref}>Button with a ref attached to it</Button>);
+    expect(ref.current.type).toBe('button');
+  });
 });
diff --git a/packages/flame/src/Button/Button.tsx b/packages/flame/src/Button/Button.tsx
index ac14ce6e..4ed7a852 100644
--- a/packages/flame/src/Button/Button.tsx
+++ b/packages/flame/src/Button/Button.tsx
@@ -177,53 +177,59 @@ export type ButtonProps = Merge<
 /**
  * Buttons are used to take action or confirm a decision. They help merchants get things done.
  */
-export const Button: React.FunctionComponent<ButtonProps> = ({
-  loading,
-  children,
-  size,
-  noSpacing,
-  fill,
-  variant,
-  forcedState,
-  className,
-  disabled,
-  ...restProps
-}) => {
-  const iconAdjustments = loneIconAdjustments(children, size);
-
-  const nextChildren = React.Children.map(children, child => remapChild(child, size));
-  const LinkifiedButton = restProps.href
-    ? ExtendedBaseButton.withComponent('a')
-    : ExtendedBaseButton;
-
-  const nextForceState = forcedState && `cr-button--${forcedState}`;
-  const isDisabled = loading || disabled;
-
-  // TODO: Need to rework or split off the link buttons from the main component.
-  // It causes some gnarly typing errors and overloads the component with even
-  // more useless logic, since emotion supports the `as` prop anyways...
-  return (
-    // @ts-ignore
-    <LinkifiedButton
-      isFillButton={fill}
-      size={size}
-      disabled={isDisabled}
-      variant={variant === 'default' ? 'neutral' : variant}
-      className={[className, nextForceState].join(' ')}
-      {...iconAdjustments}
-      {...(restProps as any)}
-    >
-      {loading && (
-        <SpinnerWrapper title="Loading...">
-          <StyledSpinner fill={fill} variant={variant} size={size} />
-        </SpinnerWrapper>
-      )}
-      <ChildWrapper noSpacing={noSpacing} isLoading={loading}>
-        {nextChildren}
-      </ChildWrapper>
-    </LinkifiedButton>
-  );
-};
+export const Button = React.forwardRef<HTMLButtonElement | HTMLAnchorElement, ButtonProps>(
+  (
+    {
+      loading,
+      children,
+      size,
+      noSpacing,
+      fill,
+      variant,
+      forcedState,
+      className,
+      disabled,
+      ...restProps
+    },
+    ref,
+  ) => {
+    const iconAdjustments = loneIconAdjustments(children, size);
+
+    const nextChildren = React.Children.map(children, child => remapChild(child, size));
+    const LinkifiedButton = restProps.href
+      ? ExtendedBaseButton.withComponent('a')
+      : ExtendedBaseButton;
+
+    const nextForceState = forcedState && `cr-button--${forcedState}`;
+    const isDisabled = loading || disabled;
+
+    // TODO: Need to rework or split off the link buttons from the main component.
+    // It causes some gnarly typing errors and overloads the component with even
+    // more useless logic, since emotion supports the `as` prop anyways...
+    return (
+      // @ts-ignore
+      <LinkifiedButton
+        ref={ref}
+        isFillButton={fill}
+        size={size}
+        disabled={isDisabled}
+        variant={variant === 'default' ? 'neutral' : variant}
+        className={[className, nextForceState].join(' ')}
+        {...iconAdjustments}
+        {...(restProps as any)}
+      >
+        {loading && (
+          <SpinnerWrapper title="Loading...">
+            <StyledSpinner fill={fill} variant={variant} size={size} />
+          </SpinnerWrapper>
+        )}
+        <ChildWrapper noSpacing={noSpacing} isLoading={loading}>
+          {nextChildren}
+        </ChildWrapper>
+      </LinkifiedButton>
+    );
+  },
+);
 
 Button.defaultProps = {
   size: 'medium',
diff --git a/packages/flame/src/Switch/Switch.test.tsx b/packages/flame/src/Switch/Switch.test.tsx
index af50c3e4..f5eebdcb 100644
--- a/packages/flame/src/Switch/Switch.test.tsx
+++ b/packages/flame/src/Switch/Switch.test.tsx
@@ -38,4 +38,10 @@ describe('<Switch />', () => {
       expect(container.querySelectorAll('.custom-class')).toBeTruthy();
     });
   });
+
+  it('should forward the ref properly', () => {
+    const ref = React.createRef<HTMLInputElement>();
+    customRender(<Switch ref={ref} />);
+    expect(ref.current.type).toBe('checkbox');
+  });
 });
diff --git a/packages/flame/src/Switch/Switch.tsx b/packages/flame/src/Switch/Switch.tsx
index c8aefb4b..063d5f7f 100644
--- a/packages/flame/src/Switch/Switch.tsx
+++ b/packages/flame/src/Switch/Switch.tsx
@@ -122,17 +122,25 @@ export type SwitchProps = Merge<
 /**
  * A toggleable control which stays on (or off) until manually triggered once more.
  */
-export const Switch: React.FC<SwitchProps> = ({ className, checked, ...restProps }) => (
-  <WrapperLabel role="presentation" className={className}>
-    <SwitchInput type="checkbox" checked={checked} value={checked ? 1 : 0} {...restProps} />
-    <SwitchWrapper>
-      <SwitchSlider>
-        <SwitchSliderIcon />
-      </SwitchSlider>
-      <IconWrapper>
-        <Checkmark size="0.75rem" />
-        <Cross size="0.75rem" />
-      </IconWrapper>
-    </SwitchWrapper>
-  </WrapperLabel>
+export const Switch = React.forwardRef<HTMLInputElement, SwitchProps>(
+  ({ className, checked, ...restProps }, ref) => (
+    <WrapperLabel role="presentation" className={className}>
+      <SwitchInput
+        type="checkbox"
+        checked={checked}
+        value={checked ? 1 : 0}
+        ref={ref}
+        {...restProps}
+      />
+      <SwitchWrapper>
+        <SwitchSlider>
+          <SwitchSliderIcon />
+        </SwitchSlider>
+        <IconWrapper>
+          <Checkmark size="0.75rem" />
+          <Cross size="0.75rem" />
+        </IconWrapper>
+      </SwitchWrapper>
+    </WrapperLabel>
+  ),
 );