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

Add insertObject() function. #11425

Merged
merged 29 commits into from
Apr 1, 2022
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
eb52d8d
WIP: Initial version of fix handling insertion of blocks to document …
CatStrategist Mar 7, 2022
28c1e6d
Add more tests to changing image type and fix tests with deleting blo…
CatStrategist Mar 8, 2022
98c6c82
Change passing elements to copy attributes from to selection and make…
CatStrategist Mar 8, 2022
4b7281a
Create selection of insertion context element passed as option in ins…
CatStrategist Mar 8, 2022
1a65927
Initial version of insertBlock function - handling tables and changin…
CatStrategist Mar 9, 2022
1b8f202
Add a note to remove import later.
CatStrategist Mar 9, 2022
0276261
Finish PoC for insertObject() handling all the cases for document lis…
CatStrategist Mar 10, 2022
976bea3
Add more tests, handling setSelection in insertObject, move getAttrib…
CatStrategist Mar 15, 2022
46ae8c5
Remove unnecessary code.
CatStrategist Mar 15, 2022
0f99e6e
Fix tests.
CatStrategist Mar 15, 2022
9337275
Fixes for inserting content at higher indent.
CatStrategist Mar 16, 2022
d3a5cd6
Code refactoring in progress.
niegowski Mar 16, 2022
ebc3465
Code refactoring.
niegowski Mar 17, 2022
c8aeb96
Add tests.
Mar 30, 2022
de7aeaa
Merge branch 'ck/epic/2973-document-lists' into ck/11198-make-sure-li…
Mar 30, 2022
9e7cde2
Add documentation.
Mar 30, 2022
0c50e19
Fix insertObject() test.
Mar 30, 2022
55479e3
Add tests and documentation to widget type around.
Mar 30, 2022
56584dd
Move setAllowedAttributes() to schema and fix tests.
Mar 30, 2022
7c5b536
Start writing API docs for insertObject().
Mar 30, 2022
7274d3a
Fix API docs.
Mar 30, 2022
fc56ba6
Docs: Documented insertObject().
oleq Mar 31, 2022
02779bb
Review fixes.
oleq Mar 31, 2022
98cf7b6
PR fixes.
Mar 31, 2022
c0e4b21
Merge branch 'ck/11198-make-sure-list-isnt-split' of github.com:ckedi…
Mar 31, 2022
d03fe12
Fix import and revert unnecessary changes to insert table command.
Mar 31, 2022
2561d47
Stub console warn in tests and make engine's findOptimalInsertionRang…
Apr 1, 2022
c528b67
Merge branch 'ck/epic/2973-document-lists' into ck/11198-make-sure-li…
Apr 1, 2022
ee8d2e7
Adjusted tests after mergin the epic branch.
oleq Apr 1, 2022
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
16 changes: 16 additions & 0 deletions packages/ckeditor5-engine/src/model/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import getSelectedContent from './utils/getselectedcontent';
import { injectSelectionPostFixer } from './utils/selection-post-fixer';
import { autoParagraphEmptyRoots } from './utils/autoparagraphing';
import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
import insertObject from '@ckeditor/ckeditor5-widget/src/insertobject';

// @if CK_DEBUG_ENGINE // const { dumpTrees } = require( '../dev-utils/utils' );
// @if CK_DEBUG_ENGINE // const { OperationReplayer } = require( '../dev-utils/operationreplayer' ).default;
Expand Down Expand Up @@ -452,6 +453,21 @@ export default class Model {
return insertContent( this, content, selectable, placeOrOffset );
}

/*
Place for exceptional documentation
Object - an object that we would like to insert
Selectable - 99% - document selection, but sometimes custom one.
Offset - just to pass to insert content
Options - setSelection: 'on|in|after',
findOptimalPosition: true,
// Maybe:
doNotInheritBlockAttributes: true
*/
insertObject( object, selectable, offset, options ) {
// TODO: Get rid of import from widget package, so engine does not depend on it.
return insertObject( this, object, selectable, offset, options );
}

