Skip to content

Commit

Permalink
Fix virtualized code blocks broken line wrapping
Browse files Browse the repository at this point in the history
- because react-window virtualization sets the same height for each line, we need to enforce a white-space of `pre` for virtaulization

- Fix incorrect type union not actually enforcing `overflowHeight` (isVirtualized needs to be set to `false`), and add unit tests with ts-expect-error for incorrect props

- Improve prop docs dev experience slightly, move default comment blocks to to main props interface and have the ExclusiveUnion only handle prop differences
  • Loading branch information
cee-chen committed Nov 22, 2021
1 parent 35f2bf5 commit 5ddb91f
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 22 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
- Fixed `EuiCodeBlock` not closing full-screen mode when the Escape key is pressed ([#5379](https://github.com/elastic/eui/pull/5379))
- Fixed virtualized `EuiCodeBlock`s blanking out when entering & exiting full-screen mode ([#5379](https://github.com/elastic/eui/pull/5379))
- Fixed `EuiCodeBlock`'s full-screen mode to use a large font and padding size & added several missing wrapper classes ([#5379](https://github.com/elastic/eui/pull/5379))
- Fixed `EuiCodeBlock` broken line wrapping when using virtualization ([#5379](https://github.com/elastic/eui/pull/5379))

**Theme: Amsterdam**

Expand Down
23 changes: 17 additions & 6 deletions src-docs/src/views/code/code_example.js
Original file line number Diff line number Diff line change
Expand Up @@ -253,12 +253,23 @@ export const CodeExample = {
},
],
text: (
<p>
For large blocks of code, add <EuiCode>isVirtualized</EuiCode> to
reduce the number of rendered rows and improve load times. Note that{' '}
<EuiCode>overflowHeight</EuiCode> is required when using this
configuration.
</p>
<>
<p>
For large blocks of code, add <EuiCode>isVirtualized</EuiCode> to
reduce the number of rendered rows and improve load times. Note that
when using virtualization:
</p>
<ul>
<li>
<EuiCode>overflowHeight</EuiCode> is required
</li>
<li>
<EuiCode>whiteSpace</EuiCode> is enforced as{' '}
<EuiCode>pre</EuiCode>, and cannot be set to{' '}
<EuiCode>pre-wrap</EuiCode>
</li>
</ul>
</>
),
props: { EuiCodeBlock },
snippet: codeBlockVirtualizedSnippet,
Expand Down
2 changes: 1 addition & 1 deletion src/components/code/__snapshots__/code_block.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ exports[`EuiCodeBlock virtualization renders a virtualized code block 1`] = `
data-eui="EuiAutoSizer"
>
<pre
class="euiCodeBlock__pre euiCodeBlock__pre--whiteSpacePreWrap euiCodeBlock__pre--isVirtualized"
class="euiCodeBlock__pre euiCodeBlock__pre--whiteSpacePre euiCodeBlock__pre--isVirtualized"
style="position:relative;height:600px;width:600px;overflow:auto;-webkit-overflow-scrolling:touch;will-change:transform;direction:ltr"
tabindex="0"
>
Expand Down
24 changes: 23 additions & 1 deletion src/components/code/code_block.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import React, { useState, useEffect } from 'react';
import ReactDOM from 'react-dom';
import { mount, render } from 'enzyme';
import { mount, render, shallow } from 'enzyme';
import { act } from 'react-dom/test-utils';
import { requiredProps } from '../../test/required_props';

Expand Down Expand Up @@ -199,6 +199,28 @@ describe('EuiCodeBlock', () => {
);
expect(component).toMatchSnapshot();
});

describe('type checks', () => {
it('requires overflowHeight', () => {
// @ts-expect-error should expect overflowHeight
shallow(<EuiCodeBlock isVirtualized overflowHeight={undefined} />);
});

it('only allows whiteSpace of pre', () => {
shallow(
// @ts-expect-error should only accept "pre"
<EuiCodeBlock
isVirtualized
overflowHeight={50}
whiteSpace="pre-wrap"
/>
);
// OK
shallow(
<EuiCodeBlock isVirtualized overflowHeight={50} whiteSpace="pre" />
);
});
});
});

describe('line numbers', () => {
Expand Down
37 changes: 23 additions & 14 deletions src/components/code/code_block.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,25 +68,17 @@ const paddingSizeToClassNameMap: { [paddingSize in PaddingSize]: string } = {

export const PADDING_SIZES = keysOf(paddingSizeToClassNameMap);

// overflowHeight is required when using virtualization
// This exclusive union enforces specific props based on isVirtualized
type VirtualizedOptionProps = ExclusiveUnion<
{
/**
* Renders code block lines virtually.
* Useful for improving load times of large code blocks.
* `overflowHeight` is required when using this configuration.
*/
isVirtualized: true;
/**
* Sets the maximum container height.
* Accepts a pixel value (`300`) or a percentage (`'100%'`)
* Ensure the container has calcuable height when using a percentage
*/
overflowHeight: number | string;
whiteSpace?: 'pre';
},
{
isVirtualized?: boolean;
isVirtualized?: false;
overflowHeight?: number | string;
whiteSpace?: 'pre' | 'pre-wrap';
}
>;

Expand Down Expand Up @@ -117,6 +109,22 @@ export type EuiCodeBlockProps = EuiCodeSharedProps & {
* `{ start: 100, highlight: '1, 5-10, 20-30, 40' }`
*/
lineNumbers?: boolean | LineNumbersConfig;

/**
* Sets the maximum container height.
* Accepts a pixel value (`300`) or a percentage (`'100%'`)
* Ensure the container has calcuable height when using a percentage
*/
overflowHeight?: number | string;

/**
* Renders code block lines virtually.
* Useful for improving load times of large code blocks.
*
* When using this configuration, `overflowHeight` is required and
* `whiteSpace` can only be `pre`.
*/
isVirtualized?: boolean;
} & VirtualizedOptionProps;

export const EuiCodeBlock: FunctionComponent<EuiCodeBlockProps> = ({
Expand Down Expand Up @@ -209,8 +217,9 @@ export const EuiCodeBlock: FunctionComponent<EuiCodeBlockProps> = ({
);

const preClasses = classNames('euiCodeBlock__pre', {
'euiCodeBlock__pre--whiteSpacePre': whiteSpace === 'pre',
'euiCodeBlock__pre--whiteSpacePreWrap': whiteSpace === 'pre-wrap',
'euiCodeBlock__pre--whiteSpacePre': whiteSpace === 'pre' || isVirtualized,
'euiCodeBlock__pre--whiteSpacePreWrap':
whiteSpace === 'pre-wrap' && !isVirtualized,
'euiCodeBlock__pre--isVirtualized': isVirtualized,
});
const preFullscreenProps = useMemo(
Expand Down

0 comments on commit 5ddb91f

Please sign in to comment.