From 945a9207bd6d37650dd65705655cde15c7995499 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Tue, 14 Jan 2020 13:26:22 -0700 Subject: [PATCH 1/4] Effective reverting of #2324 --- .../views/responsive/responsive_example.js | 2 +- src/components/code/_code_block.js | 200 +++++++++--------- 2 files changed, 103 insertions(+), 99 deletions(-) diff --git a/src-docs/src/views/responsive/responsive_example.js b/src-docs/src/views/responsive/responsive_example.js index 4b52b06dba4..a9f88badef8 100644 --- a/src-docs/src/views/responsive/responsive_example.js +++ b/src-docs/src/views/responsive/responsive_example.js @@ -26,7 +26,7 @@ function renderSizes(size, index) { code += ' +'; } - return `${code}\n`; + return
{code}
; } export const ResponsiveExample = { diff --git a/src/components/code/_code_block.js b/src/components/code/_code_block.js index 256a2624ad2..6aa3107e26c 100644 --- a/src/components/code/_code_block.js +++ b/src/components/code/_code_block.js @@ -13,6 +13,7 @@ import { EuiFocusTrap } from '../focus_trap'; import { keyCodes } from '../../services'; import { EuiI18n } from '../i18n'; +import { EuiInnerText } from '../inner_text'; const fontSizeToClassNameMap = { s: 'euiCodeBlock--fontSmall', @@ -96,23 +97,6 @@ export class EuiCodeBlockImpl extends Component { ...otherProps } = this.props; - // To maintain backwards compatibility with incorrect datatypes being passed, this - // logic is a bit expanded to support arrays of non-strings. In those cases two bugs are present: - // * "copy" button does not have access to the correct string values to put on the clipboard - // * dynamically changing the content fails to update the DOM - // - // When this is converted to typescript and children that do not meet `string | string[]` - // invalid uses will fail at compile time and this can be removed for the much simpler - // const childrenAsString = typeof children === 'string' ? children : children.join(''); - let childrenAsString = children; - if (Array.isArray(children)) { - const isArrayOfStrings = - children.filter(item => typeof item !== 'string').length === 0; - if (isArrayOfStrings === true) { - childrenAsString = children.join(''); - } - } - const classes = classNames( 'euiCodeBlock', fontSizeToClassNameMap[fontSize], @@ -140,7 +124,7 @@ export class EuiCodeBlockImpl extends Component { }} className={codeClasses} {...otherProps}> - {childrenAsString} + {children} ); @@ -153,28 +137,32 @@ export class EuiCodeBlockImpl extends Component { return {codeSnippet}; } - let copyButton; - - if (isCopyable) { - copyButton = ( -
- - {copyButton => ( - - {copy => ( - - )} - - )} - -
- ); + function getCopyButton(textToCopy) { + let copyButton; + + if (isCopyable) { + copyButton = ( +
+ + {copyButton => ( + + {copy => ( + + )} + + )} + +
+ ); + } + + return copyButton; } let fullScreenButton; @@ -203,78 +191,94 @@ export class EuiCodeBlockImpl extends Component { ); } - let codeBlockControls; + function getCodeBlockControls(textToCopy) { + let codeBlockControls; + const copyButton = getCopyButton(textToCopy); + + if (copyButton || fullScreenButton) { + codeBlockControls = ( +
+ {fullScreenButton} + {copyButton} +
+ ); + } - if (copyButton || fullScreenButton) { - codeBlockControls = ( -
- {fullScreenButton} - {copyButton} -
- ); + return codeBlockControls; } - let fullScreenDisplay; - - if (this.state.isFullScreen) { - { - /* - Force fullscreen to use large font and padding. - */ + const getFullScreenDisplay = codeBlockControls => { + let fullScreenDisplay; + + if (this.state.isFullScreen) { + { + /* + Force fullscreen to use large font and padding. + */ + } + const fullScreenClasses = classNames( + 'euiCodeBlock', + fontSizeToClassNameMap[fontSize], + 'euiCodeBlock-paddingLarge', + 'euiCodeBlock-isFullScreen', + className + ); + + fullScreenDisplay = ( + + +
+ 
+                   {
+                      this.codeFullScreen = ref;
+                    }}
+                    className={codeClasses}
+                    tabIndex={0}
+                    onKeyDown={this.onKeyDown}>
+                    {children}
+                  
+
+ + {codeBlockControls} +
+ + + ); } - const fullScreenClasses = classNames( - 'euiCodeBlock', - fontSizeToClassNameMap[fontSize], - 'euiCodeBlock-paddingLarge', - 'euiCodeBlock-isFullScreen', - className - ); - fullScreenDisplay = ( - - -
- 
-                 {
-                    this.codeFullScreen = ref;
-                  }}
-                  className={codeClasses}
-                  tabIndex={0}
-                  onKeyDown={this.onKeyDown}>
-                  {childrenAsString}
-                
+      return fullScreenDisplay;
+    };
+
+    return (
+      
+        {(innerTextRef, innerText) => {
+          const codeBlockControls = getCodeBlockControls(innerText);
+          return (
+            

+              
+                {codeSnippet}
               

 
+              {/*
+              If the below fullScreen code renders, it actually attaches to the body because of
+              EuiOverlayMask's React portal usage.
+            */}
               {codeBlockControls}
+              {getFullScreenDisplay(codeBlockControls)}
             

-          
-        
-      );
-    }
-
-    return (
-      

-        
-          {codeSnippet}
-        

-
-        {/*
-          If the below fullScreen code renders, it actually attaches to the body because of
-          EuiOverlayMask's React portal usage.
-        */}
-        {codeBlockControls}
-        {fullScreenDisplay}
-      

+          );
+        }}
+      
     );
   }
 }
 
 EuiCodeBlockImpl.propTypes = {
-  children: PropTypes.oneOfType([
-    PropTypes.string,
-    PropTypes.arrayOf(PropTypes.string),
-  ]),
+  children: PropTypes.node,
   className: PropTypes.string,
   paddingSize: PropTypes.oneOf(PADDING_SIZES),
 

From ec4f479d6f89001aefb0df62877a37b3c9bb121e Mon Sep 17 00:00:00 2001
From: Chandler Prall 
Date: Thu, 16 Jan 2020 13:59:21 -0700
Subject: [PATCH 2/4] Made EuiCodeBlock updates React-safe

---
 .../__snapshots__/code_block.test.js.snap     | 30 +++++++
 src/components/code/_code_block.js            | 81 ++++++++++++-------
 src/components/code/code_block.test.js        | 48 ++++++++++-
 3 files changed, 129 insertions(+), 30 deletions(-)

diff --git a/src/components/code/__snapshots__/code_block.test.js.snap b/src/components/code/__snapshots__/code_block.test.js.snap
index 7766bf49ddc..fb8e3a432ac 100644
--- a/src/components/code/__snapshots__/code_block.test.js.snap
+++ b/src/components/code/__snapshots__/code_block.test.js.snap
@@ -1,5 +1,35 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`EuiCodeBlock dynamic content updates DOM when input changes 1`] = `
+"

+  

+    
+      
+        

+          constvalue =
+          'State 1'
+        

+      
+    

+  

+
"
+`;
+
+exports[`EuiCodeBlock dynamic content updates DOM when input changes 2`] = `
+"

+  

+    
+      
+        

+          constvalue =
+          'State 2'
+        

+      
+    

+  

+
"
+`;
+
 exports[`EuiCodeBlock props fontSize l is rendered 1`] = `
 
 {
+    // because React maintains a mapping between its Virtual DOM representation and the actual
+    // DOM elements (including text nodes), and hljs modifies the DOM structure which leads
+    // to React updating detached nodes, we render to a document fragment and
+    // copy from that fragment into the target elements
+    // (https://github.com/elastic/eui/issues/2322)
+    const html = this.codeTarget.innerHTML;
+
+    if (this.code) {
+      this.code.innerHTML = html;
+    }
+    if (this.codeFullScreen) {
+      this.codeFullScreen.innerHTML = html;
+    }
+
     if (this.props.language) {
       hljs.highlightBlock(this.code);
 
@@ -123,9 +140,8 @@ export class EuiCodeBlockImpl extends Component {
           this.code = ref;
         }}
         className={codeClasses}
-        {...otherProps}>
-        {children}
-      
+        {...otherProps}
+      />
     );
 
     const wrapperProps = {
@@ -134,7 +150,12 @@ export class EuiCodeBlockImpl extends Component {
     };
 
     if (inline) {
-      return {codeSnippet};
+      return (
+        <>
+          {createPortal(
{children}
, this.codeTarget)}
+          {codeSnippet}
+        
+      );
     }
 
     function getCopyButton(textToCopy) {
@@ -235,9 +256,8 @@ export class EuiCodeBlockImpl extends Component {
                     }}
                     className={codeClasses}
                     tabIndex={0}
-                    onKeyDown={this.onKeyDown}>
-                    {children}
-                  
+                    onKeyDown={this.onKeyDown}
+                  />
{codeBlockControls} @@ -251,28 +271,31 @@ export class EuiCodeBlockImpl extends Component { }; return ( - - {(innerTextRef, innerText) => { - const codeBlockControls = getCodeBlockControls(innerText); - return ( -
- 
-                {codeSnippet}
-
- - {/* - If the below fullScreen code renders, it actually attaches to the body because of - EuiOverlayMask's React portal usage. - */} - {codeBlockControls} - {getFullScreenDisplay(codeBlockControls)} -
- ); - }} - + <> + {createPortal(
{children}
, this.codeTarget)} + + {(innerTextRef, innerText) => { + const codeBlockControls = getCodeBlockControls(innerText); + return ( +
+ 
+                  {codeSnippet}
+
+ + {/* + If the below fullScreen code renders, it actually attaches to the body because of + EuiOverlayMask's React portal usage. + */} + {codeBlockControls} + {getFullScreenDisplay(codeBlockControls)} +
+ ); + }} + + ); } } diff --git a/src/components/code/code_block.test.js b/src/components/code/code_block.test.js index 34471b967d9..2c1b24c45b1 100644 --- a/src/components/code/code_block.test.js +++ b/src/components/code/code_block.test.js @@ -1,5 +1,7 @@ -import React from 'react'; +import React, { useState, useEffect } from 'react'; +import ReactDOM from 'react-dom'; import { render } from 'enzyme'; +import html from 'html'; import { requiredProps } from '../../test/required_props'; import { EuiCodeBlock } from './code_block'; @@ -82,4 +84,48 @@ describe('EuiCodeBlock', () => { }); }); }); + + describe('dynamic content', () => { + it('updates DOM when input changes', done => { + expect.assertions(2); + + function takeSnapshot() { + expect( + html.prettyPrint(appDiv.innerHTML, { + indent_size: 2, + unformatted: [], // Expand all tags, including spans + }) + ).toMatchSnapshot(); + } + + // enzyme does not recreate enough of the React<->DOM interaction to reproduce this bug + const appDiv = document.createElement('div'); + + function App() { + const [value, setValue] = useState('State 1'); + + useEffect(() => { + takeSnapshot(); + setValue('State 2'); + }, []); + + useEffect(() => { + if (value === 'State 2') { + takeSnapshot(); + done(); + } + }, [value]); + + return ( +
+ + const value = '{value}' + +
+ ); + } + + ReactDOM.render(, appDiv); + }); + }); }); From c5249ac5276bf45c36fac3811b969d24f7ad006f Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Fri, 24 Jan 2020 14:57:27 -0700 Subject: [PATCH 3/4] Updated all EuiCode* tests working as full mounts --- .../__snapshots__/_code_block.test.js.snap | 28 +++++++--- .../code/__snapshots__/code.test.js.snap | 4 +- .../__snapshots__/code_block.test.js.snap | 54 +++++++++++++------ src/components/code/_code_block.test.js | 36 +++++++------ src/components/code/code.test.js | 14 +++-- src/components/code/code_block.test.js | 40 ++++++++------ 6 files changed, 117 insertions(+), 59 deletions(-) diff --git a/src/components/code/__snapshots__/_code_block.test.js.snap b/src/components/code/__snapshots__/_code_block.test.js.snap index b8a20d13d4e..061ef4b8564 100644 --- a/src/components/code/__snapshots__/_code_block.test.js.snap +++ b/src/components/code/__snapshots__/_code_block.test.js.snap @@ -8,8 +8,10 @@ exports[`EuiCodeBlockImpl block highlights javascript code, adding "js" class 1` class="euiCodeBlock__pre" > + class="euiCodeBlock__code js hljs javascript" + > +
+
`; @@ -26,8 +28,10 @@ exports[`EuiCodeBlockImpl block renders a pre block tag 1`] = ` class="euiCodeBlock__code" data-test-subj="test subject string" > - var some = 'code'; +
+ var some = 'code'; console.log(some); +
@@ -42,7 +46,9 @@ exports[`EuiCodeBlockImpl block renders with transparent background 1`] = ` > + > +
+
`; @@ -52,8 +58,10 @@ exports[`EuiCodeBlockImpl inline highlights javascript code, adding "js" class 1 class="euiCodeBlock euiCodeBlock--fontSmall euiCodeBlock--paddingLarge euiCodeBlock--inline" > + class="euiCodeBlock__code js hljs javascript" + > +
+ `; @@ -66,8 +74,10 @@ exports[`EuiCodeBlockImpl inline renders an inline code tag 1`] = ` class="euiCodeBlock__code" data-test-subj="test subject string" > - var some = 'code'; +
+ var some = 'code'; console.log(some); +
`; @@ -78,6 +88,8 @@ exports[`EuiCodeBlockImpl inline renders with transparent background 1`] = ` > + > +
+ `; diff --git a/src/components/code/__snapshots__/code.test.js.snap b/src/components/code/__snapshots__/code.test.js.snap index 09b5bfe4f3a..500e653a4fc 100644 --- a/src/components/code/__snapshots__/code.test.js.snap +++ b/src/components/code/__snapshots__/code.test.js.snap @@ -9,8 +9,10 @@ exports[`EuiCode renders a code snippet 1`] = ` class="euiCodeBlock__code" data-test-subj="test subject string" > - var some = 'code'; +
+ var some = 'code'; console.log(some); +
`; diff --git a/src/components/code/__snapshots__/code_block.test.js.snap b/src/components/code/__snapshots__/code_block.test.js.snap index fb8e3a432ac..3ef7eb0328a 100644 --- a/src/components/code/__snapshots__/code_block.test.js.snap +++ b/src/components/code/__snapshots__/code_block.test.js.snap @@ -40,8 +40,10 @@ exports[`EuiCodeBlock props fontSize l is rendered 1`] = ` - var some = 'code'; +
+ var some = 'code'; console.log(some); +
@@ -57,8 +59,10 @@ exports[`EuiCodeBlock props fontSize m is rendered 1`] = ` - var some = 'code'; +
+ var some = 'code'; console.log(some); +
@@ -74,8 +78,10 @@ exports[`EuiCodeBlock props fontSize s is rendered 1`] = ` - var some = 'code'; +
+ var some = 'code'; console.log(some); +
@@ -91,8 +97,10 @@ exports[`EuiCodeBlock props isCopyable is rendered 1`] = ` - var some = 'code'; +
+ var some = 'code'; console.log(some); +
- var some = 'code'; +
+ var some = 'code'; console.log(some); +
@@ -146,17 +156,19 @@ console.log(some); exports[`EuiCodeBlock props overflowHeight is rendered 1`] = ` 
     
