From 51a5f7feee29382caf14ba5ba4673e282bbb93c2 Mon Sep 17 00:00:00 2001 From: Russell Anderson Date: Tue, 25 Oct 2022 10:16:22 -0500 Subject: [PATCH] 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 --- packages/button/components/ButtonBase.tsx | 192 +++++++++--------- .../__snapshots__/ButtonBase.test.tsx.snap | 5 + packages/link/components/UnstyledLink.tsx | 14 +- packages/link/types.ts | 4 +- .../tests/__snapshots__/Modal.test.tsx.snap | 102 ++++++---- .../components/SegmentedControlButton.tsx | 34 +++- .../shared/components/FocusStyleManager.tsx | 2 +- 7 files changed, 205 insertions(+), 148 deletions(-) diff --git a/packages/button/components/ButtonBase.tsx b/packages/button/components/ButtonBase.tsx index 33a71c5d7..975eca85c 100644 --- a/packages/button/components/ButtonBase.tsx +++ b/packages/button/components/ButtonBase.tsx @@ -91,56 +91,24 @@ 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) => { - if (!disabled && onClick) { - onClick(e); - } - }; - - const getIconStart = icon => { - return ( - - - - ); - }; - - const getIconEnd = icon => { - return ( - Boolean(icon) && ( +const ButtonContent = ({ iconStart, iconEnd, isProcessing, children }) => { + return iconStart || iconEnd ? ( + + {iconStart && ( + + + + )} + {children && ( + + {children} + + )} + {iconEnd && ( { padding("left", "xxs") )} > - + - ) - ); - }; - - const getButtonContent = () => { - const { iconStart, iconEnd, isProcessing, children } = props; + )} + + ) : ( + {children} + ); +}; - return iconStart || iconEnd ? ( - - {iconStart && getIconStart(iconStart)} - {children && ( - - {children} - - )} - {iconEnd && getIconEnd(iconEnd)} - - ) : ( - - {children} - +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) => { + if (!disabled && onClick) { + onClick(e); + } + }; + if (url) { - return !disabled && !isProcessing ? ( + const enabled = !disabled && !isProcessing; + return ( - {getButtonContent()} - - ) : ( - - {getButtonContent()} + + {children} + ); } @@ -211,18 +199,32 @@ const ButtonBase = (props: ButtonBaseProps) => { onClick={handleClick} tabIndex={0} type={type} + ref={ref as React.ForwardedRef} + aria-haspopup={ariaHaspopup} + aria-label={ariaLabel} {...other} > - {getButtonContent()} + + {children} + ); - }; + } +); +const ButtonBase = (props: ButtonBaseProps) => { return ( - {getButtonNode()} + ); }; diff --git a/packages/button/tests/__snapshots__/ButtonBase.test.tsx.snap b/packages/button/tests/__snapshots__/ButtonBase.test.tsx.snap index 9db366a3e..01f6756ef 100644 --- 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`] = ` } + + @@ -2137,17 +2145,24 @@ exports[`Modal DialogModal renders DialogModal 1`] = ` - + + @@ -3974,17 +3989,25 @@ exports[`Modal FullscreenModal renders FullscreenModal 1`] = ` - + + @@ -4012,17 +4035,24 @@ exports[`Modal FullscreenModal renders FullscreenModal 1`] = ` - + + diff --git a/packages/segmentedControl/components/SegmentedControlButton.tsx b/packages/segmentedControl/components/SegmentedControlButton.tsx index 76aa1b66b..470130c55 100644 --- a/packages/segmentedControl/components/SegmentedControlButton.tsx +++ b/packages/segmentedControl/components/SegmentedControlButton.tsx @@ -39,23 +39,27 @@ export interface SegmentedControlButtonProps { children?: React.ReactNode; } -const SegmentedControlButton = ({ - id = nextId("segmentedControlButton-"), - isActive, - onChange, - name, - value, - tooltipContent, - children -}: SegmentedControlButtonProps) => { - return ( - +const SegmentedControlLabel = React.forwardRef( + ( + { + id = nextId("segmentedControlButton-"), + isActive, + onChange, + name, + value, + tooltipContent, + children + }: SegmentedControlButtonProps, + ref + ) => { + return ( + ); + } +); + +const SegmentedControlButton = (props: SegmentedControlButtonProps) => { + return ( + + {props.children} ); }; diff --git a/packages/shared/components/FocusStyleManager.tsx b/packages/shared/components/FocusStyleManager.tsx index bfa656e09..1a227b330 100644 --- a/packages/shared/components/FocusStyleManager.tsx +++ b/packages/shared/components/FocusStyleManager.tsx @@ -11,7 +11,7 @@ const FocusStyleManager = ({ focusEnabledClass, children }: FocusStyleManagerProps) => { - const focusWrapperRef = React.useRef(); + const focusWrapperRef = React.useRef(); React.useEffect(() => { if (focusWrapperRef.current) {