-
Notifications
You must be signed in to change notification settings - Fork 9
Add the strikethrough feature. #58
Changes from 11 commits
5de488a
a522c17
b5b19ca
8fc5b99
18fb556
02cc56e
890c792
ae47ebc
cf3ba7e
b770d65
1def162
bfda441
4312bc7
0bd0a2d
6909875
69e6a79
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
/** | ||
* @license Copyright (c) 2017, CKSource - Rémy Hubscher. All rights reserved. | ||
* For licensing, see LICENSE.md. | ||
*/ | ||
|
||
/** | ||
* @module basic-styles/strikethrough | ||
*/ | ||
|
||
import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; | ||
import StrikethroughEngine from './strikethroughengine'; | ||
import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; | ||
import strikethroughIcon from '../theme/icons/strikethrough.svg'; | ||
|
||
/** | ||
* The strikethrough feature. It introduces the Strikethrough button and the <kbd>Ctrl+Shift+X</kbd> keystroke. | ||
* | ||
* It uses the {@link module:basic-styles/strikethroughengine~StrikethroughEngine strikethrough engine feature}. | ||
* | ||
* @extends module:core/plugin~Plugin | ||
*/ | ||
export default class Strikethrough extends Plugin { | ||
/** | ||
* @inheritDoc | ||
*/ | ||
static get requires() { | ||
return [ StrikethroughEngine ]; | ||
} | ||
|
||
/** | ||
* @inheritDoc | ||
*/ | ||
static get pluginName() { | ||
return 'Strikethrough'; | ||
} | ||
|
||
/** | ||
* @inheritDoc | ||
*/ | ||
init() { | ||
const editor = this.editor; | ||
const t = editor.t; | ||
const command = editor.commands.get( 'strikethrough' ); | ||
const keystroke = 'CTRL+SHIFT+X'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. """ From https://www.w3.org/International/questions/qa-html-dir We somehow got into this issue here: mozilla/notes#409 |
||
|
||
// Add strikethrough button to feature components. | ||
editor.ui.componentFactory.add( 'strikethrough', locale => { | ||
const view = new ButtonView( locale ); | ||
|
||
view.set( { | ||
label: t( 'Strikethrough' ), | ||
icon: strikethroughIcon, | ||
keystroke, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @vladikoff I check that out and I found out it's a bug https://github.com/ckeditor/ckeditor5-utils/issues/206. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @szymonkups Could you check if ckeditor/ckeditor5-utils#207 fixes the issue? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @oleq - checked, tested and merged :) |
||
tooltip: true | ||
} ); | ||
|
||
view.bind( 'isOn', 'isEnabled' ).to( command, 'value', 'isEnabled' ); | ||
|
||
// Execute command. | ||
this.listenTo( view, 'execute', () => editor.execute( 'strikethrough' ) ); | ||
|
||
return view; | ||
} ); | ||
|
||
// Set the Ctrl+Shift+X keystroke. | ||
editor.keystrokes.set( keystroke, 'strikethrough' ); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
/** | ||
* @license Copyright (c) 2017, CKSource - Rémy Hubscher. All rights reserved. | ||
* For licensing, see LICENSE.md. | ||
*/ | ||
|
||
/** | ||
* @module basic-styles/strikeengine | ||
*/ | ||
|
||
import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; | ||
import buildModelConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildmodelconverter'; | ||
import buildViewConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildviewconverter'; | ||
import AttributeCommand from './attributecommand'; | ||
|
||
const STRIKETHROUGH = 'strikethrough'; | ||
|
||
/** | ||
* The strikethrough engine feature. | ||
* | ||
* It registers the `strikethrough` command and introduces the | ||
* `strikethroughsthrough` attribute in the model which renders to the view | ||
* as a `<s>` element. | ||
* | ||
* @extends module:core/plugin~Plugin | ||
*/ | ||
export default class StrikethroughEngine extends Plugin { | ||
/** | ||
* @inheritDoc | ||
*/ | ||
init() { | ||
const editor = this.editor; | ||
const data = editor.data; | ||
const editing = editor.editing; | ||
|
||
// Allow strikethrough attribute on all inline nodes. | ||
editor.document.schema.allow( { name: '$inline', attributes: STRIKETHROUGH, inside: '$block' } ); | ||
// Temporary workaround. See https://github.com/ckeditor/ckeditor5/issues/477. | ||
editor.document.schema.allow( { name: '$inline', attributes: STRIKETHROUGH, inside: '$clipboardHolder' } ); | ||
|
||
// Build converter from model to view for data and editing pipelines. | ||
buildModelConverter().for( data.modelToView, editing.modelToView ) | ||
.fromAttribute( STRIKETHROUGH ) | ||
.toElement( 's' ); | ||
|
||
// Build converter from view to model for data pipeline. | ||
buildViewConverter().for( data.viewToModel ) | ||
.fromElement( 's' ) | ||
.fromElement( 'del' ) | ||
.fromElement( 'strikethrough' ) | ||
.fromAttribute( 'style', { 'text-decoration': 'line-through' } ) | ||
.toAttribute( STRIKETHROUGH, true ); | ||
|
||
// Create strikethrough command. | ||
editor.commands.add( STRIKETHROUGH, new AttributeCommand( editor, STRIKETHROUGH ) ); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
<div id="editor"> | ||
<p><i>This</i> <code>is an</code> <strong>editor</strong> <u>instance</u>.</p> | ||
<p><i>This</i> <s>is</s> <code>an</code> <strong>editor</strong> <u>instance</u>.</p> | ||
</div> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,9 @@ | ||
## Basic styles | ||
|
||
1. The data should be loaded with: | ||
* italic "This", | ||
* bold "editor", | ||
* italic _"This"_, | ||
* bold **"editor"**, | ||
* underline "instance", | ||
* code "is an". | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should also update There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
2. Test the bold, italic, underline and code features live. | ||
* strikethrough ~~"is"~~, | ||
* code `"an"`. | ||
2. Test the bold, italic, strikethrough, underline and code features live. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
/** | ||
* @license Copyright (c) 2003-2017, CKSource - Rémy Hubscher. All rights reserved. | ||
* For licensing, see LICENSE.md. | ||
*/ | ||
|
||
/* globals document */ | ||
|
||
import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; | ||
import Strikethrough from '../src/strike'; | ||
import StrikethroughEngine from '../src/strikeengine'; | ||
import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; | ||
import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; | ||
import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard'; | ||
|
||
testUtils.createSinonSandbox(); | ||
|
||
describe( 'Strikethrough', () => { | ||
let editor, strikeView; | ||
|
||
beforeEach( () => { | ||
const editorElement = document.createElement( 'div' ); | ||
document.body.appendChild( editorElement ); | ||
|
||
return ClassicTestEditor | ||
.create( editorElement, { | ||
plugins: [ Strikethrough ] | ||
} ) | ||
.then( newEditor => { | ||
editor = newEditor; | ||
|
||
strikeView = editor.ui.componentFactory.create( 'strike' ); | ||
} ); | ||
} ); | ||
|
||
afterEach( () => { | ||
return editor.destroy(); | ||
} ); | ||
|
||
it( 'should be loaded', () => { | ||
expect( editor.plugins.get( Strikethrough ) ).to.be.instanceOf( Strikethrough ); | ||
} ); | ||
|
||
it( 'should load StrikethroughEngine', () => { | ||
expect( editor.plugins.get( StrikethroughEngine ) ).to.be.instanceOf( StrikethroughEngine ); | ||
} ); | ||
|
||
it( 'should register strike feature component', () => { | ||
expect( strikeView ).to.be.instanceOf( ButtonView ); | ||
expect( strikeView.isOn ).to.be.false; | ||
expect( strikeView.label ).to.equal( 'Strikethrough' ); | ||
expect( strikeView.icon ).to.match( /<svg / ); | ||
expect( strikeView.keystroke ).to.equal( 'CTRL+SHIFT+X' ); | ||
} ); | ||
|
||
it( 'should execute strike command on model execute event', () => { | ||
const executeSpy = testUtils.sinon.spy( editor, 'execute' ); | ||
|
||
strikeView.fire( 'execute' ); | ||
|
||
sinon.assert.calledOnce( executeSpy ); | ||
sinon.assert.calledWithExactly( executeSpy, 'strike' ); | ||
} ); | ||
|
||
it( 'should bind model to strike command', () => { | ||
const command = editor.commands.get( 'strike' ); | ||
|
||
expect( strikeView.isOn ).to.be.false; | ||
|
||
expect( strikeView.isEnabled ).to.be.false; | ||
|
||
command.value = true; | ||
expect( strikeView.isOn ).to.be.true; | ||
|
||
command.isEnabled = true; | ||
expect( strikeView.isEnabled ).to.be.true; | ||
} ); | ||
|
||
it( 'should set keystroke in the model', () => { | ||
expect( strikeView.keystroke ).to.equal( 'CTRL+SHIFT+X' ); | ||
} ); | ||
|
||
it( 'should set editor keystroke', () => { | ||
const spy = sinon.spy( editor, 'execute' ); | ||
const keyEventData = { | ||
keyCode: keyCodes.x, | ||
shiftKey: true, | ||
ctrlKey: true, | ||
preventDefault: sinon.spy(), | ||
stopPropagation: sinon.spy() | ||
}; | ||
|
||
const wasHandled = editor.keystrokes.press( keyEventData ); | ||
|
||
expect( wasHandled ).to.be.true; | ||
expect( spy.calledOnce ).to.be.true; | ||
expect( keyEventData.preventDefault.calledOnce ).to.be.true; | ||
} ); | ||
} ); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,108 @@ | ||
/** | ||
* @license Copyright (c) 2003-2017, CKSource - Rémy Hubscher. All rights reserved. | ||
* For licensing, see LICENSE.md. | ||
*/ | ||
|
||
import StrikethroughEngine from '../src/strikeengine'; | ||
|
||
import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor'; | ||
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; | ||
import AttributeCommand from '../src/attributecommand'; | ||
|
||
import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; | ||
import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view'; | ||
|
||
describe( 'StrikethroughEngine', () => { | ||
let editor, doc; | ||
|
||
beforeEach( () => { | ||
return VirtualTestEditor | ||
.create( { | ||
plugins: [ Paragraph, StrikethroughEngine ] | ||
} ) | ||
.then( newEditor => { | ||
editor = newEditor; | ||
|
||
doc = editor.document; | ||
} ); | ||
} ); | ||
|
||
afterEach( () => { | ||
return editor.destroy(); | ||
} ); | ||
|
||
it( 'should be loaded', () => { | ||
expect( editor.plugins.get( StrikethroughEngine ) ).to.be.instanceOf( StrikethroughEngine ); | ||
} ); | ||
|
||
it( 'should set proper schema rules', () => { | ||
expect( doc.schema.check( { name: '$inline', attributes: 'strike', inside: '$root' } ) ).to.be.false; | ||
expect( doc.schema.check( { name: '$inline', attributes: 'strike', inside: '$block' } ) ).to.be.true; | ||
expect( doc.schema.check( { name: '$inline', attributes: 'strike', inside: '$clipboardHolder' } ) ).to.be.true; | ||
} ); | ||
|
||
describe( 'command', () => { | ||
it( 'should register strike command', () => { | ||
const command = editor.commands.get( 'strike' ); | ||
|
||
expect( command ).to.be.instanceOf( AttributeCommand ); | ||
expect( command ).to.have.property( 'attributeKey', 'strike' ); | ||
} ); | ||
} ); | ||
|
||
describe( 'data pipeline conversions', () => { | ||
it( 'should convert <strike> to strike attribute', () => { | ||
editor.setData( '<p><strike>foo</strike>bar</p>' ); | ||
|
||
expect( getModelData( doc, { withoutSelection: true } ) ) | ||
.to.equal( '<paragraph><$text strike="true">foo</$text>bar</paragraph>' ); | ||
|
||
expect( editor.getData() ).to.equal( '<p><s>foo</s>bar</p>' ); | ||
} ); | ||
it( 'should convert <del> to strike attribute', () => { | ||
editor.setData( '<p><del>foo</del>bar</p>' ); | ||
|
||
expect( getModelData( doc, { withoutSelection: true } ) ) | ||
.to.equal( '<paragraph><$text strike="true">foo</$text>bar</paragraph>' ); | ||
|
||
expect( editor.getData() ).to.equal( '<p><s>foo</s>bar</p>' ); | ||
} ); | ||
|
||
it( 'should convert <s> to strike attribute', () => { | ||
editor.setData( '<p><s>foo</s>bar</p>' ); | ||
|
||
expect( getModelData( doc, { withoutSelection: true } ) ) | ||
.to.equal( '<paragraph><$text strike="true">foo</$text>bar</paragraph>' ); | ||
|
||
expect( editor.getData() ).to.equal( '<p><s>foo</s>bar</p>' ); | ||
} ); | ||
|
||
it( 'should convert text-decoration:line-through to strike attribute', () => { | ||
editor.setData( '<p><span style="text-decoration: line-through;">foo</span>bar</p>' ); | ||
|
||
expect( getModelData( doc, { withoutSelection: true } ) ) | ||
.to.equal( '<paragraph><$text strike="true">foo</$text>bar</paragraph>' ); | ||
|
||
expect( editor.getData() ).to.equal( '<p><s>foo</s>bar</p>' ); | ||
} ); | ||
|
||
it( 'should be integrated with autoparagraphing', () => { | ||
// Incorrect results because autoparagraphing works incorrectly (issue in paragraph). | ||
// https://github.com/ckeditor/ckeditor5-paragraph/issues/10 | ||
|
||
editor.setData( '<s>foo</s>bar' ); | ||
|
||
expect( getModelData( doc, { withoutSelection: true } ) ).to.equal( '<paragraph>foobar</paragraph>' ); | ||
|
||
expect( editor.getData() ).to.equal( '<p>foobar</p>' ); | ||
} ); | ||
} ); | ||
|
||
describe( 'editing pipeline conversion', () => { | ||
it( 'should convert attribute', () => { | ||
setModelData( doc, '<paragraph><$text strike="true">foo</$text>bar</paragraph>' ); | ||
|
||
expect( getViewData( editor.editing.view, { withoutSelection: true } ) ).to.equal( '<p><s>foo</s>bar</p>' ); | ||
} ); | ||
} ); | ||
} ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are not modifying standard header for each developer. "CKSource - Frederico Knabben" it's our company full name. We really appreciate your contribution, it will be noted in GitHub history and our change log, but we cannot change copyright information - this is also why we needed CLA to be signed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Natim - are you ok with me modifying headers back to original state and merging this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I forgot about that, it should be fixed now.