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

Allow to disable new line padding on paste for specific elements #17068

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/ckeditor5-clipboard/src/clipboardpipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ export default class ClipboardPipeline extends Plugin {
this.listenTo<ViewDocumentClipboardOutputEvent>( viewDocument, 'clipboardOutput', ( evt, data ) => {
if ( !data.content.isEmpty ) {
data.dataTransfer.setData( 'text/html', this.editor.data.htmlProcessor.toData( data.content ) );
data.dataTransfer.setData( 'text/plain', viewToPlainText( data.content ) );
data.dataTransfer.setData( 'text/plain', viewToPlainText( editor.data.htmlProcessor.domConverter, data.content ) );
}

if ( data.method == 'cut' ) {
Expand Down
16 changes: 13 additions & 3 deletions packages/ckeditor5-clipboard/src/utils/viewtoplaintext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @module clipboard/utils/viewtoplaintext
*/

import type { ViewDocumentFragment, ViewElement, ViewItem } from '@ckeditor/ckeditor5-engine';
import type { DomConverter, ViewDocumentFragment, ViewElement, ViewItem } from '@ckeditor/ckeditor5-engine';

// Elements which should not have empty-line padding.
// Most `view.ContainerElement` want to be separate by new-line, but some are creating one structure
Expand All @@ -19,10 +19,14 @@ const listElements = [ 'ol', 'ul' ];
/**
* Converts {@link module:engine/view/item~Item view item} and all of its children to plain text.
*
* @param converter The converter instance.
* @param viewItem View item to convert.
* @returns Plain text representation of `viewItem`.
*/
export default function viewToPlainText( viewItem: ViewItem | ViewDocumentFragment ): string {
export default function viewToPlainText(
converter: DomConverter,
viewItem: ViewItem | ViewDocumentFragment
): string {
Mati365 marked this conversation as resolved.
Show resolved Hide resolved
if ( viewItem.is( '$text' ) || viewItem.is( '$textProxy' ) ) {
return viewItem.data;
}
Expand All @@ -44,7 +48,7 @@ export default function viewToPlainText( viewItem: ViewItem | ViewDocumentFragme
let prev: ViewElement | null = null;

for ( const child of ( viewItem as ViewElement | ViewDocumentFragment ).getChildren() ) {
text += newLinePadding( child as ViewElement, prev ) + viewToPlainText( child );
text += newLinePadding( converter, child as ViewElement, prev ) + viewToPlainText( converter, child );
prev = child as ViewElement;
}

Expand All @@ -55,6 +59,7 @@ export default function viewToPlainText( viewItem: ViewItem | ViewDocumentFragme
* Returns new line padding to prefix the given elements with.
*/
function newLinePadding(
converter: DomConverter,
element: ViewElement,
previous: ViewElement | null
): string {
Expand Down Expand Up @@ -94,6 +99,11 @@ function newLinePadding(
return '';
}

if ( !converter.isBlockViewElement( previous ) && !converter.isBlockViewElement( element ) ) {
// Don't add padding between non-block elements.
return '';
}

// Add empty lines between container elements.
return '\n\n';
}
23 changes: 21 additions & 2 deletions packages/ckeditor5-clipboard/tests/utils/viewtoplaintext.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,26 @@
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
*/

import { DomConverter, StylesProcessor, ViewDocument } from '@ckeditor/ckeditor5-engine';
import viewToPlainText from '../../src/utils/viewtoplaintext.js';

import { parse as parseView } from '@ckeditor/ckeditor5-engine/src/dev-utils/view.js';

describe( 'viewToPlainText()', () => {
let converter, viewDocument;

beforeEach( () => {
viewDocument = new ViewDocument( new StylesProcessor() );
converter = new DomConverter( viewDocument );
} );

afterEach( () => {
viewDocument.destroy();
} );

function testViewToPlainText( viewString, expectedText ) {
const view = parseView( viewString );
const text = viewToPlainText( view );
const text = viewToPlainText( converter, view );

expect( text ).to.equal( expectedText );
}
Expand All @@ -34,14 +46,21 @@ describe( 'viewToPlainText()', () => {
);
} );

it( 'should not put empty line between inline container elements', () => {
testViewToPlainText(
'<container:strong>Foo</container:strong><container:strong>Bar</container:strong>',
'FooBar'
);
} );

it( 'should not put empty line before or after the element with `dataPipeline:transparentRendering` property', () => {
const viewString = 'Abc <container:h1>Header</container:h1> xyz';
const expectedText = 'Abc Header xyz';

const view = parseView( viewString );
view.getChild( 1 )._setCustomProperty( 'dataPipeline:transparentRendering', true );

const text = viewToPlainText( view );
const text = viewToPlainText( converter, view );

expect( text ).to.equal( expectedText );
} );
Expand Down
18 changes: 9 additions & 9 deletions packages/ckeditor5-engine/src/view/domconverter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,7 @@ export default class DomConverter {

if ( viewChild !== null ) {
// Whitespace cleaning before entering a block element (between block elements).
if ( this._isBlockViewElement( viewChild ) ) {
if ( this.isBlockViewElement( viewChild ) ) {
this._processDomInlineNodes( domElement, inlineNodes, options );
}

Expand Down Expand Up @@ -1162,6 +1162,13 @@ export default class DomConverter {
return node && node.nodeType == Node.DOCUMENT_FRAGMENT_NODE;
}

/**
* Returns `true` if a view node belongs to {@link #blockElements}. `false` otherwise.
*/
public isBlockViewElement( node: ViewNode ): boolean {
return node.is( 'element' ) && this.blockElements.includes( node.name );
}

/**
* Checks if the node is an instance of the block filler for this DOM converter.
*
Expand Down Expand Up @@ -1443,7 +1450,7 @@ export default class DomConverter {
if ( this._isViewElementWithRawContent( viewElement, options ) ) {
viewElement._setCustomProperty( '$rawContent', ( domNode as DomElement ).innerHTML );

if ( !this._isBlockViewElement( viewElement ) ) {
if ( !this.isBlockViewElement( viewElement ) ) {
inlineNodes.push( viewElement );
}

Expand Down Expand Up @@ -1743,13 +1750,6 @@ export default class DomConverter {
return this.isElement( node ) && this.blockElements.includes( node.tagName.toLowerCase() );
}

/**
* Returns `true` if a view node belongs to {@link #blockElements}. `false` otherwise.
*/
private _isBlockViewElement( node: ViewNode ): boolean {
return node.is( 'element' ) && this.blockElements.includes( node.name );
}

/**
* Returns `true` if a DOM node belongs to {@link #inlineObjectElements}. `false` otherwise.
*/
Expand Down
28 changes: 28 additions & 0 deletions packages/ckeditor5-engine/tests/view/domconverter/domconverter.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { StylesProcessor } from '../../../src/view/stylesmap.js';
import ViewPosition from '../../../src/view/position.js';
import ViewRange from '../../../src/view/range.js';
import { ViewText } from '@ckeditor/ckeditor5-engine';
import { parse as parseView } from '@ckeditor/ckeditor5-engine/src/dev-utils/view.js';

describe( 'DomConverter', () => {
let converter, viewDocument;
Expand Down Expand Up @@ -425,6 +426,33 @@ describe( 'DomConverter', () => {
} );
} );

describe( 'isBlockViewElement()', () => {
const blockElements = [
'address', 'article', 'aside', 'blockquote', 'caption', 'center', 'dd', 'details', 'dir', 'div',
'dl', 'dt', 'fieldset', 'figcaption', 'figure', 'footer', 'form', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'header',
'hgroup', 'legend', 'li', 'main', 'menu', 'nav', 'ol', 'p', 'pre', 'section', 'summary', 'table', 'tbody',
'td', 'tfoot', 'th', 'thead', 'tr', 'ul'
];

const inlineElements = [ 'span', 'i', 'b', 'strong', 'a' ];

for ( const elementName of blockElements ) {
it( `should return true for <${ elementName }>`, () => {
const view = parseView( `<${ elementName }></${ elementName }>` );

expect( converter.isBlockViewElement( view ) ).to.be.true;
} );
}

for ( const elementName of inlineElements ) {
it( `should return false for <${ elementName }>`, () => {
const view = parseView( `<${ elementName }></${ elementName }>` );

expect( converter.isBlockViewElement( view ) ).to.be.false;
} );
}
} );

describe( 'shouldRenderAttribute()', () => {
it( 'should allow all in data pipeline', () => {
expect( converter.shouldRenderAttribute( 'onclick', 'anything' ) ).to.be.false;
Expand Down