From 087bbd9891ba567bff4a7304f67617e16fb07afe Mon Sep 17 00:00:00 2001 From: Greg Thompson Date: Tue, 26 Jul 2022 11:32:42 -0500 Subject: [PATCH 1/6] memoize components created inside of hooks --- src/components/code/code_block.tsx | 156 +++++++++++++++-------------- 1 file changed, 82 insertions(+), 74 deletions(-) diff --git a/src/components/code/code_block.tsx b/src/components/code/code_block.tsx index b8261e255bb..c63613f9dd5 100644 --- a/src/components/code/code_block.tsx +++ b/src/components/code/code_block.tsx @@ -351,30 +351,33 @@ const useCopy = ({ const showCopyButton = isCopyable && textToCopy; - const CopyButton = () => { - if (!showCopyButton) return null; - - return ( -
- - {(copyButton: string) => ( - - {(copy) => ( - - )} - - )} - -
- ); - }; - - return { innerTextRef, showCopyButton, CopyButton }; + const _CopyButton = useMemo(() => { + const CopyButton: FunctionComponent = () => { + if (!showCopyButton) return null; + + return ( +
+ + {(copyButton: string) => ( + + {(copy) => ( + + )} + + )} + +
+ ); + }; + return CopyButton; + }, [showCopyButton, textToCopy]); + + return { innerTextRef, showCopyButton, CopyButton: _CopyButton }; }; /** @@ -388,9 +391,9 @@ const useFullScreen = ({ }) => { const [isFullScreen, setIsFullScreen] = useState(false); - const toggleFullScreen = () => { + const toggleFullScreen = useCallback(() => { setIsFullScreen(!isFullScreen); - }; + }, [isFullScreen]); const onKeyDown = useCallback((event: KeyboardEvent) => { if (event.key === keys.ESCAPE) { @@ -402,57 +405,62 @@ const useFullScreen = ({ const showFullScreenButton = !!overflowHeight; - const FullScreenButton: React.FC = () => { - if (!showFullScreenButton) return null; - return ( - - {([fullscreenCollapse, fullscreenExpand]: string[]) => ( - - )} - - ); - }; - - const FullScreenDisplay: React.FC<{ className: string }> = ({ - children, - className, - }) => { - if (!isFullScreen) return null; - - // Force fullscreen to use large font and padding. - const fullScreenClasses = classNames( - className, - 'euiCodeBlock--fontLarge', - 'euiCodeBlock--paddingLarge', - 'euiCodeBlock-isFullScreen' - ); - - // Attaches to the body because of EuiOverlayMask's React portal usage. - return ( - - -
{children}
-
-
- ); - }; + const _FullScreenButton = useMemo(() => { + const FullScreenButton: FunctionComponent = () => { + if (!showFullScreenButton) return null; + return ( + + {([fullscreenCollapse, fullscreenExpand]: string[]) => ( + + )} + + ); + }; + return FullScreenButton; + }, [isFullScreen, showFullScreenButton, toggleFullScreen]); + + const _FullScreenDisplay = useMemo(() => { + const FullScreenDisplay: FunctionComponent<{ + className: string; + }> = ({ children, className }) => { + if (!isFullScreen) return null; + + // Force fullscreen to use large font and padding. + const fullScreenClasses = classNames( + className, + 'euiCodeBlock--fontLarge', + 'euiCodeBlock--paddingLarge', + 'euiCodeBlock-isFullScreen' + ); + + // Attaches to the body because of EuiOverlayMask's React portal usage. + return ( + + +
{children}
+
+
+ ); + }; + return FullScreenDisplay; + }, [isFullScreen]); return { showFullScreenButton, - FullScreenButton, - FullScreenDisplay, + FullScreenButton: _FullScreenButton, + FullScreenDisplay: _FullScreenDisplay, onKeyDown, }; }; From fc121d0a30374cab0d2e6393c394e04106c05af3 Mon Sep 17 00:00:00 2001 From: Greg Thompson Date: Tue, 26 Jul 2022 11:38:06 -0500 Subject: [PATCH 2/6] memoize constants that could otherwise cause component remounts --- src/components/code/code_block.tsx | 47 +++++++++++++++++------------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/src/components/code/code_block.tsx b/src/components/code/code_block.tsx index c63613f9dd5..27d72d55cdc 100644 --- a/src/components/code/code_block.tsx +++ b/src/components/code/code_block.tsx @@ -232,28 +232,35 @@ export const EuiCodeBlock: FunctionComponent = ({ [preClasses, onKeyDown] ); - const optionalStyles: CSSProperties = {}; - - if (overflowHeight) { - const property = - typeof overflowHeight === 'string' ? 'height' : 'maxHeight'; - optionalStyles[property] = overflowHeight; - } + const optionalStyles: CSSProperties = useMemo(() => { + if (overflowHeight) { + const property = + typeof overflowHeight === 'string' ? 'height' : 'maxHeight'; + return { + [property]: overflowHeight, + }; + } + return {}; + }, [overflowHeight]); - const wrapperProps = { - className: classes, - style: optionalStyles, - }; + const wrapperProps = useMemo( + () => ({ + className: classes, + style: optionalStyles, + }), + [classes, optionalStyles] + ); - let codeBlockControls; - if (showCopyButton || showFullScreenButton) { - codeBlockControls = ( -
- - -
- ); - } + const codeBlockControls = useMemo(() => { + if (showCopyButton || showFullScreenButton) { + return ( +
+ + +
+ ); + } + }, [CopyButton, FullScreenButton, showCopyButton, showFullScreenButton]); return (
From f3d0007095749cb6ad7bc1cafb9a0f52d664c89a Mon Sep 17 00:00:00 2001 From: Greg Thompson Date: Tue, 26 Jul 2022 11:49:13 -0500 Subject: [PATCH 3/6] CL --- upcoming_changelogs/6077.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 upcoming_changelogs/6077.md diff --git a/upcoming_changelogs/6077.md b/upcoming_changelogs/6077.md new file mode 100644 index 00000000000..d667e382a76 --- /dev/null +++ b/upcoming_changelogs/6077.md @@ -0,0 +1,4 @@ +**Bug fixes** + +- Fixed uninentional subcomponent remounting in `EuiCodeBlock` during rerenders + From 98526869ba287ea8c2cf4af552baf4a6f691cc86 Mon Sep 17 00:00:00 2001 From: Greg Thompson Date: Wed, 27 Jul 2022 16:46:59 -0500 Subject: [PATCH 4/6] optionalStyles -> overflowHeightStyles --- src/components/code/code_block.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/code/code_block.tsx b/src/components/code/code_block.tsx index 27d72d55cdc..4c723ad0fac 100644 --- a/src/components/code/code_block.tsx +++ b/src/components/code/code_block.tsx @@ -232,7 +232,7 @@ export const EuiCodeBlock: FunctionComponent = ({ [preClasses, onKeyDown] ); - const optionalStyles: CSSProperties = useMemo(() => { + const overflowHeightStyles: CSSProperties = useMemo(() => { if (overflowHeight) { const property = typeof overflowHeight === 'string' ? 'height' : 'maxHeight'; @@ -246,9 +246,9 @@ export const EuiCodeBlock: FunctionComponent = ({ const wrapperProps = useMemo( () => ({ className: classes, - style: optionalStyles, + style: overflowHeightStyles, }), - [classes, optionalStyles] + [classes, overflowHeightStyles] ); const codeBlockControls = useMemo(() => { From 70d5af7468abdbf2a5fec343cac08b49f7f80b84 Mon Sep 17 00:00:00 2001 From: Greg Thompson Date: Wed, 27 Jul 2022 16:50:36 -0500 Subject: [PATCH 5/6] pull components out of hooks; EuiI18n -> useI18n --- .../__snapshots__/code_block.test.tsx.snap | 153 ++++++------- src/components/code/code_block.tsx | 207 +++++++++--------- 2 files changed, 168 insertions(+), 192 deletions(-) diff --git a/src/components/code/__snapshots__/code_block.test.tsx.snap b/src/components/code/__snapshots__/code_block.test.tsx.snap index e642da216f7..8c1bd5ff1b5 100644 --- a/src/components/code/__snapshots__/code_block.test.tsx.snap +++ b/src/components/code/__snapshots__/code_block.test.tsx.snap @@ -85,54 +85,41 @@ exports[`EuiCodeBlock fullscreen displays content in fullscreen mode 1`] = `
- - - + + - - + /> + + + -
`; @@ -412,74 +399,68 @@ exports[`EuiCodeBlock props isCopyable is rendered 1`] = `
- - +
- - + - - - - - - - - - + /> + + + + + +
- `; diff --git a/src/components/code/code_block.tsx b/src/components/code/code_block.tsx index 4c723ad0fac..37cffeed115 100644 --- a/src/components/code/code_block.tsx +++ b/src/components/code/code_block.tsx @@ -27,7 +27,7 @@ import { EuiButtonIcon } from '../button'; import { keysOf, ExclusiveUnion } from '../common'; import { EuiCopy } from '../copy'; import { EuiFocusTrap } from '../focus_trap'; -import { EuiI18n } from '../i18n'; +import { useEuiI18n } from '../i18n'; import { useInnerText } from '../inner_text'; import { useMutationObserver } from '../observer/mutation_observer'; import { useResizeObserver } from '../observer/resize_observer'; @@ -171,7 +171,7 @@ export const EuiCodeBlock: FunctionComponent = ({ [_isVirtualized, data] ); - const { innerTextRef, showCopyButton, CopyButton } = useCopy({ + const { innerTextRef, showCopyButton, textToCopy } = useCopy({ isCopyable, isVirtualized, children, @@ -187,8 +187,8 @@ export const EuiCodeBlock: FunctionComponent = ({ const { showFullScreenButton, onKeyDown, - FullScreenButton, - FullScreenDisplay, + isFullScreen, + toggleFullScreen, } = useFullScreen({ overflowHeight }); // Classes used in both fullscreen and non-fullscreen mode @@ -255,12 +255,23 @@ export const EuiCodeBlock: FunctionComponent = ({ if (showCopyButton || showFullScreenButton) { return (
- - + {showFullScreenButton && ( + + )} + {showCopyButton && }
); } - }, [CopyButton, FullScreenButton, showCopyButton, showFullScreenButton]); + }, [ + isFullScreen, + toggleFullScreen, + showCopyButton, + showFullScreenButton, + textToCopy, + ]); return (
@@ -275,7 +286,7 @@ export const EuiCodeBlock: FunctionComponent = ({ ) : (
@@ -284,21 +295,23 @@ export const EuiCodeBlock: FunctionComponent = ({
       )}
       {codeBlockControls}
 
-      
-        {isVirtualized ? (
-          
-        ) : (
-          
-            {content}
-          
- )} - {codeBlockControls} -
+ {isFullScreen && ( + + {isVirtualized ? ( + + ) : ( +
+              {content}
+            
+ )} + {codeBlockControls} +
+ )}
); }; @@ -340,6 +353,26 @@ const useOverflowDetection = () => { * Copy logic */ +const CopyButton: FunctionComponent<{ + textToCopy: string; +}> = ({ textToCopy }) => { + const copyButton = useEuiI18n('euiCodeBlock.copyButton', 'Copy'); + return ( +
+ + {(copy) => ( + + )} + +
+ ); +}; + const useCopy = ({ isCopyable, isVirtualized, @@ -358,39 +391,53 @@ const useCopy = ({ const showCopyButton = isCopyable && textToCopy; - const _CopyButton = useMemo(() => { - const CopyButton: FunctionComponent = () => { - if (!showCopyButton) return null; - - return ( -
- - {(copyButton: string) => ( - - {(copy) => ( - - )} - - )} - -
- ); - }; - return CopyButton; - }, [showCopyButton, textToCopy]); - - return { innerTextRef, showCopyButton, CopyButton: _CopyButton }; + return { innerTextRef, showCopyButton, textToCopy }; }; /** * Fullscreen logic */ +const FullScreenButton: FunctionComponent<{ + isFullScreen: boolean; + toggleFullScreen: () => void; +}> = ({ isFullScreen, toggleFullScreen }) => { + const [fullscreenCollapse, fullscreenExpand] = useEuiI18n( + ['euiCodeBlock.fullscreenCollapse', 'euiCodeBlock.fullscreenExpand'], + ['Collapse', 'Expand'] + ); + return ( + + ); +}; + +const FullScreenDisplay: FunctionComponent<{ + className: string; +}> = ({ children, className }) => { + // Force fullscreen to use large font and padding. + const fullScreenClasses = classNames( + className, + 'euiCodeBlock--fontLarge', + 'euiCodeBlock--paddingLarge', + 'euiCodeBlock-isFullScreen' + ); + + // Attaches to the body because of EuiOverlayMask's React portal usage. + return ( + + +
{children}
+
+
+ ); +}; + const useFullScreen = ({ overflowHeight, }: { @@ -399,8 +446,8 @@ const useFullScreen = ({ const [isFullScreen, setIsFullScreen] = useState(false); const toggleFullScreen = useCallback(() => { - setIsFullScreen(!isFullScreen); - }, [isFullScreen]); + setIsFullScreen((isFullScreen) => !isFullScreen); + }, []); const onKeyDown = useCallback((event: KeyboardEvent) => { if (event.key === keys.ESCAPE) { @@ -412,62 +459,10 @@ const useFullScreen = ({ const showFullScreenButton = !!overflowHeight; - const _FullScreenButton = useMemo(() => { - const FullScreenButton: FunctionComponent = () => { - if (!showFullScreenButton) return null; - return ( - - {([fullscreenCollapse, fullscreenExpand]: string[]) => ( - - )} - - ); - }; - return FullScreenButton; - }, [isFullScreen, showFullScreenButton, toggleFullScreen]); - - const _FullScreenDisplay = useMemo(() => { - const FullScreenDisplay: FunctionComponent<{ - className: string; - }> = ({ children, className }) => { - if (!isFullScreen) return null; - - // Force fullscreen to use large font and padding. - const fullScreenClasses = classNames( - className, - 'euiCodeBlock--fontLarge', - 'euiCodeBlock--paddingLarge', - 'euiCodeBlock-isFullScreen' - ); - - // Attaches to the body because of EuiOverlayMask's React portal usage. - return ( - - -
{children}
-
-
- ); - }; - return FullScreenDisplay; - }, [isFullScreen]); - return { showFullScreenButton, - FullScreenButton: _FullScreenButton, - FullScreenDisplay: _FullScreenDisplay, + isFullScreen, + toggleFullScreen, onKeyDown, }; }; From 4b7556c9347bc78910fbc65074adb9d3d90c6c42 Mon Sep 17 00:00:00 2001 From: Greg Thompson Date: Thu, 28 Jul 2022 08:34:31 -0500 Subject: [PATCH 6/6] Update upcoming_changelogs/6077.md Co-authored-by: Constance --- upcoming_changelogs/6077.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/upcoming_changelogs/6077.md b/upcoming_changelogs/6077.md index d667e382a76..9f6b78681df 100644 --- a/upcoming_changelogs/6077.md +++ b/upcoming_changelogs/6077.md @@ -1,4 +1,4 @@ **Bug fixes** -- Fixed uninentional subcomponent remounting in `EuiCodeBlock` during rerenders +- Fixed unintentional subcomponent remounting in `EuiCodeBlock` during rerenders