Browse Source

Improved safe keys implementation.

Piotrek Koszuliński 9 years ago
parent
commit
264d164a9c
2 changed files with 46 additions and 23 deletions
  1. 25 9
      packages/ckeditor5-typing/src/typing.js
  2. 21 14
      packages/ckeditor5-typing/tests/typing.js

+ 25 - 9
packages/ckeditor5-typing/src/typing.js

@@ -83,8 +83,6 @@ export default class Typing extends Feature {
 		doc.enqueueChanges( () => {
 			doc.composer.deleteContents( this._buffer.batch, doc.selection );
 		} );
-
-		// No 'preventDefalt', not to prevent mutations.
 	}
 
 	/**
@@ -231,20 +229,38 @@ class MutationHandler {
 	}
 }
 
-// This is absolutely lame, but it's enough for now.
-
-const safeKeystrokes = [
-	getCode( 'shift' ),
+const safeKeycodes = [
 	getCode( 'arrowUp' ),
 	getCode( 'arrowRight' ),
 	getCode( 'arrowDown' ),
-	getCode( 'arrowLeft' )
+	getCode( 'arrowLeft' ),
+	16, // Shift
+	17, // Ctrl
+	18, // Alt
+	20, // CapsLock
+	27, // Escape
+	33, // PageUp
+	34, // PageDown
+	35, // Home
+	36, // End
 ];
 
+// Function keys.
+for ( let code = 112; code <= 135; code++ ) {
+	safeKeycodes.push( code );
+}
+
+// Returns true if a keystroke should not cause any content change caused by "typing".
+//
+// Note: this implementation is very simple and will need to be refined with time.
+//
+// @param {engine.view.observer.keyObserver.KeyEventData} keyData
+// @returns {Boolean}
 function isSafeKeystroke( keyData ) {
-	if ( keyData.ctrlKey || keyData.altKey ) {
+	// Keystrokes which contain Ctrl don't represent typing.
+	if ( keyData.ctrlKey ) {
 		return true;
 	}
 
-	return safeKeystrokes.indexOf( keyData.keyCode ) > -1;
+	return safeKeycodes.includes( keyData.keyCode );
 }

+ 21 - 14
packages/ckeditor5-typing/tests/typing.js

@@ -163,7 +163,7 @@ describe( 'Typing feature', () => {
 				] );
 			}, null, 1000000 );
 
-			view.fire( 'keydown',  { keyCode: getCode( 'y' ) } );
+			view.fire( 'keydown', { keyCode: getCode( 'y' ) } );
 
 			expect( getModelData( model ) ).to.equal( '<paragraph>foy<selection />ar</paragraph>' );
 			expect( getViewData( view ) ).to.equal( '<p>foy{}ar</p>' );
@@ -175,11 +175,9 @@ describe( 'Typing feature', () => {
 					ModelRange.createFromParentsAndOffsets( modelRoot.getChild( 0 ), 2, modelRoot.getChild( 0 ), 4 ) ] );
 			} );
 
-			listenter.listenTo( view, 'keydown', () => {
-				expect( getModelData( model ) ).to.equal( '<paragraph>fo<selection>ob</selection>ar</paragraph>' );
-			}, null, 1000000 );
+			view.fire( 'keydown', { keyCode: getCode( 'arrowright' ) } );
 
-			view.fire( 'keydown',  { keyCode: getCode( 'arrowright' ) } );
+			expect( getModelData( model ) ).to.equal( '<paragraph>fo<selection>ob</selection>ar</paragraph>' );
 		} );
 
 		it( 'should do nothing on ctrl combinations', () => {
@@ -188,19 +186,28 @@ describe( 'Typing feature', () => {
 					ModelRange.createFromParentsAndOffsets( modelRoot.getChild( 0 ), 2, modelRoot.getChild( 0 ), 4 ) ] );
 			} );
 
-			listenter.listenTo( view, 'keydown', () => {
-				expect( getModelData( model ) ).to.equal( '<paragraph>fo<selection>ob</selection>ar</paragraph>' );
-			}, null, 1000000 );
+			view.fire( 'keydown', { ctrlKey: true, keyCode: getCode( 'c' ) } );
 
-			view.fire( 'keydown',  { ctrlKey: true, keyCode: getCode( 'c' ) } );
+			expect( getModelData( model ) ).to.equal( '<paragraph>fo<selection>ob</selection>ar</paragraph>' );
 		} );
 
-		it( 'should do nothing is selection is collapsed', () => {
-			listenter.listenTo( view, 'keydown', () => {
-				expect( getModelData( model ) ).to.equal( '<paragraph>foo<selection />bar</paragraph>' );
-			}, null, 1000000 );
+		it( 'should do nothing on non printable keys', () => {
+			model.enqueueChanges( () => {
+				model.selection.setRanges( [
+					ModelRange.createFromParentsAndOffsets( modelRoot.getChild( 0 ), 2, modelRoot.getChild( 0 ), 4 ) ] );
+			} );
 
-			view.fire( 'keydown',  { ctrlKey: true, keyCode: getCode( 'c' ) } );
+			view.fire( 'keydown', { keyCode: 16 } ); // Shift
+			view.fire( 'keydown', { keyCode: 35 } ); // Home
+			view.fire( 'keydown', { keyCode: 112 } ); // F1
+
+			expect( getModelData( model ) ).to.equal( '<paragraph>fo<selection>ob</selection>ar</paragraph>' );
+		} );
+
+		it( 'should do nothing if selection is collapsed', () => {
+			view.fire( 'keydown', { ctrlKey: true, keyCode: getCode( 'c' ) } );
+
+			expect( getModelData( model ) ).to.equal( '<paragraph>foo<selection />bar</paragraph>' );
 		} );
 	} );