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

Commit

Permalink
Code refac. Deobfuscated some pieces of code. Allowed custom title fo…
Browse files Browse the repository at this point in the history
…r paragraph option. Enhanced tests.
  • Loading branch information
oleq committed Mar 10, 2017
1 parent 7b2c1c8 commit 65ac56c
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 68 deletions.
14 changes: 4 additions & 10 deletions src/heading.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export default class Heading extends Plugin {
for ( let option of options ) {
// Add the option to the collection.
dropdownItems.add( new Model( {
modelElement: option.modelElement,
commandName: option.modelElement,
label: option.title
} ) );

Expand Down Expand Up @@ -82,8 +82,8 @@ export default class Heading extends Plugin {
const dropdown = createListDropdown( dropdownModel, locale );

// Execute command when an item from the dropdown is selected.
this.listenTo( dropdown, 'execute', ( { source: { modelElement } } ) => {
editor.execute( modelElement );
this.listenTo( dropdown, 'execute', ( evt ) => {
editor.execute( evt.source.commandName );
editor.editing.view.focus();
} );

Expand Down Expand Up @@ -113,13 +113,7 @@ export default class Heading extends Plugin {
};

return editor.config.get( 'heading.options' ).map( option => {
let title;

if ( option.modelElement == 'paragraph' ) {
title = localizedTitles.Paragraph;
} else {
title = localizedTitles[ option.title ];
}
const title = localizedTitles[ option.title ];

if ( title && title != option.title ) {
// Clone the option to avoid altering the original `config.heading.options`.
Expand Down
21 changes: 7 additions & 14 deletions src/headingcommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,38 +77,31 @@ export default class HeadingCommand extends Command {
_doExecute( options = {} ) {
const editor = this.editor;
const document = editor.document;
const selection = document.selection;
const ranges = [ ...selection.getRanges() ];
const isSelectionBackward = selection.isBackward;

// If current option is same as new option - toggle already applied option back to default one.
const shouldRemove = this.value;

document.enqueueChanges( () => {
const batch = options.batch || document.batch();

for ( let element of document.selection.getSelectedBlocks() ) {
for ( let block of document.selection.getSelectedBlocks() ) {
// When removing applied option.
if ( shouldRemove ) {
if ( element.name === this.modelElement ) {
// Apply paragraph to the selection withing that particular element only instead
if ( block.is( this.modelElement ) ) {
// Apply paragraph to the selection withing that particular block only instead
// of working on the entire document selection.
const selection = new Selection();
selection.addRange( Range.createIn( element ) );
selection.addRange( Range.createIn( block ) );

// Share the batch with the paragraph command.
editor.execute( 'paragraph', { selection, batch } );
}
}
// When applying new option.
else {
batch.rename( element, this.modelElement );
else if ( !block.is( this.modelElement ) ) {
batch.rename( block, this.modelElement );
}
}

// If range's selection start/end is placed directly in renamed block - we need to restore it's position
// after renaming, because renaming puts new element there.
selection.setRanges( ranges, isSelectionBackward );
} );
}

Expand All @@ -121,7 +114,7 @@ export default class HeadingCommand extends Command {
const block = this.editor.document.selection.getSelectedBlocks().next().value;

if ( block ) {
this.value = this.modelElement == block.name;
this.value = block.is( this.modelElement );
}
}
}
Expand Down
10 changes: 4 additions & 6 deletions src/headingengine.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export default class HeadingEngine extends Plugin {
// more properties (https://github.com/ckeditor/ckeditor5/issues/403).
editor.config.define( 'heading', {
options: [
{ modelElement: 'paragraph' },
{ modelElement: 'paragraph', title: 'Paragraph' },
{ modelElement: 'heading1', viewElement: 'h2', title: 'Heading 1' },
{ modelElement: 'heading2', viewElement: 'h3', title: 'Heading 2' },
{ modelElement: 'heading3', viewElement: 'h4', title: 'Heading 3' }
Expand All @@ -55,7 +55,6 @@ export default class HeadingEngine extends Plugin {
const data = editor.data;
const editing = editor.editing;
const options = editor.config.get( 'heading.options' );
let command;

for ( let option of options ) {
// Skip paragraph - it is defined in required Paragraph feature.
Expand All @@ -74,8 +73,7 @@ export default class HeadingEngine extends Plugin {
.toElement( option.modelElement );

// Register the heading command for this option.
command = new HeadingCommand( editor, option );
editor.commands.set( command.modelElement, command );
editor.commands.set( option.modelElement, new HeadingCommand( editor, option ) );
}
}
}
Expand All @@ -94,9 +92,9 @@ export default class HeadingEngine extends Plugin {
this.listenTo( enterCommand, 'afterExecute', ( evt, data ) => {
const positionParent = editor.document.selection.getFirstPosition().parent;
const batch = data.batch;
const isHeading = options.some( option => option.modelElement == positionParent.name );
const isHeading = options.some( option => positionParent.is( option.modelElement ) );

if ( isHeading && positionParent.name != defaultModelElement && positionParent.childCount === 0 ) {
if ( isHeading && !positionParent.is( defaultModelElement ) && positionParent.childCount === 0 ) {
batch.rename( positionParent, defaultModelElement );
}
} );
Expand Down
84 changes: 57 additions & 27 deletions tests/heading.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe( 'Heading', () => {
const executeSpy = testUtils.sinon.spy( editor, 'execute' );
const dropdown = editor.ui.componentFactory.create( 'headings' );

dropdown.modelElement = 'paragraph';
dropdown.commandName = 'paragraph';
dropdown.fire( 'execute' );

sinon.assert.calledOnce( executeSpy );
Expand All @@ -74,7 +74,7 @@ describe( 'Heading', () => {
const focusSpy = testUtils.sinon.spy( editor.editing.view, 'focus' );
const dropdown = editor.ui.componentFactory.create( 'headings' );

dropdown.modelElement = 'paragraph';
dropdown.commandName = 'paragraph';
dropdown.fire( 'execute' );

sinon.assert.calledOnce( focusSpy );
Expand Down Expand Up @@ -118,35 +118,18 @@ describe( 'Heading', () => {
let commands;

beforeEach( () => {
const editorElement = document.createElement( 'div' );

return ClassicTestEditor.create( editorElement, {
plugins: [ Heading ],
toolbar: [ 'heading' ],
lang: 'pl',
heading: {
options: [
{ modelElement: 'paragraph', viewElement: 'p', title: 'Paragraph' },
{ modelElement: 'heading1', viewElement: 'h2', title: 'Heading 1' },
{ modelElement: 'heading2', viewElement: 'h3', title: 'Not automatically localized' }
]
}
} )
.then( newEditor => {
editor = newEditor;
dropdown = editor.ui.componentFactory.create( 'headings' );
commands = {};
editor.config.get( 'heading.options' ).forEach( ( { modelElement } ) => {
commands[ modelElement ] = editor.commands.get( modelElement );
} );
} );
return localizedEditor( [
{ modelElement: 'paragraph', title: 'Paragraph' },
{ modelElement: 'heading1', viewElement: 'h2', title: 'Heading 1' },
{ modelElement: 'heading2', viewElement: 'h3', title: 'Heading 2' }
] );
} );

it( 'does not alter the original config', () => {
expect( editor.config.get( 'heading.options' ) ).to.deep.equal( [
{ modelElement: 'paragraph', viewElement: 'p', title: 'Paragraph' },
{ modelElement: 'paragraph', title: 'Paragraph' },
{ modelElement: 'heading1', viewElement: 'h2', title: 'Heading 1' },
{ modelElement: 'heading2', viewElement: 'h3', title: 'Not automatically localized' }
{ modelElement: 'heading2', viewElement: 'h3', title: 'Heading 2' }
] );
} );

Expand All @@ -164,9 +147,56 @@ describe( 'Heading', () => {
expect( listView.items.map( item => item.label ) ).to.deep.equal( [
'Akapit',
'Nagłówek 1',
'Not automatically localized'
'Nagłówek 2'
] );
} );

it( 'allows custom titles', () => {
return localizedEditor( [
{ modelElement: 'paragraph', title: 'Custom paragraph title' },
{ modelElement: 'heading1', title: 'Custom heading1 title' }
] ).then( () => {
const listView = dropdown.listView;

expect( listView.items.map( item => item.label ) ).to.deep.equal( [
'Custom paragraph title',
'Custom heading1 title',
] );
} );
} );

it( 'translates default using the the locale', () => {
return localizedEditor( [
{ modelElement: 'paragraph', title: 'Paragraph' }
] ).then( () => {
const listView = dropdown.listView;

expect( listView.items.map( item => item.label ) ).to.deep.equal( [
'Akapit'
] );
} );
} );

function localizedEditor( options ) {
const editorElement = document.createElement( 'div' );

return ClassicTestEditor.create( editorElement, {
plugins: [ Heading ],
toolbar: [ 'heading' ],
lang: 'pl',
heading: {
options: options
}
} )
.then( newEditor => {
editor = newEditor;
dropdown = editor.ui.componentFactory.create( 'headings' );
commands = {};
editor.config.get( 'heading.options' ).forEach( ( { modelElement } ) => {
commands[ modelElement ] = editor.commands.get( modelElement );
} );
} );
}
} );
} );
} );
2 changes: 1 addition & 1 deletion tests/headingcommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ describe( 'HeadingCommand', () => {
}

it( 'converts all elements where selection is applied', () => {
setData( document, '<heading1>foo[</heading1><heading2>bar</heading2><heading2>]baz</heading2>' );
setData( document, '<heading1>foo[</heading1><heading2>bar</heading2><heading3>]baz</heading3>' );
commands.heading3._doExecute();

expect( getData( document ) ).to.equal(
Expand Down
12 changes: 2 additions & 10 deletions tests/headingengine.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,8 @@ 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 Enter from '@ckeditor/ckeditor5-enter/src/enter';
import { add } from '@ckeditor/ckeditor5-utils/src/translation-service';
import { getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';

add( 'pl', {
'Paragraph': 'Akapit',
'Heading 1': 'Nagłówek 1',
'Heading 2': 'Nagłówek 2',
'Heading 3': 'Nagłówek 3',
} );

describe( 'HeadingEngine', () => {
let editor, document;

Expand Down Expand Up @@ -113,7 +105,7 @@ describe( 'HeadingEngine', () => {
describe( 'default value', () => {
it( 'should be set', () => {
expect( editor.config.get( 'heading.options' ) ).to.deep.equal( [
{ modelElement: 'paragraph' },
{ modelElement: 'paragraph', title: 'Paragraph' },
{ modelElement: 'heading1', viewElement: 'h2', title: 'Heading 1' },
{ modelElement: 'heading2', viewElement: 'h3', title: 'Heading 2' },
{ modelElement: 'heading3', viewElement: 'h4', title: 'Heading 3' }
Expand All @@ -123,7 +115,7 @@ describe( 'HeadingEngine', () => {

it( 'should customize options', () => {
const options = [
{ modelElement: 'paragraph', viewElement: 'p', title: 'Paragraph' },
{ modelElement: 'paragraph', title: 'Paragraph' },
{ modelElement: 'h4', viewElement: 'h4', title: 'H4' }
];

Expand Down

0 comments on commit 65ac56c

Please sign in to comment.