/**
* Deletes content of the selection and merge siblings. The resulting selection is always collapsed.
*
Expand Down
22 changes: 22 additions & 0 deletions packages/ckeditor5-engine/src/model/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -863,6 +863,28 @@ export default class Schema {
}
}

// TODO: API DOCS
getAttributesWithProperty( node, propertyName, propertyValue ) {
const attributes = {};

if ( !node ) {
return attributes;
}

const nodeAttributes = node.getAttributes();

for ( const [ attributeName, attributeValue ] of nodeAttributes ) {
const attributeProperties = this.getAttributeProperties( attributeName );
const isPropertyValueValid = ( !propertyValue || attributeProperties[ propertyName ] === propertyValue );

if ( attributeProperties[ propertyName ] && isPropertyValueValid ) {
attributes[ attributeName ] = attributeValue;
}
}

return attributes;
}

/**
* Creates an instance of the schema context.
*
Expand Down
9 changes: 6 additions & 3 deletions packages/ckeditor5-engine/src/model/utils/deletecontent.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,10 @@ export default function deleteContent( model, selection, options = {} ) {
// Check if a text is allowed in the new container. If not, try to create a new paragraph (if it's allowed here).
// If autoparagraphing is off, we assume that you know what you do so we leave the selection wherever it was.
if ( !options.doNotAutoparagraph && shouldAutoparagraph( schema, startPosition ) ) {
insertParagraph( writer, startPosition, selection );
const selectedElement = selection.getSelectedElement();
const attributesToCopy = schema.getAttributesWithProperty( selectedElement, 'copyOnReplace', true );

insertParagraph( writer, startPosition, selection, attributesToCopy );
}

startPosition.detach();
Expand Down Expand Up @@ -482,8 +485,8 @@ function isCrossingLimitElement( leftPos, rightPos, schema ) {
return true;
}

function insertParagraph( writer, position, selection ) {
const paragraph = writer.createElement( 'paragraph' );
function insertParagraph( writer, position, selection, attributes ) {
const paragraph = writer.createElement( 'paragraph', attributes );

writer.insert( paragraph, position );

Expand Down
24 changes: 6 additions & 18 deletions packages/ckeditor5-horizontal-line/src/horizontallinecommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,24 +44,12 @@ export default class HorizontalLineCommand extends Command {
model.change( writer => {
const horizontalElement = writer.createElement( 'horizontalLine' );

model.insertContent( horizontalElement );

let nextElement = horizontalElement.nextSibling;

// Check whether an element next to the inserted horizontal line is defined and can contain a text.
const canSetSelection = nextElement && model.schema.checkChild( nextElement, '$text' );

// If the element is missing, but a paragraph could be inserted next to the horizontal line, let's add it.
if ( !canSetSelection && model.schema.checkChild( horizontalElement.parent, 'paragraph' ) ) {
nextElement = writer.createElement( 'paragraph' );

model.insertContent( nextElement, writer.createPositionAfter( horizontalElement ) );
}

// Put the selection inside the element, at the beginning.
if ( nextElement ) {
writer.setSelection( nextElement, 0 );
}
model.insertObject(
horizontalElement,
undefined,
undefined,
{ setSelection: 'after' }
);
} );
}
}
Expand Down
67 changes: 67 additions & 0 deletions packages/ckeditor5-horizontal-line/tests/horizontallinecommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -300,5 +300,72 @@ describe( 'HorizontalLineCommand', () => {
'<paragraph>foo</paragraph><horizontalLine></horizontalLine><paragraph>[]bar</paragraph>'
);
} );

describe( 'inheriting attributes', () => {
beforeEach( () => {
const attributes = [ 'smart', 'pretty' ];

model.schema.extend( '$block', {
allowAttributes: attributes
} );

model.schema.extend( '$blockObject', {
allowAttributes: attributes
} );

for ( const attribute of attributes ) {
model.schema.setAttributeProperties( attribute, {
copyOnReplace: true
} );
}
} );

it( 'should copy $block attributes on a horizontal line element when inserting it in $block', () => {
setModelData( model, '<paragraph pretty="true" smart="true">[]</paragraph>' );

command.execute();

expect( getModelData( model ) ).to.equalMarkup(
'<horizontalLine pretty="true" smart="true"></horizontalLine>' +
'<paragraph>[]</paragraph>'
);
} );

it( 'should copy attributes from first selected element', () => {
setModelData( model, '<paragraph pretty="true">[foo</paragraph><paragraph smart="true">bar]</paragraph>' );

command.execute();

expect( getModelData( model ) ).to.equalMarkup(
'<horizontalLine pretty="true"></horizontalLine>' +
'<paragraph>[]</paragraph>'
);
} );

it( 'should only copy $block attributes marked with copyOnReplace', () => {
setModelData( model, '<paragraph pretty="true" smart="true" nice="true">[]</paragraph>' );

command.execute();

expect( getModelData( model ) ).to.equalMarkup(
'<horizontalLine pretty="true" smart="true"></horizontalLine>' +
'<paragraph>[]</paragraph>'
);
} );

it( 'should copy attributes from object when it is selected during insertion', () => {
model.schema.register( 'object', { isObject: true, inheritAllFrom: '$blockObject' } );
editor.conversion.for( 'downcast' ).elementToElement( { model: 'object', view: 'object' } );

setModelData( model, '[<object pretty="true" smart="true"></object>]' );

command.execute();

expect( getModelData( model ) ).to.equalMarkup(
'<horizontalLine pretty="true" smart="true"></horizontalLine>' +
'<paragraph>[]</paragraph>'
);
} );
} );
} );
} );
8 changes: 6 additions & 2 deletions packages/ckeditor5-html-embed/src/htmlembedcommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,12 @@ export default class HtmlEmbedCommand extends Command {
} else {
htmlEmbedElement = writer.createElement( 'rawHtml' );

model.insertContent( htmlEmbedElement );
writer.setSelection( htmlEmbedElement, 'on' );
model.insertObject(
htmlEmbedElement,
undefined,
undefined,
{ setSelection: 'on' }
);
}

writer.setAttribute( 'value', value, htmlEmbedElement );
Expand Down
63 changes: 63 additions & 0 deletions packages/ckeditor5-html-embed/tests/htmlembedcommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -253,5 +253,68 @@ describe( 'HtmlEmbedCommand', () => {
expect( model.document.getRoot().getChild( 0 ) ).to.equal( initialEmbedElement );
} );
} );

describe( 'inheriting attributes', () => {
beforeEach( () => {
const attributes = [ 'smart', 'pretty' ];

model.schema.extend( '$block', {
allowAttributes: attributes
} );

model.schema.extend( '$blockObject', {
allowAttributes: attributes
} );

for ( const attribute of attributes ) {
model.schema.setAttributeProperties( attribute, {
copyOnReplace: true
} );
}
} );

it( 'should copy $block attributes on a html embed element when inserting it in $block', () => {
setModelData( model, '<paragraph pretty="true" smart="true">[]</paragraph>' );

command.execute( '<b>Foo.</b>' );

expect( getModelData( model ) ).to.equalMarkup(
'[<rawHtml pretty="true" smart="true" value="<b>Foo.</b>"></rawHtml>]'
);
} );

it( 'should copy attributes from first selected element', () => {
setModelData( model, '<paragraph pretty="true">[foo</paragraph><paragraph smart="true">bar]</paragraph>' );

command.execute( '<b>Foo.</b>' );

expect( getModelData( model ) ).to.equalMarkup(
'[<rawHtml pretty="true" value="<b>Foo.</b>"></rawHtml>]'
);
} );

it( 'should only copy $block attributes marked with copyOnReplace', () => {
setModelData( model, '<paragraph pretty="true" smart="true" nice="true">[]</paragraph>' );

command.execute( '<b>Foo.</b>' );

expect( getModelData( model ) ).to.equalMarkup(
'[<rawHtml pretty="true" smart="true" value="<b>Foo.</b>"></rawHtml>]'
);
} );

it( 'should copy attributes from object when it is selected during insertion', () => {
model.schema.register( 'object', { isObject: true, inheritAllFrom: '$blockObject' } );
editor.conversion.for( 'downcast' ).elementToElement( { model: 'object', view: 'object' } );

setModelData( model, '[<object pretty="true" smart="true"></object>]' );

command.execute( '<b>Foo.</b>' );

expect( getModelData( model ) ).to.equalMarkup(
'[<rawHtml pretty="true" smart="true" value="<b>Foo.</b>"></rawHtml>]'
);
} );
} );
} );
} );
20 changes: 11 additions & 9 deletions packages/ckeditor5-image/src/imageutils.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,18 +103,20 @@ export default class ImageUtils extends Plugin {
return model.change( writer => {
const imageElement = writer.createElement( imageType, attributes );

// If we want to insert a block image (for whatever reason) then we don't want to split text blocks.
// This applies only when we don't have the selectable specified (i.e., we insert multiple block images at once).
if ( !selectable && imageType != 'imageInline' ) {
selectable = findOptimalInsertionRange( selection, model );
}

model.insertContent( imageElement, selectable );
model.insertObject(
imageElement,
selectable,
undefined,
{
setSelection: 'on',
// If we want to insert a block image (for whatever reason) then we don't want to split text blocks.
// This applies only when we don't have the selectable specified (i.e., we insert multiple block images at once).
findOptimalPosition: !selectable && imageType != 'imageInline'
}
);

// Inserting an image might've failed due to schema regulations.
if ( imageElement.parent ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should this condition be also included in setSelection inside insertObject?

writer.setSelection( imageElement, 'on' );

return imageElement;
}

Expand Down
41 changes: 41 additions & 0 deletions packages/ckeditor5-image/tests/image/imagetypecommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -671,5 +671,46 @@ describe( 'ImageTypeCommand', () => {
setModelData( model, `[<imageBlock src="${ imgSrc }"><caption>foo</caption></imageBlock>]` );
} );
} );

describe( 'inheriting attributes', () => {
const imgSrc = '/foo.jpg';

beforeEach( () => {
const attributes = [ 'smart', 'pretty' ];

model.schema.extend( '$block', {
allowAttributes: attributes
} );

model.schema.extend( '$blockObject', {
allowAttributes: attributes
} );

for ( const attribute of attributes ) {
model.schema.setAttributeProperties( attribute, {
copyOnReplace: true
} );
}
} );

it( 'should copy parent block attributes to image block', () => {
setModelData( model, `<paragraph pretty="true" smart="true">[<imageInline src="${ imgSrc }"></imageInline>]</paragraph>` );

blockCommand.execute();

expect( getModelData( model ) ).to.equal( `[<imageBlock pretty="true" smart="true" src="${ imgSrc }"></imageBlock>]` );
} );

it( 'should copy a block image attributes to an inline image\'s parent block', () => {
setModelData( model, `[<imageBlock pretty="true" smart="true" src="${ imgSrc }"></imageBlock>]` );

inlineCommand.execute();

expect( getModelData( model ) ).to.equal(
'<paragraph pretty="true" smart="true">' +
`[<imageInline src="${ imgSrc }"></imageInline>]` +
'</paragraph>' );
} );
} );
} );
} );
Loading