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

T/1: Initial implementation of Highlight editing without UI. #2

Merged
merged 20 commits into from
Nov 10, 2017
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
7e095b4
Other: Bootstrap highlight feature classes.
jodator Nov 6, 2017
60285c7
Other: Implement HighlightCommand value and isEnabled properties calc…
jodator Nov 6, 2017
8a2e63d
Other: Initial implementation of `HighlightCommand#execute` method.
jodator Nov 6, 2017
cfa059d
Other: Do nothing on collapsed ranges in `HighlightCommand#exectue()`.
jodator Nov 6, 2017
351c192
Other: Provide configuration stub.
jodator Nov 6, 2017
23adf92
Tests: Fix paths in test files.
jodator Nov 6, 2017
c167c13
Tests: Add manual tests stub.
jodator Nov 6, 2017
c284402
Other: Define highlight feature conversions.
jodator Nov 6, 2017
4f69319
Tests: Remove unknown button from toolbar in manual tests.
jodator Nov 6, 2017
8ec55f8
Other: Simplify model converter.
jodator Nov 7, 2017
b6a3970
Docs: Update documentation.
jodator Nov 7, 2017
82d70ad
Tests: Update highlight manual tests.
jodator Nov 7, 2017
31d547f
Tests: Add some integration tests and remove skipped.
jodator Nov 7, 2017
45716e5
Other: Add missing dev dependencies.
jodator Nov 7, 2017
cbc2927
Other: Move configuration definition to constructor.
jodator Nov 9, 2017
c9c102a
Docs: Fix documentation links and highlighters description.
jodator Nov 9, 2017
7de95c6
Other: Add workaround to allow highlighters in clipboard holder.
jodator Nov 9, 2017
9fc0670
Other: Simplify toAttribute converter method.
jodator Nov 9, 2017
b18525f
Other: Save precious ms by using generator directly.
jodator Nov 9, 2017
dfca70c
Added command execution examples to manual test.
szymonkups Nov 10, 2017
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
10 changes: 10 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,18 @@
"ckeditor5-feature"
],
"dependencies": {
"@ckeditor/ckeditor5-core": "^1.0.0-alpha.1",
"@ckeditor/ckeditor5-engine": "^1.0.0-alpha.1",
"@ckeditor/ckeditor5-ui": "^1.0.0-alpha.1"
},
"devDependencies": {
"@ckeditor/ckeditor5-block-quote": "^1.0.0-alpha.1",
"@ckeditor/ckeditor5-enter": "^1.0.0-alpha.1",
"@ckeditor/ckeditor5-heading": "^1.0.0-alpha.1",
"@ckeditor/ckeditor5-image": "^1.0.0-alpha.1",
"@ckeditor/ckeditor5-list": "^1.0.0-alpha.1",
"@ckeditor/ckeditor5-paragraph": "^1.0.0-alpha.1",
"@ckeditor/ckeditor5-typing": "^1.0.0-alpha.1",
"eslint": "^4.8.0",
"eslint-config-ckeditor5": "^1.0.6",
"husky": "^0.14.3",
Expand Down
36 changes: 36 additions & 0 deletions src/highlight.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/**
* @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

/**
* @module highlight/highlight
*/

import Plugin from '@ckeditor/ckeditor5-core/src/plugin';

import HighlightEditing from './highlightediting';
import HighlightUI from './highlightui';

/**
* The highlight plugin.
*
* It requires {@link module:highlight/highlightediting~HighlightEditing} and {@link module:highlight/highlightui~HighlightUI} plugins.
*
* @extends module:core/plugin~Plugin
*/
export default class Highlight extends Plugin {
/**
* @inheritDoc
*/
static get requires() {
return [ HighlightEditing, HighlightUI ];
}

/**
* @inheritDoc
*/
static get pluginName() {
return 'Highlight';
}
}
68 changes: 68 additions & 0 deletions src/highlightcommand.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/**
* @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

/**
* @module highlight/highlightcommand
*/

import Command from '@ckeditor/ckeditor5-core/src/command';

/**
* The highlight command. It is used by the {@link module:highlight/highlight~HighlightEditing highlight feature}
Copy link
Contributor

Choose a reason for hiding this comment

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

Gives some docs errors, probably wrong module in @link.

* to apply text highlighting.
*
* @extends module:core/command~Command
*/
export default class HighlightCommand extends Command {
/**
* @inheritDoc
*/
refresh() {
const doc = this.editor.document;

this.value = doc.selection.getAttribute( 'highlight' );
this.isEnabled = doc.schema.checkAttributeInSelection( doc.selection, 'highlight' );
}

/**
* Executes the command.
*
* @protected
* @param {Object} [options] Options for the executed command.
* @param {String} options.class Name of highlighter class.
* @param {module:engine/model/batch~Batch} [options.batch] A batch to collect all the change steps.
* A new batch will be created if this option is not set.
*/
execute( options = {} ) {
const doc = this.editor.document;
const selection = doc.selection;

// Do not apply highlight no collapsed selection.
if ( selection.isCollapsed ) {
return;
}

doc.enqueueChanges( () => {
const ranges = doc.schema.getValidRanges( selection.getRanges(), 'highlight' );
const batch = options.batch || doc.batch();

for ( const range of ranges ) {
if ( options.class ) {
batch.setAttribute( range, 'highlight', options.class );
} else {
batch.removeAttribute( range, 'highlight' );
}
}
} );
}
}

/**
* Holds current highlight class. If there is no highlight in selection then value will be undefined.
*
* @observable
* @readonly
* @member {undefined|String} HighlightCommand#value
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

This gives some docs errors - it should be moved probably to the class constructor or use full module name.

121 changes: 121 additions & 0 deletions src/highlightediting.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
/**
* @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

/**
* @module highlight/highlightediting
*/

import Plugin from '@ckeditor/ckeditor5-core/src/plugin';

import buildViewConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildviewconverter';
import buildModelConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildmodelconverter';

import AttributeElement from '@ckeditor/ckeditor5-engine/src/view/attributeelement';

import HighlightCommand from './highlightcommand';

/**
* The highlight editing feature. It introduces `highlight` command which allow to highlight selected text with defined 'marker' or 'pen'.
*
* @extends module:core/plugin~Plugin
*/
export default class HighlightEditing extends Plugin {
/**
* @inheritDoc
*/
init() {
const editor = this.editor;
const data = editor.data;
const editing = editor.editing;

editor.config.define( 'highlight', [
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be defined inside the class constructor?

{ class: 'marker', title: 'Marker', color: '#ffff66', type: 'marker' },
{ class: 'marker-green', title: 'Green Marker', color: '#66ff00', type: 'marker' },
{ class: 'marker-pink', title: 'Pink Marker', color: '#ff6fff', type: 'marker' },
{ class: 'pen-red', title: 'Red Pen', color: '#ff0000', type: 'pen' },
{ class: 'pen-blue', title: 'Blue Pen', color: '#0000ff', type: 'pen' }
] );

// Allow highlight attribute on all elements
editor.document.schema.allow( { name: '$inline', attributes: 'highlight', inside: '$block' } );
Copy link
Contributor

@szymonkups szymonkups Nov 9, 2017

Choose a reason for hiding this comment

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

Basic styles contains this workaround. It probably should be included here as well.


// Convert highlight attribute to a mark element with associated class.
buildModelConverter()
.for( data.modelToView, editing.modelToView )
.fromAttribute( 'highlight' )
.toElement( data => new AttributeElement( 'mark', { class: data } ) );

const configuredClasses = editor.config.get( 'highlight' ).map( config => config.class );

// Convert `mark` attribute with class name to model's highlight attribute.
buildViewConverter()
.for( data.viewToModel )
.fromElement( 'mark' )
.toAttribute( viewElement => {
const viewClassNames = [ ...viewElement.getClassNames() ];
Copy link
Contributor

Choose a reason for hiding this comment

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

This function could be simplified to:

const viewClassNames = [ ...viewElement.getClassNames() ];

for( const className of viewClassNames ) {
	if ( configuredClasses.indexOf( className ) > -1 ) {
		return { key: 'highlight', value: className };
	}
}

Copy link

Choose a reason for hiding this comment

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

Writing:

for( const className of viewElement.getClassNames() ) {}

We'd take advantage of generators and save extra ms ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ma2ciek 👏 yeah... I've disabled thinking apparently :D


if ( !viewClassNames.length ) {
return;
}

const highlightClassNames = viewClassNames.filter( className => configuredClasses.includes( className ) );

if ( !highlightClassNames.length ) {
return;
}

return { key: 'highlight', value: highlightClassNames[ 0 ] };
} );

editor.commands.add( 'highlight', new HighlightCommand( editor ) );
}
}

/**
* Highlight option descriptor.
*
* @typedef {Object} module:highlight/highlightediting~HeadingOption
* @property {String} class The class which is used to differentiate highlighters.
* @property {String} title The user-readable title of the option.
* @property {String} color Color used for highlighter. Should be coherent with CSS class definition.
* @property {'marker'|'pen'} type The type of highlighter. Either "marker" - will use #color as background name
* of the view element that will be used to represent the model element in the view.
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like pen option description is missing.

*/

/**
* The configuration of the {@link module:highlight/highlightediting~HighlightEditing Highlight feature}.
*
* Read more in {@link module:highlight/highlightediting~HighlightEditingConfig}.
*
* @member {module:highlight/highlightediting~HighlightEditingConfig} module:core/editor/editorconfig~EditorConfig#alignment
Copy link
Contributor

Choose a reason for hiding this comment

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

alignment => highlight

*/

/**
* The configuration of the {@link module:highlight/highlightediting~HighlightEditing Highlight feature}.
*
* ClassicEditor
* .create( editorElement, {
* highlight: ... // Highlight feature config.
* } )
* .then( ... )
* .catch( ... );
*
* See {@link module:core/editor/editorconfig~EditorConfig all editor options}.
*
* @interface HighlightEditingConfig
*/

/**
* Available highlighters options.
*
* There are two types of highlighters:
* - 'marker' - rendered as `<mark>` element with defined background color.
* - 'pen' - rendered as `<mark>` element with defined foreground (font) color.
*
* Note: Each highlighter must have it's own CSS class defined to properly match content data. Also it is advised
* that color value should match the values defined in content CSS stylesheet.
*
* @member {Array.<module:heading/heading~HeadingOption>} module:heading/heading~HeadingConfig#options
*/
33 changes: 33 additions & 0 deletions src/highlightui.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/**
* @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

/**
* @module highlight/highlightui
*/

import Plugin from '@ckeditor/ckeditor5-core/src/plugin';

import HighlightEditing from './highlightediting';

/**
* The default Highlight UI plugin.
*
* @extends module:core/plugin~Plugin
*/
export default class HighlightUI extends Plugin {
/**
* @inheritDoc
*/
static get requires() {
return [ HighlightEditing ];
}

/**
* @inheritDoc
*/
static get pluginName() {
return 'HighlightUI';
}
}
39 changes: 39 additions & 0 deletions tests/highlight.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

/* global document */

import Highlight from './../src/highlight';
import HighlightEditing from './../src/highlightediting';
import HighlightUI from './../src/highlightui';

import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';

describe( 'Highlight', () => {
let editor, element;

beforeEach( () => {
element = document.createElement( 'div' );
document.body.appendChild( element );

return ClassicTestEditor
.create( element, {
plugins: [ Highlight ]
} )
.then( newEditor => {
editor = newEditor;
} );
} );

afterEach( () => {
element.remove();

return editor.destroy();
} );

it( 'requires HighlightEditing and HighlightUI', () => {
expect( Highlight.requires ).to.deep.equal( [ HighlightEditing, HighlightUI ] );
} );
} );
Loading