Skip to content

Commit

Permalink
Fix: Long values should be truncated in the Tree and PropertyList to …
Browse files Browse the repository at this point in the history
…avoid performance issues. Closes #31.
  • Loading branch information
oleq committed Mar 19, 2019
1 parent aef8995 commit 70aac89
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 6 deletions.
8 changes: 6 additions & 2 deletions src/components/propertylist.js
Expand Up @@ -4,15 +4,19 @@
*/

import React, { Component } from 'react';
import { uid } from './utils';
import { uid, truncateString } from './utils';
import './propertylist.css';

const MAX_PROPERTY_VALUE_LENGTH = 2000;

export default class PropertyList extends Component {
render() {
const listUid = uid();

return <dl className="ck-inspector-property-list ck-inspector-code">
{this.props.items.map( ( [ name, value ] ) => {
value = truncateString( String( value ), MAX_PROPERTY_VALUE_LENGTH );

return [
<dt key={`${ name }-name`}>
<label htmlFor={`${ listUid }-${ name }-input`}>
Expand All @@ -23,7 +27,7 @@ export default class PropertyList extends Component {
<input
id={`${ listUid }-${ name }-input`}
type="text"
value={String( value )}
value={value}
readOnly={true}
/>
</dd>
Expand Down
8 changes: 6 additions & 2 deletions src/components/tree.js
Expand Up @@ -4,8 +4,11 @@
*/

import React, { Component } from 'react';
import { truncateString } from './utils';
import './tree.css';

const MAX_ATTRIBUTE_VALUE_LENGTH = 500;

export default class Tree extends Component {
render() {
let treeContent;
Expand Down Expand Up @@ -131,15 +134,16 @@ export class TreeTextNode extends TreeNode {
export class TreeNodeAttribute extends Component {
render() {
let valueElement;
const value = truncateString( this.props.value, MAX_ATTRIBUTE_VALUE_LENGTH );

if ( !this.props.dontRenderValue ) {
valueElement = <span className="ck-inspector-tree-node__attribute__value">
{this.props.value}
{value}
</span>;
}

return <span className="ck-inspector-tree-node__attribute">
<span className="ck-inspector-tree-node__attribute__name" title={this.props.value}>
<span className="ck-inspector-tree-node__attribute__name" title={value}>
{this.props.name}
</span>
{valueElement}
Expand Down
8 changes: 8 additions & 0 deletions src/components/utils.js
Expand Up @@ -48,3 +48,11 @@ export function stringifyPropertyList( list ) {
return [ name, stringify( value ) ];
} );
}

export function truncateString( string, length ) {
if ( string.length > length ) {
return string.substr( 0, length ) + `… [${ string.length - length } characters left]`;
}

return string;
}
19 changes: 19 additions & 0 deletions tests/inspector/components/propertylist.js
Expand Up @@ -43,4 +43,23 @@ describe( '<PropertyList />', () => {
expect( dt1.find( 'label' ) ).to.have.attr( 'for' ).equal( dd1.find( 'input' ).prop( 'id' ) );
expect( dt2.find( 'label' ) ).to.have.attr( 'for' ).equal( dd2.find( 'input' ).prop( 'id' ) );
} );

it( 'truncates property values to 2000 characters', () => {
const items = [
[ 'foo', new Array( 1999 ).fill( 0 ).join( '' ) ],
[ 'bar', new Array( 2000 ).fill( 0 ).join( '' ) ],
[ 'baz', new Array( 2100 ).fill( 0 ).join( '' ) ]
];

wrapper = mount( <PropertyList items={items} /> );

const dd1 = wrapper.children().childAt( 1 );
const dd2 = wrapper.children().childAt( 3 );
const dd3 = wrapper.children().childAt( 5 );

expect( dd1.find( 'input' ).props().value ).to.have.length( 1999 );
expect( dd2.find( 'input' ).props().value ).to.have.length( 2000 );
expect( dd3.find( 'input' ).props().value ).to.have.lengthOf.below( 2100 );
expect( dd3.find( 'input' ).props().value ).to.match( /characters left]$/ );
} );
} );
33 changes: 32 additions & 1 deletion tests/inspector/components/tree.js
Expand Up @@ -9,7 +9,8 @@ import Tree, {
TreePlainText,
TreeSelection,
TreeElement,
TreeComment
TreeComment,
TreeNodeAttribute
} from '../../../src/components/tree';

describe( '<Tree />', () => {
Expand Down Expand Up @@ -378,4 +379,34 @@ describe( '<Tree />', () => {
expect( childAB.html() ).to.equal( '<span class="ck-inspector-tree-comment"><b>bar</b></span>' );
} );
} );

describe( 'attribute', () => {
it( 'truncates values above 500 characters', () => {
const wrapper = mount( <Tree
items={[
{
type: 'text',
node: 'node',
attributes: [
[ 'foo', new Array( 499 ).fill( 0 ).join( '' ) ],
[ 'bar', new Array( 500 ).fill( 0 ).join( '' ) ],
[ 'baz', new Array( 550 ).fill( 0 ).join( '' ) ]
],
children: []
}
]}
/> );

const firstAttribute = wrapper.children().find( TreeNodeAttribute ).first();
const secondAttribute = wrapper.children().find( TreeNodeAttribute ).at( 1 );
const thirdAttribute = wrapper.children().find( TreeNodeAttribute ).last();

expect( firstAttribute.childAt( 0 ).childAt( 1 ).text() ).to.have.length( 499 );
expect( secondAttribute.childAt( 0 ).childAt( 1 ).text() ).to.have.length( 500 );
expect( thirdAttribute.childAt( 0 ).childAt( 1 ).text() ).to.have.lengthOf.below( 550 );
expect( thirdAttribute.childAt( 0 ).childAt( 1 ).text() ).to.match( /characters left]$/ );

wrapper.unmount();
} );
} );
} );
15 changes: 14 additions & 1 deletion tests/inspector/components/utils.js
Expand Up @@ -6,7 +6,8 @@
import {
stringify,
uid,
stringifyPropertyList
stringifyPropertyList,
truncateString,
} from '../../../src/components/utils';

describe( 'Utils', () => {
Expand Down Expand Up @@ -49,4 +50,16 @@ describe( 'Utils', () => {
] );
} );
} );

describe( 'truncateString()', () => {
it( 'truncates when too long', () => {
expect( truncateString( '1234', 3 ) ).to.equal( '123… [1 characters left]' );
expect( truncateString( '1234', 2 ) ).to.equal( '12… [2 characters left]' );
} );

it( 'does nothing when in limit', () => {
expect( truncateString( '1234', 4 ) ).to.equal( '1234' );
expect( truncateString( '1234', 5 ) ).to.equal( '1234' );
} );
} );
} );

0 comments on commit 70aac89

Please sign in to comment.