-      var some = 'code';
+      

+        var some = 'code';
 console.log(some);
+
- var some = 'code'; +
+ var some = 'code'; console.log(some); +
@@ -209,8 +223,10 @@ exports[`EuiCodeBlock props paddingSize m is rendered 1`] = ` - var some = 'code'; +
+ var some = 'code'; console.log(some); +
@@ -226,8 +242,10 @@ exports[`EuiCodeBlock props paddingSize none is rendered 1`] = ` - var some = 'code'; +
+ var some = 'code'; console.log(some); +
@@ -243,8 +261,10 @@ exports[`EuiCodeBlock props paddingSize s is rendered 1`] = ` - var some = 'code'; +
+ var some = 'code'; console.log(some); +
@@ -260,8 +280,10 @@ exports[`EuiCodeBlock props transparentBackground is rendered 1`] = ` - var some = 'code'; +
+ var some = 'code'; console.log(some); +
@@ -279,8 +301,10 @@ exports[`EuiCodeBlock renders a code block 1`] = ` class="euiCodeBlock__code" data-test-subj="test subject string" > - var some = 'code'; +
+ var some = 'code'; console.log(some); +
diff --git a/src/components/code/_code_block.test.js b/src/components/code/_code_block.test.js index f736c862311..236ab41b493 100644 --- a/src/components/code/_code_block.test.js +++ b/src/components/code/_code_block.test.js @@ -1,66 +1,72 @@ import React from 'react'; -import { render } from 'enzyme'; +import { mount } from 'enzyme'; import { requiredProps } from '../../test/required_props'; import { EuiCodeBlockImpl } from './_code_block'; +function snapshotCodeBlock(component) { + // Get the Portal's sibling and return its html + const renderedHtml = component.find('Portal + *').html(); + const container = document.createElement('div'); + container.innerHTML = renderedHtml; + return container.firstChild; +} + const code = `var some = 'code'; console.log(some);`; describe('EuiCodeBlockImpl', () => { describe('inline', () => { test('renders an inline code tag', () => { - const component = render( + const component = mount( {code} ); - expect(component).toMatchSnapshot(); + expect(snapshotCodeBlock(component)).toMatchSnapshot(); }); test('highlights javascript code, adding "js" class', () => { - const component = render( - - ); + const component = mount(); - expect(component).toMatchSnapshot(); + expect(snapshotCodeBlock(component)).toMatchSnapshot(); }); test('renders with transparent background', () => { - const component = render( + const component = mount( ); - expect(component).toMatchSnapshot(); + expect(snapshotCodeBlock(component)).toMatchSnapshot(); }); }); describe('block', () => { test('renders a pre block tag', () => { - const component = render( + const component = mount( {code} ); - expect(component).toMatchSnapshot(); + expect(snapshotCodeBlock(component)).toMatchSnapshot(); }); test('highlights javascript code, adding "js" class', () => { - const component = render( + const component = mount( ); - expect(component).toMatchSnapshot(); + expect(snapshotCodeBlock(component)).toMatchSnapshot(); }); test('renders with transparent background', () => { - const component = render( + const component = mount( ); - expect(component).toMatchSnapshot(); + expect(snapshotCodeBlock(component)).toMatchSnapshot(); }); }); }); diff --git a/src/components/code/code.test.js b/src/components/code/code.test.js index d7fdebe5e3f..4dee84b378d 100644 --- a/src/components/code/code.test.js +++ b/src/components/code/code.test.js @@ -1,16 +1,24 @@ import React from 'react'; -import { render } from 'enzyme'; +import { mount } from 'enzyme'; import { requiredProps } from '../../test/required_props'; import { EuiCode } from './code'; +function snapshotCodeBlock(component) { + // Get the Portal's sibling and return its html + const renderedHtml = component.find('Portal + *').html(); + const container = document.createElement('div'); + container.innerHTML = renderedHtml; + return container.firstChild; +} + const code = `var some = 'code'; console.log(some);`; describe('EuiCode', () => { test('renders a code snippet', () => { - const component = render({code}); + const component = mount({code}); - expect(component).toMatchSnapshot(); + expect(snapshotCodeBlock(component)).toMatchSnapshot(); }); }); diff --git a/src/components/code/code_block.test.js b/src/components/code/code_block.test.js index 2c1b24c45b1..66e92dd16fd 100644 --- a/src/components/code/code_block.test.js +++ b/src/components/code/code_block.test.js @@ -1,73 +1,79 @@ import React, { useState, useEffect } from 'react'; import ReactDOM from 'react-dom'; -import { render } from 'enzyme'; +import { mount } from 'enzyme'; import html from 'html'; import { requiredProps } from '../../test/required_props'; import { EuiCodeBlock } from './code_block'; import { FONT_SIZES, PADDING_SIZES } from './_code_block'; +function snapshotCodeBlock(component) { + // Get the Portal's sibling and return its html + const renderedHtml = component.find('Portal + *').html(); + const container = document.createElement('div'); + container.innerHTML = renderedHtml; + return container.firstChild; +} + const code = `var some = 'code'; console.log(some);`; describe('EuiCodeBlock', () => { test('renders a code block', () => { - const component = render( + const component = mount( {code} ); - expect(component).toMatchSnapshot(); + expect(snapshotCodeBlock(component)).toMatchSnapshot(); }); describe('props', () => { describe('transparentBackground', () => { it('is rendered', () => { - const component = render( + const component = mount( {code} ); - expect(component).toMatchSnapshot(); + expect(snapshotCodeBlock(component)).toMatchSnapshot(); }); }); describe('isCopyable', () => { it('is rendered', () => { - const component = render( - {code} - ); + const component = mount({code}); - expect(component).toMatchSnapshot(); + expect(snapshotCodeBlock(component)).toMatchSnapshot(); }); }); describe('overflowHeight', () => { it('is rendered', () => { - const component = render( + const component = mount( {code} ); - expect(component).toMatchSnapshot(); + expect(snapshotCodeBlock(component)).toMatchSnapshot(); }); }); describe('language', () => { it('is rendered', () => { - const component = render( + const component = mount( {code} ); - expect(component).toMatchSnapshot(); + expect(snapshotCodeBlock(component)).toMatchSnapshot(); }); }); describe('fontSize', () => { FONT_SIZES.forEach(fontSize => { test(`${fontSize} is rendered`, () => { - const component = render( + const component = mount( {code} ); - expect(component).toMatchSnapshot(); + expect(snapshotCodeBlock(component)).toMatchSnapshot(); }); }); }); @@ -75,11 +81,11 @@ describe('EuiCodeBlock', () => { describe('paddingSize', () => { PADDING_SIZES.forEach(paddingSize => { test(`${paddingSize} is rendered`, () => { - const component = render( + const component = mount( {code} ); - expect(component).toMatchSnapshot(); + expect(snapshotCodeBlock(component)).toMatchSnapshot(); }); }); }); From 5beb6cbcca59492bb3ae5efcc97d5f69bae2b26a Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Mon, 3 Feb 2020 12:13:39 -0700 Subject: [PATCH 4/4] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79e59a99879..a3edf0ada0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ **Bug fixes** - Exported missing `EuiSelectProps` type ([#2815](https://github.com/elastic/eui/pull/2815)) +- Fixed `EuiCode`'s & `EuiCodeBlock`'s ability to accept non-string children ([#2792](https://github.com/elastic/eui/pull/2792)) **Breaking changes**