Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Added objects handling to modifySelection method #709

Merged
merged 2 commits into from
Dec 1, 2016
Merged

Added objects handling to modifySelection method #709

merged 2 commits into from
Dec 1, 2016

Conversation

szymonkups
Copy link
Contributor

@szymonkups szymonkups commented Nov 30, 2016

document.schema.registerItem( 'p', '$block' );
document.schema.registerItem( 'x', '$block' );
document.schema.registerItem( 'img', '$inline' );
document.schema.registerItem( 'b', '$inline' );
Copy link
Member

Choose a reason for hiding this comment

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

This is confusing. We don't have inline containers in the model. <b> should rather be an attribute. This test:

<p>foo<b>[]</b><inlineObj>bar</inlineObj></p>

have no sense. In the paragraph you can either have text or inline objects.

Copy link
Member

Choose a reason for hiding this comment

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

BTW. It's better to have many schema definitions – one per some describe() block. It's then easier to remember all setting for each block. E.g. you can move the additional setting to the newly added describe() (and keep the rest here for all tests).

describe( 'objects handling', () => {
test(
'extends over next object element when at the end of an element',
'<p>foo[]</p><obj>bar</obj>',
Copy link
Member

Choose a reason for hiding this comment

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

I miss a test case for <blockquote><p>foo[]</p></blockquote><obj></obj>.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This looks like a bigger case and it will be handled separately.

Copy link
Member

@Reinmar Reinmar left a comment

Choose a reason for hiding this comment

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

I left 3 comments.

@Reinmar Reinmar merged commit 24c9fac into master Dec 1, 2016
@Reinmar Reinmar deleted the t/708 branch December 1, 2016 12:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants