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

I/5862: Disable text transformations inside code blocks #222

Merged
merged 23 commits into from
Jan 30, 2020
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
72298e0
Fix: Add ability to disable listening TextWatcher
panr Jan 20, 2020
580310b
Fix: Mix TextTransform plugin with forceDisabled and clearForceDisabl…
panr Jan 20, 2020
7c230b2
Remove typing state because it's been moved to the Plugin class
panr Jan 24, 2020
f8239cc
Remove reference to removed ForceDisabledMixin
panr Jan 26, 2020
af79703
Add tests covering plugin enabling / disabling watchers
panr Jan 27, 2020
150825a
Add test covering plugin disabling text transformation inside code bl…
panr Jan 27, 2020
ea81fae
Fix ClassicTestEditor plugin config for text transformation
panr Jan 27, 2020
cd364e1
Add @ckeditor/ckeditor5-code-block to package.json
panr Jan 27, 2020
876553c
Fix plugin tests
panr Jan 27, 2020
f1d534c
Fix the tests...
panr Jan 27, 2020
af9b8c2
Change plugin variable name in #plugin tests
panr Jan 27, 2020
55fee2f
Destroy text transformation plugin after each #plugin tests
panr Jan 27, 2020
ee9c9ed
Fix test block description from 'plugin' to '#isEnabled' and remove …
panr Jan 28, 2020
138af5f
Change setting TextTransformation#isEnabled on selection change:range
panr Jan 28, 2020
161b1a8
Remove _watchersStack since it's no longer necessary and bind watcher…
panr Jan 28, 2020
fe42b41
Add an observable #isEnabled to TextWatcher
panr Jan 28, 2020
267595a
Remove tests covering _wathersStack since they're no longer necessary
panr Jan 28, 2020
30671da
Add tests to TextWatcher covering #isEnabled observable
panr Jan 28, 2020
153192e
Toggle text watching on isEnabled state change.
jodator Jan 28, 2020
8eb686a
Remove EmitterMixin and document #isEnabled observable
panr Jan 29, 2020
0bbaeee
Remove watcher.off() from afterEach section in tests
panr Jan 29, 2020
75b95bf
Docs improvements for TextWatcher#isEnabled.
jodator Jan 29, 2020
0a889f2
Small docs improvements for TextWatcher#isEnabled.
jodator Jan 29, 2020
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
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"@ckeditor/ckeditor5-list": "^16.0.0",
"@ckeditor/ckeditor5-paragraph": "^16.0.0",
"@ckeditor/ckeditor5-undo": "^16.0.0",
"@ckeditor/ckeditor5-code-block": "^16.0.0",
"eslint": "^5.5.0",
"eslint-config-ckeditor5": "^2.0.0",
"husky": "^1.3.1",
Expand Down
26 changes: 24 additions & 2 deletions src/texttransformation.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,31 @@ export default class TextTransformation extends Plugin {
include: DEFAULT_TRANSFORMATIONS
}
} );

this.editor = editor;
}

/**
* @inheritDoc
*/
init() {
const model = this.editor.model;
const modelSelection = model.document.selection;

modelSelection.on( 'change:range', () => {
// Disable plugin when selection is inside a code block.
this.isEnabled = !modelSelection.anchor.parent.is( 'codeBlock' );
} );

this._enableTransformationWatchers();
}

/**
* Create new set of TextWatchers listening to the editor for typing and selection events.
*
* @private
*/
_enableTransformationWatchers() {
const editor = this.editor;
const model = editor.model;
const input = editor.plugins.get( 'Input' );
Expand All @@ -110,7 +129,7 @@ export default class TextTransformation extends Plugin {

const watcher = new TextWatcher( editor.model, text => from.test( text ) );

watcher.on( 'matched:data', ( evt, data ) => {
const watcherCallback = ( evt, data ) => {
if ( !input.isInput( data.batch ) ) {
return;
}
Expand Down Expand Up @@ -142,7 +161,10 @@ export default class TextTransformation extends Plugin {
changeIndex += replaceWith.length;
}
} );
} );
};

watcher.on( 'matched:data', watcherCallback );
watcher.bind( 'isEnabled' ).to( this );
}
}
}
Expand Down
38 changes: 33 additions & 5 deletions src/textwatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
*/

import mix from '@ckeditor/ckeditor5-utils/src/mix';
import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin';
import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin';
import getLastTextLine from './utils/getlasttextline';

/**
Expand All @@ -25,12 +25,41 @@ export default class TextWatcher {
* Creates a text watcher instance.
* @param {module:engine/model/model~Model} model
* @param {Function} testCallback The function used to match the text.
*
* @mixes module:utils/observablemixin~ObservableMixin
*/
constructor( model, testCallback ) {
this.model = model;
this.testCallback = testCallback;
this.hasMatch = false;

/**
* Flag indicating whether the TextWatcher is enabled or disabled.
* A disabled TextWatcher will not evaluate text.
*
* TextWatcher can be simply disabled like that:
*
* // Disable the TextWatcher so that it doesn't evaluate text.
* const watcher = new TextWatcher( editor.model, text => from.test( text ) );
* watcher.isEnabled = false;
*
*
* @observable
* @readonly
* @member {Boolean} #isEnabled
*/
this.set( 'isEnabled', true );
panr marked this conversation as resolved.
Show resolved Hide resolved

// Toggle text watching on isEnabled state change.
this.on( 'change:isEnabled', () => {
if ( this.isEnabled ) {
this._startListening();
} else {
this.stopListening( model.document.selection );
this.stopListening( model.document );
}
} );

this._startListening();
}

Expand All @@ -43,7 +72,7 @@ export default class TextWatcher {
const model = this.model;
panr marked this conversation as resolved.
Show resolved Hide resolved
const document = model.document;

document.selection.on( 'change:range', ( evt, { directChange } ) => {
this.listenTo( document.selection, 'change:range', ( evt, { directChange } ) => {
// Indirect changes (i.e. when the user types or external changes are applied) are handled in the document's change event.
if ( !directChange ) {
return;
Expand All @@ -62,7 +91,7 @@ export default class TextWatcher {
this._evaluateTextBeforeSelection( 'selection' );
} );

document.on( 'change:data', ( evt, batch ) => {
this.listenTo( document, 'change:data', ( evt, batch ) => {
if ( batch.type == 'transparent' ) {
return;
}
Expand Down Expand Up @@ -127,5 +156,4 @@ export default class TextWatcher {
}
}

mix( TextWatcher, EmitterMixin );

mix( TextWatcher, ObservableMixin );
37 changes: 32 additions & 5 deletions tests/texttransformation.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import TextTransformation from '../src/texttransformation';
import { getData, setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold';
import CodeBlock from '@ckeditor/ckeditor5-code-block/src/codeblock';

describe( 'Text transformation feature', () => {
let editorElement, editor, model, doc;
Expand Down Expand Up @@ -40,6 +41,24 @@ describe( 'Text transformation feature', () => {
} );
} );

describe( '#isEnabled', () => {
let plugin;

beforeEach( () => {
return createEditorInstance().then( () => {
plugin = editor.plugins.get( TextTransformation );
} );
} );

afterEach( () => {
plugin.destroy();
} );

it( 'should be enabled after initialization', () => {
expect( plugin.isEnabled ).to.be.true;
} );
} );

describe( 'transformations', () => {
beforeEach( createEditorInstance );

Expand Down Expand Up @@ -155,6 +174,18 @@ describe( 'Text transformation feature', () => {
.to.equal( '<paragraph>"Foo <softBreak></softBreak>“Bar”</paragraph>' );
} );

it( 'should be disabled inside code blocks', () => {
setData( model, '<codeBlock language="plaintext">some [] code</codeBlock>' );

simulateTyping( '1/2' );

const plugin = editor.plugins.get( 'TextTransformation' );

expect( plugin.isEnabled ).to.be.false;
expect( getData( model, { withoutSelection: true } ) )
.to.equal( '<codeBlock language="plaintext">some 1/2 code</codeBlock>' );
} );

function testTransformation( transformFrom, transformTo, textInParagraph = 'A foo' ) {
it( `should transform "${ transformFrom }" to "${ transformTo }"`, () => {
setData( model, `<paragraph>${ textInParagraph }[]</paragraph>` );
Expand Down Expand Up @@ -355,18 +386,14 @@ describe( 'Text transformation feature', () => {
function createEditorInstance( additionalConfig = {} ) {
return ClassicTestEditor
.create( editorElement, Object.assign( {
plugins: [ Typing, Paragraph, Bold, TextTransformation ]
plugins: [ Typing, Paragraph, Bold, TextTransformation, CodeBlock ]
}, additionalConfig ) )
.then( newEditor => {
editor = newEditor;

model = editor.model;
doc = model.document;

model.schema.register( 'softBreak', {
allowWhere: '$text',
isInline: true
} );
editor.conversion.elementToElement( {
model: 'softBreak',
view: 'br'
Expand Down
104 changes: 103 additions & 1 deletion tests/textwatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,18 @@ describe( 'TextWatcher', () => {
}
} );

describe( '#isEnabled', () => {
it( 'should be enabled after initialization', () => {
expect( watcher.isEnabled ).to.be.true;
} );

it( 'should be disabled after setting #isEnabled to false', () => {
watcher.isEnabled = false;

expect( watcher.isEnabled ).to.be.false;
} );
} );

describe( 'testCallback', () => {
it( 'should evaluate text before caret for data changes', () => {
model.change( writer => {
Expand Down Expand Up @@ -97,6 +109,35 @@ describe( 'TextWatcher', () => {

sinon.assert.notCalled( testCallbackStub );
} );

it( 'should not evaluate text when watcher is disabled', () => {
watcher.isEnabled = false;

model.change( writer => {
writer.insertText( '@', doc.selection.getFirstPosition() );
} );

sinon.assert.notCalled( testCallbackStub );
} );

it( 'should evaluate text when watcher is enabled again', () => {
watcher.isEnabled = false;

model.change( writer => {
writer.insertText( '@', doc.selection.getFirstPosition() );
} );

sinon.assert.notCalled( testCallbackStub );

watcher.isEnabled = true;

model.change( writer => {
writer.insertText( '@', doc.selection.getFirstPosition() );
} );

sinon.assert.calledOnce( testCallbackStub );
sinon.assert.calledWithExactly( testCallbackStub, 'foo @@' );
} );
} );

describe( 'events', () => {
Expand All @@ -113,6 +154,22 @@ describe( 'TextWatcher', () => {
sinon.assert.notCalled( unmatchedSpy );
} );

it( 'should not fire "matched:data" event when watcher is disabled' +
' (even when test callback returns true for model data changes)', () => {
watcher.isEnabled = false;

testCallbackStub.returns( true );

model.change( writer => {
writer.insertText( '@', doc.selection.getFirstPosition() );
} );

sinon.assert.notCalled( testCallbackStub );
sinon.assert.notCalled( matchedDataSpy );
sinon.assert.notCalled( matchedSelectionSpy );
sinon.assert.notCalled( unmatchedSpy );
} );

it( 'should fire "matched:selection" event when test callback returns true for model data changes', () => {
testCallbackStub.returns( true );

Expand All @@ -130,6 +187,26 @@ describe( 'TextWatcher', () => {
sinon.assert.notCalled( unmatchedSpy );
} );

it( 'should not fire "matched:selection" event when when watcher is disabled' +
' (even when test callback returns true for model data changes)', () => {
watcher.isEnabled = false;

testCallbackStub.returns( true );

model.enqueueChange( 'transparent', writer => {
writer.insertText( '@', doc.selection.getFirstPosition() );
} );

model.change( writer => {
writer.setSelection( doc.getRoot().getChild( 0 ), 0 );
} );

sinon.assert.notCalled( testCallbackStub );
sinon.assert.notCalled( matchedDataSpy );
sinon.assert.notCalled( matchedSelectionSpy );
sinon.assert.notCalled( unmatchedSpy );
} );

it( 'should not fire "matched" event when test callback returns false', () => {
testCallbackStub.returns( false );

Expand Down Expand Up @@ -188,6 +265,31 @@ describe( 'TextWatcher', () => {
sinon.assert.notCalled( matchedSelectionSpy );
sinon.assert.calledOnce( unmatchedSpy );
} );

it( 'should not fire "umatched" event when selection is expanded if watcher is disabled', () => {
watcher.isEnabled = false;

testCallbackStub.returns( true );

model.change( writer => {
writer.insertText( '@', doc.selection.getFirstPosition() );
} );

sinon.assert.notCalled( testCallbackStub );
sinon.assert.notCalled( matchedDataSpy );
sinon.assert.notCalled( matchedSelectionSpy );
sinon.assert.notCalled( unmatchedSpy );

model.change( writer => {
const start = writer.createPositionAt( doc.getRoot().getChild( 0 ), 0 );

writer.setSelection( writer.createRange( start, start.getShiftedBy( 1 ) ) );
} );

sinon.assert.notCalled( testCallbackStub );
sinon.assert.notCalled( matchedDataSpy );
sinon.assert.notCalled( matchedSelectionSpy );
sinon.assert.notCalled( unmatchedSpy );
} );
} );
} );

jodator marked this conversation as resolved.
Show resolved Hide resolved