Introduce a 320 character limit for inbox note contents (https://github.com/woocommerce/woocommerce-admin/pull/7958)

* Introduce a 320 char limit for inbox notes

* Extract truncateRenderableHTML to utils and use truncate from lodash to support word break

* Replace omission with blank so that lodash does not add ... at the end

* Add test cases for truncateRenderableHTML

* Add a test case with unicode string

* Add changelog

* Add a test case for preserving whole words
This commit is contained in:
Moon 2021-11-23 15:47:48 -08:00 committed by GitHub
parent b41ab2f37f
commit d64a276032
4 changed files with 124 additions and 3 deletions

View File

@ -0,0 +1,4 @@
Significance: minor
Type: Update
Introduce a 320 character limit for inbox note contents #7958

View File

@ -24,7 +24,7 @@ import {
* Internal dependencies * Internal dependencies
*/ */
import { ActivityCard } from '../header/activity-panel/activity-card'; import { ActivityCard } from '../header/activity-panel/activity-card';
import { hasValidNotes } from './utils'; import { hasValidNotes, truncateRenderableHTML } from './utils';
import { getScreenName } from '../utils'; import { getScreenName } from '../utils';
import DismissAllModal from './dissmiss-all-modal'; import DismissAllModal from './dissmiss-all-modal';
import './index.scss'; import './index.scss';
@ -179,7 +179,10 @@ const InboxPanel = ( { showHeader = true } ) => {
} = select( NOTES_STORE_NAME ); } = select( NOTES_STORE_NAME );
return { return {
notes: getNotes( INBOX_QUERY ), notes: getNotes( INBOX_QUERY ).map( ( note ) => {
note.content = truncateRenderableHTML( note.content, 320 );
return note;
} ),
isError: Boolean( isError: Boolean(
getNotesError( 'getNotes', [ INBOX_QUERY ] ) getNotesError( 'getNotes', [ INBOX_QUERY ] )
), ),

View File

@ -0,0 +1,54 @@
/**
* Internal dependencies
*/
import { truncateRenderableHTML } from '../utils';
describe( 'truncateRenderableHTML', () => {
test( 'it should recover malformed HTML when truncated', () => {
const malformed = '<div>this is a test sentence</asdf>';
expect( truncateRenderableHTML( malformed, 7 ) ).toBe(
'<div>this is</div>...'
);
} );
test( 'it should not truncate if the length does not exceed', () => {
const sample = '<div>this is a test sentence</div>';
expect( truncateRenderableHTML( sample, sample.length ) ).toBe(
sample
);
} );
test( 'it should consider &nbsp; as a single space', () => {
const samplewithSpace = '<div>this &nbsp;&nbsp; is</div>';
// this(4 chars) + space (1 char) + &nbsp;&nbsp;(2 chars) = 7
expect( truncateRenderableHTML( samplewithSpace, 7 ) ).toBe(
'<div>this &nbsp;&nbsp;</div>...'
);
} );
test( 'it should not count nested tags as text', () => {
const sampleWithNestedTags = '<div>this <br/><br/> is</div>';
// this (4 chars) + space (1 char) + space (1char) + is (2 chars)) = 8
expect( truncateRenderableHTML( sampleWithNestedTags, 8 ) ).toBe(
'<div>this <br/><br/> is</div>'
);
} );
test( 'it should work with unicode text', () => {
const sampleWithUnicode = '<div>테스트 입니다.</div>';
expect( truncateRenderableHTML( sampleWithUnicode, 3 ) ).toBe(
'<div>테스트</div>...'
);
} );
test( 'it should preserve whole words when truncated', () => {
const sample = '<div>this is a test sentence</div>';
// it should return 'this is a' (9 chars) when length 11 is given
// since 'this is a t' (11 chars) cannot include 'test' word without
// breaking the word.
expect( truncateRenderableHTML( sample, 11 ) ).toBe(
'<div>this is a</div>...'
);
} );
} );

View File

@ -1,7 +1,7 @@
/** /**
* External dependencies * External dependencies
*/ */
import { filter } from 'lodash'; import { filter, truncate } from 'lodash';
/** /**
* Get the count of the unread notes from the received list. * Get the count of the unread notes from the received list.
@ -41,3 +41,63 @@ export function hasValidNotes( notes ) {
} ); } );
return validNotes.length > 0; return validNotes.length > 0;
} }
/**
* Truncates characters inside of an element.
* Currently does not count <br> as a character even though it should.
*
* @param {HTMLElement} element HTML element
* @param {number} limit number of characters to limit to
*/
const truncateElement = ( element, limit ) => {
const truncatedNode = document.createElement( 'div' );
const childNodes = Array.from( element.childNodes );
for ( let i = 0; i < childNodes.length; i++ ) {
// Deep clone.
let clone = childNodes[ i ].cloneNode( true );
if (
truncatedNode.textContent.length + clone.textContent.length <=
limit
) {
// No problem including a whole child node, no need to consider truncating at all.
truncatedNode.appendChild( clone );
} else {
const charactersRemaining =
limit - truncatedNode.textContent.length;
if (
! clone.innerHTML ||
clone.textContent.slice( 0, charactersRemaining ) ===
clone.innerHTML.slice( 0, charactersRemaining )
) {
// If text until the limit doesn't contain any markup, we're all good to truncate.
clone.textContent = truncate( clone.textContent, {
length: charactersRemaining,
separator: ' ',
omission: '',
} );
} else {
// If it does, then we'd need to recursively run this with balance of characters remaining.
clone = truncateElement( clone, charactersRemaining );
}
truncatedNode.appendChild( clone );
// Exceeded limit at this point, safe to exit loop.
break;
}
}
return truncatedNode;
};
/**
* Truncates characters from a HTML string excluding markup. Truncated strings will be appended with ellipsis.
*
* @param {string} originalHTML HTML string
* @param {number} limit number of characters to limit to
*/
export const truncateRenderableHTML = ( originalHTML, limit ) => {
const tempNode = document.createElement( 'div' );
tempNode.innerHTML = originalHTML;
if ( tempNode.textContent.length > limit ) {
return truncateElement( tempNode, limit ).innerHTML + '...';
}
return originalHTML;
};