Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed paste bugs on headings and lists #3949

Merged
merged 3 commits into from Dec 18, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions blocks/api/raw-handling/index.js
Expand Up @@ -34,9 +34,10 @@ import shortcodeConverter from './shortcode-converter';
* * 'AUTO': Decide based on the content passed.
* * 'INLINE': Always handle as inline content, and return string.
* * 'BLOCKS': Always handle as blocks, and return array of blocks.
* @param {Array} [options.tagName] The tag into which content will be inserted.
* @return {Array|String} A list of blocks or a string, depending on `handlerMode`.
*/
export default function rawHandler( { HTML, plainText = '', mode = 'AUTO' } ) {
export default function rawHandler( { HTML, plainText = '', mode = 'AUTO', tagName } ) {
// First of all, strip any meta tags.
HTML = HTML.replace( /<meta[^>]+>/, '' );

Expand Down Expand Up @@ -65,7 +66,7 @@ export default function rawHandler( { HTML, plainText = '', mode = 'AUTO' } ) {
const hasShortcodes = pieces.length > 1;

// True if mode is auto, no shortcode is included and HTML verifies the isInlineContent condition
const isAutoModeInline = mode === 'AUTO' && isInlineContent( HTML ) && ! hasShortcodes;
const isAutoModeInline = mode === 'AUTO' && isInlineContent( HTML, tagName ) && ! hasShortcodes;

// Return filtered HTML if condition is true
if ( mode === 'INLINE' || isAutoModeInline ) {
Expand All @@ -74,7 +75,7 @@ export default function rawHandler( { HTML, plainText = '', mode = 'AUTO' } ) {
formattingTransformer,
stripAttributes,
commentRemover,
createUnwrapper( ( node ) => ! isInline( node ) ),
createUnwrapper( ( node ) => ! isInline( node, tagName ) ),
] );

// Allows us to ask for this information when we get a report.
Expand Down
10 changes: 5 additions & 5 deletions blocks/api/raw-handling/is-inline-content.js
Expand Up @@ -3,19 +3,19 @@
*/
import { isInline, isDoubleBR } from './utils';

export default function( HTML ) {
export default function( HTML, tagName ) {
const doc = document.implementation.createHTMLDocument( '' );

doc.body.innerHTML = HTML;

const nodes = Array.from( doc.body.children );

return ! nodes.some( isDoubleBR ) && deepCheck( nodes );
return ! nodes.some( isDoubleBR ) && deepCheck( nodes, tagName );
}

function deepCheck( nodes ) {
function deepCheck( nodes, tagName ) {
return nodes.every( ( node ) => {
return ( 'SPAN' === node.nodeName || isInline( node ) ) &&
deepCheck( Array.from( node.children ) );
return ( 'SPAN' === node.nodeName || isInline( node, tagName ) ) &&
deepCheck( Array.from( node.children ), tagName );
} );
}
4 changes: 4 additions & 0 deletions blocks/api/raw-handling/test/is-inline-content.js
Expand Up @@ -12,12 +12,16 @@ describe( 'stripWrappers', () => {
it( 'should be inline content', () => {
equal( isInlineContent( '<em>test</em>' ), true );
equal( isInlineContent( '<span>test</span>' ), true );
equal( isInlineContent( '<li>test</li>', 'ul' ), true );
} );

it( 'should not be inline content', () => {
equal( isInlineContent( '<div>test</div>' ), false );
equal( isInlineContent( '<em>test</em><div>test</div>' ), false );
equal( isInlineContent( 'test<br><br>test' ), false );
equal( isInlineContent( '<em><div>test</div></em>' ), false );
equal( isInlineContent( '<li>test</li>', 'p' ), false );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest adding a test that mixes the list group with the header group.

equal( isInlineContent( '<li>test</li>', 'h1' ), false );
equal( isInlineContent( '<h1>test</h1>', 'li' ), false );
} );
} );
37 changes: 35 additions & 2 deletions blocks/api/raw-handling/utils.js
@@ -1,8 +1,23 @@
/**
* External dependencies
*/
import { includes } from 'lodash';

/**
* Browser dependencies
*/
const { ELEMENT_NODE, TEXT_NODE } = window.Node;

/**
* An array of tag groups used by isInlineForTag function.
* If tagName and nodeName are present in the same group, the node should be treated as inline.
* @type {Array}
*/
const inlineWhitelistTagGroups = [
[ 'ul', 'li', 'ol' ],
[ 'h1', 'h2', 'h3', 'h4', 'h5', 'h6' ],
];

const inlineWhitelist = {
strong: {},
em: {},
Expand Down Expand Up @@ -63,8 +78,26 @@ export function isAttributeWhitelisted( tag, attribute ) {
);
}

export function isInline( node ) {
return !! inlineWhitelist[ node.nodeName.toLowerCase() ];
/**
* Checks if nodeName should be treated as inline when being added to tagName.
* This happens if nodeName and tagName are in the same group defined in inlineWhitelistTagGroups.
*
* @param {String} nodeName Node name.
* @param {String} tagName Tag name.
* @return {Boolean} True if nodeName is inline in the context of tagName and false otherwise.
*/
function isInlineForTag( nodeName, tagName ) {
if ( ! tagName || ! nodeName ) {
return false;
}
return inlineWhitelistTagGroups.some( tagGroup =>
includes( tagGroup, nodeName ) && includes( tagGroup, tagName )
);
}

export function isInline( node, tagName ) {
const nodeName = node.nodeName.toLowerCase();
return !! inlineWhitelist[ nodeName ] || isInlineForTag( nodeName, tagName );
}

export function isInlineWrapper( node ) {
Expand Down
1 change: 1 addition & 0 deletions blocks/editable/index.js
Expand Up @@ -352,6 +352,7 @@ export default class Editable extends Component {
HTML: this.pastedContent || HTML,
plainText: this.pastedPlainText,
mode,
tagName: this.props.tagName,
} );

if ( typeof content === 'string' ) {
Expand Down