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

Commit

Permalink
Merge pull request #114 from ckeditor/t/98
Browse files Browse the repository at this point in the history
Other: The `<h1>` elements are now converted to `<heading1>` elements instead of being converted to `<paragraph>`s by default. Closes #98. Closes ckeditor/ckeditor5-paste-from-office#2.
  • Loading branch information
Reinmar committed Oct 30, 2018
2 parents a4b1a93 + 1bd3f38 commit c49b573
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 2 deletions.
4 changes: 3 additions & 1 deletion docs/features/headings.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ The {@link module:heading/heading~Heading} feature enables support for headings.

## Heading levels

By default this feature is configured to support `<h2>`, `<h3>` and `<h4>` elements which are named: "Heading 1", "Heading 2" and "Heading 3", respectively. The rationale behind starting from `<h2>` is that `<h1>` should be reserved for the page's main title and the page content will usually start from `<h2>`.
By default this feature is configured to support `<h2>`, `<h3>` and `<h4>` elements which are named: "Heading 1", "Heading 2" and "Heading 3", respectively. Additionally, the `<h1>` element is supported and is converted to `<h2>` ("Heading 1") with a low priority by default.

The rationale behind such behaviour is that `<h1>` should be reserved for the page's main title and the page content will usually start from `<h2>`. However, when content with `<h1>` elements is used in the editor (set or pasted) it makes more sense from the user perspective and content semantics to convert `<h1>` elements into `<h2>` instead of paragraphs (`<p>`).

<info-box hint>
You can read more about why the editor should not create `<h1>` elements in the [Headings section of Editor Recommendations](http://ckeditor.github.io/editor-recommendations/features/headings.html).
Expand Down
21 changes: 21 additions & 0 deletions src/headingediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import HeadingCommand from './headingcommand';

import priorities from '@ckeditor/ckeditor5-utils/src/priorities';
import { upcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/upcast-converters';

const defaultModelElement = 'paragraph';

/**
Expand Down Expand Up @@ -67,6 +70,8 @@ export default class HeadingEditing extends Plugin {
}
}

this._addDefaultH1Conversion( editor );

// Register the heading command for this option.
editor.commands.add( 'heading', new HeadingCommand( editor, modelElements ) );
}
Expand All @@ -92,4 +97,20 @@ export default class HeadingEditing extends Plugin {
} );
}
}

/**
* Adds default conversion for `h1` -> `heading1` with a low priority.
*
* @private
* @param {module:core/editor/editor~Editor} editor Editor instance on which to add the `h1` conversion.
*/
_addDefaultH1Conversion( editor ) {
editor.conversion.for( 'upcast' ).add( upcastElementToElement( {
model: 'heading1',
view: 'h1',
// With a `low` priority, `paragraph` plugin autoparagraphing mechanism is executed. Make sure
// this listener is called before it. If not, `h1` will be transformed into a paragraph.
converterPriority: priorities.get( 'low' ) + 1
} ) );
}
}
85 changes: 85 additions & 0 deletions tests/headingediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import HeadingCommand from '../src/headingcommand';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import ParagraphCommand from '@ckeditor/ckeditor5-paragraph/src/paragraphcommand';
import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
import { getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';

describe( 'HeadingEditing', () => {
Expand Down Expand Up @@ -73,6 +74,13 @@ describe( 'HeadingEditing', () => {
expect( editor.getData() ).to.equal( '<h4>foobar</h4>' );
} );

it( 'should convert h1 to heading1 using default, low-priority converter', () => {
editor.setData( '<h1>foobar</h1>' );

expect( getData( model, { withoutSelection: true } ) ).to.equal( '<heading1>foobar</heading1>' );
expect( editor.getData() ).to.equal( '<h2>foobar</h2>' );
} );

describe( 'user defined', () => {
beforeEach( () => {
return VirtualTestEditor
Expand Down Expand Up @@ -116,6 +124,83 @@ describe( 'HeadingEditing', () => {
} );
} );

describe( 'default h1 conversion', () => {
let addDefaultConversionSpy;

testUtils.createSinonSandbox();

beforeEach( () => {
addDefaultConversionSpy = testUtils.sinon.spy( HeadingEditing.prototype, '_addDefaultH1Conversion' );
} );

it( 'should define the default h1 to heading1 converter' +
'when heading.options is not specified and apply it during conversions', () => {
return VirtualTestEditor
.create( {
plugins: [ HeadingEditing ]
} )
.then( editor => {
expect( addDefaultConversionSpy.callCount ).to.equal( 1 );

editor.setData( '<h1>Foo</h1><h2>Bar</h2><p>Baz</p>' );

expect( getData( editor.model, { withoutSelection: true } ) )
.to.equal( '<heading1>Foo</heading1><heading1>Bar</heading1><paragraph>Baz</paragraph>' );

expect( editor.getData() ).to.equal( '<h2>Foo</h2><h2>Bar</h2><p>Baz</p>' );
} );
} );

it( 'should define the default h1 to heading1 converter' +
'when heading.options is specified and apply it during conversions', () => {
const options = [
{ model: 'heading1', view: 'h3' },
{ model: 'heading2', view: 'h4' }
];

return VirtualTestEditor
.create( {
plugins: [ HeadingEditing ],
heading: { options }
} )
.then( editor => {
expect( addDefaultConversionSpy.callCount ).to.equal( 1 );

editor.setData( '<h1>Foo</h1><h3>Bar</h3><h4>Baz</h4><h2>Bax</h2>' );

expect( getData( editor.model, { withoutSelection: true } ) )
.to.equal( '<heading1>Foo</heading1><heading1>Bar</heading1><heading2>Baz</heading2><paragraph>Bax</paragraph>' );

expect( editor.getData() ).to.equal( '<h3>Foo</h3><h3>Bar</h3><h4>Baz</h4><p>Bax</p>' );
} );
} );

it( 'should define the default h1 to heading1 converter' +
'when heading.options is specified with h1 but not apply it during conversions', () => {
const options = [
{ model: 'heading1', view: 'h2' },
{ model: 'heading2', view: 'h1' },
{ model: 'heading3', view: 'h3' }
];

return VirtualTestEditor
.create( {
plugins: [ HeadingEditing ],
heading: { options }
} )
.then( editor => {
expect( addDefaultConversionSpy.callCount ).to.equal( 1 );

editor.setData( '<h1>Foo</h1><h2>Bar</h2><h3>Baz</h3><h4>Bax</h4>' );

expect( getData( editor.model, { withoutSelection: true } ) )
.to.equal( '<heading2>Foo</heading2><heading1>Bar</heading1><heading3>Baz</heading3><paragraph>Bax</paragraph>' );

expect( editor.getData() ).to.equal( '<h1>Foo</h1><h2>Bar</h2><h3>Baz</h3><p>Bax</p>' );
} );
} );
} );

it( 'should not blow up if there\'s no enter command in the editor', () => {
return VirtualTestEditor
.create( {
Expand Down
3 changes: 2 additions & 1 deletion tests/manual/heading.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<div id="editor">
<h2>Heading 1</h2>
<h1>Heading 1 (h1)</h1>
<h2>Heading 1 (h2)</h2>
<h3>Heading 2</h3>
<h4>Heading 3</h4>
<p>Paragraph</p>
Expand Down

0 comments on commit c49b573

Please sign in to comment.