fix: forwardrefs to FocusStyleManager children (#843)

I am not sure that I did this correctly. This is a tough then where the child component could literally be anything, so typing the ref is very difficult. In some cases its a button, others an a, sometimes a label. In any event, for the ui-kit components that are children of FocusStyleManager, I wrapped them in forwardRef. Resolves the console warning issues for the pages that use FocusStyleManager. Closes D2IQ-93522
d2iq · Oct 25, 2022 · 51a5f7f · 51a5f7f
1 parent 29bcf94
commit 51a5f7f
Show file tree

Hide file tree

Showing 7 changed files with 205 additions and 148 deletions.
diff --git a/packages/button/components/ButtonBase.tsx b/packages/button/components/ButtonBase.tsx
@@ -91,115 +91,103 @@ export interface ButtonBaseProps extends ButtonProps {
   className?: string;
 }
 
-const ButtonBase = (props: ButtonBaseProps) => {
-  const {
-    appearance,
-    children,
-    className,
-    disabled,
-    iconStart,
-    iconEnd,
-    isInverse,
-    isProcessing,
-    isFullWidth,
-    onClick,
-    type = "button",
-    url,
-    openInNewTab,
-    ...other
-  } = props;
-
-  const buttonClassName = cx(
-    buttonReset,
-    button(appearance),
-    buttonBase,
-    textWeight("medium"),
-    className,
-    {
-      [fullWidthButton]: isFullWidth,
-      [buttonInverse(appearance)]: isInverse,
-      [getMutedButtonStyles(appearance)]: disabled || isProcessing,
-      [getInverseMutedButtonStyles(appearance)]:
-        (disabled || isProcessing) && isInverse
-    }
-  );
-
-  const handleClick = (e: React.SyntheticEvent<HTMLElement>) => {
-    if (!disabled && onClick) {
-      onClick(e);
-    }
-  };
-
-  const getIconStart = icon => {
-    return (
-      <span className={cx(flexItem("shrink"), display("inherit"))}>
-        <IconPropAdapter icon={icon} size="xs" color="inherit" />
-      </span>
-    );
-  };
-
-  const getIconEnd = icon => {
-    return (
-      Boolean(icon) && (
+const ButtonContent = ({ iconStart, iconEnd, isProcessing, children }) => {
+  return iconStart || iconEnd ? (
+    <span className={flex({ align: "center", justify: "center" })}>
+      {iconStart && (
+        <span className={cx(flexItem("shrink"), display("inherit"))}>
+          <IconPropAdapter icon={iconStart} size="xs" color="inherit" />
+        </span>
+      )}
+      {children && (
+        <span
+          className={cx(flexItem("shrink"), padding("left", "xs"), {
+            [processingTextStyle]: isProcessing
+          })}
+        >
+          {children}
+        </span>
+      )}
+      {iconEnd && (
         <span
           className={cx(
             flexItem("shrink"),
             display("inherit"),
             padding("left", "xxs")
           )}
         >
-          <IconPropAdapter icon={icon} size="xs" color="inherit" />
+          <IconPropAdapter icon={iconEnd} size="xs" color="inherit" />
         </span>
-      )
-    );
-  };
-
-  const getButtonContent = () => {
-    const { iconStart, iconEnd, isProcessing, children } = props;
+      )}
+    </span>
+  ) : (
+    <span className={isProcessing ? processingTextStyle : ""}>{children}</span>
+  );
+};
 
-    return iconStart || iconEnd ? (
-      <span className={flex({ align: "center", justify: "center" })}>
-        {iconStart && getIconStart(iconStart)}
-        {children && (
-          <span
-            className={cx(flexItem("shrink"), padding("left", "xs"), {
-              [processingTextStyle]: isProcessing
-            })}
-          >
-            {children}
-          </span>
-        )}
-        {iconEnd && getIconEnd(iconEnd)}
-      </span>
-    ) : (
-      <span className={isProcessing ? processingTextStyle : ""}>
-        {children}
-      </span>
+const ButtonNode = React.forwardRef(
+  (
+    {
+      appearance,
+      children,
+      className,
+      disabled,
+      iconStart,
+      iconEnd,
+      isInverse,
+      isProcessing,
+      isFullWidth,
+      onClick,
+      type = "button",
+      url,
+      openInNewTab,
+      ariaHaspopup,
+      ariaLabel,
+      ...other
+    }: ButtonBaseProps,
+    ref
+  ) => {
+    const buttonClassName = cx(
+      buttonReset,
+      button(appearance),
+      buttonBase,
+      textWeight("medium"),
+      className,
+      {
+        [fullWidthButton]: isFullWidth,
+        [buttonInverse(appearance)]: isInverse,
+        [getMutedButtonStyles(appearance)]: disabled || isProcessing,
+        [getInverseMutedButtonStyles(appearance)]:
+          (disabled || isProcessing) && isInverse
+      }
     );
-  };
 
-  const getButtonNode = () => {
+    const handleClick = (e: React.SyntheticEvent<HTMLElement>) => {
+      if (!disabled && onClick) {
+        onClick(e);
+      }
+    };
+
     if (url) {
-      return !disabled && !isProcessing ? (
+      const enabled = !disabled && !isProcessing;
+      return (
         <UnstyledLink
-          href={url}
+          href={enabled ? url : undefined}
+          aria-disabled={!enabled}
           className={buttonClassName}
           onClick={handleClick}
-          tabIndex={0}
+          tabIndex={enabled ? 0 : -1}
           openInNewTab={openInNewTab}
+          ref={ref}
           {...other}
         >
-          {getButtonContent()}
-        </UnstyledLink>
-      ) : (
-        <UnstyledLink
-          className={buttonClassName}
-          aria-disabled="true"
-          tabIndex={-1}
-          openInNewTab={openInNewTab}
-          {...other}
-        >
-          {getButtonContent()}
+          <ButtonContent
+            iconStart={iconStart}
+            iconEnd={iconEnd}
+            isProcessing={isProcessing}
+          >
+            {children}
+          </ButtonContent>
         </UnstyledLink>
       );
     }
@@ -211,18 +199,32 @@ const ButtonBase = (props: ButtonBaseProps) => {
         onClick={handleClick}
         tabIndex={0}
         type={type}
+        ref={ref as React.ForwardedRef<HTMLButtonElement>}
+        aria-haspopup={ariaHaspopup}
+        aria-label={ariaLabel}
         {...other}
       >
-        {getButtonContent()}
+        <ButtonContent
+          iconStart={iconStart}
+          iconEnd={iconEnd}
+          isProcessing={isProcessing}
+        >
+          {children}
+        </ButtonContent>
       </button>
     );
-  };
+  }
+);
 
+const ButtonBase = (props: ButtonBaseProps) => {
   return (
     <FocusStyleManager
-      focusEnabledClass={focusStyleByAppearance(appearance, isInverse)}
+      focusEnabledClass={focusStyleByAppearance(
+        props.appearance,
+        props.isInverse
+      )}
     >
-      {getButtonNode()}
+      <ButtonNode {...props} />
     </FocusStyleManager>
   );
 };

diff --git a/packages/button/tests/__snapshots__/ButtonBase.test.tsx.snap b/packages/button/tests/__snapshots__/ButtonBase.test.tsx.snap
@@ -168,6 +168,7 @@ exports[`ButtonBase renders all appearances with props 1`] = `
 }
 
 <button
+    aria-haspopup="true"
     class="emotion-0 emotion-1"
     disabled=""
     tabindex="0"
@@ -389,6 +390,7 @@ exports[`ButtonBase renders all appearances with props 2`] = `
 }
 
 <button
+    aria-haspopup="true"
     class="emotion-0 emotion-1"
     disabled=""
     tabindex="0"
@@ -611,6 +613,7 @@ exports[`ButtonBase renders all appearances with props 3`] = `
 }
 
 <button
+    aria-haspopup="true"
     class="emotion-0 emotion-1"
     disabled=""
     tabindex="0"
@@ -833,6 +836,7 @@ exports[`ButtonBase renders all appearances with props 4`] = `
 }
 
 <button
+    aria-haspopup="true"
     class="emotion-0 emotion-1"
     disabled=""
     tabindex="0"
@@ -1055,6 +1059,7 @@ exports[`ButtonBase renders all appearances with props 5`] = `
 }
 
 <button
+    aria-haspopup="true"
     class="emotion-0 emotion-1"
     disabled=""
     tabindex="0"

diff --git a/packages/link/components/UnstyledLink.tsx b/packages/link/components/UnstyledLink.tsx
@@ -2,23 +2,29 @@ import React from "react";
 import { LinkComponentContext } from "../../uiKitProvider/link/context";
 import { ExpandedLinkProps } from "../types";
 
-const UnstyledLink = (props: ExpandedLinkProps) => {
+const UnstyledLink = (props: ExpandedLinkProps, ref) => {
   const { href, url, target, openInNewTab, children, ...rest } = props;
 
   // Context is used for `UIKitProvider` link delegation
   const LinkComponent = React.useContext(LinkComponentContext);
   if (LinkComponent) {
-    return <LinkComponent {...props} />;
+    return <LinkComponent ref={ref} {...props} />;
   }
 
   const rel = target === "_blank" || openInNewTab ? "noopener" : undefined;
   const blankTargetOrDefault = !target && openInNewTab ? "_blank" : target;
 
   return (
-    <a href={href || url} target={blankTargetOrDefault} rel={rel} {...rest}>
+    <a
+      ref={ref}
+      href={href || url}
+      target={blankTargetOrDefault}
+      rel={rel}
+      {...rest}
+    >
       {children}
     </a>
   );
 };
 
-export default UnstyledLink;
+export default React.forwardRef(UnstyledLink);
diff --git a/packages/link/types.ts b/packages/link/types.ts
@@ -15,4 +15,6 @@ export interface LinkProps {
 export type ExpandedLinkProps = LinkProps &
   Omit<React.HTMLProps<HTMLAnchorElement>, "ref">;
 
-export type LinkComponent = React.ComponentType<ExpandedLinkProps>;
+export type LinkComponent = React.ComponentType<
+  LinkProps & React.HTMLProps<HTMLAnchorElement>
+>;