浏览代码

Merge pull request #239 from ckeditor/i/6180

Fix: Cancelling the table and table cell property forms should not remove a newly created table. Closes ckeditor/ckeditor5#6180.
Maciej 5 年之前
父节点
当前提交
0b02c1a2b2

+ 5 - 1
packages/ckeditor5-table/src/tablecellproperties/tablecellpropertiesui.js

@@ -131,7 +131,11 @@ export default class TableCellPropertiesUI extends Plugin {
 		} );
 
 		this.listenTo( view, 'cancel', () => {
-			editor.execute( 'undo', this._undoStepBatch );
+			// https://github.com/ckeditor/ckeditor5/issues/6180
+			if ( this._undoStepBatch.operations.length ) {
+				editor.execute( 'undo', this._undoStepBatch );
+			}
+
 			this._hideView();
 		} );
 

+ 5 - 1
packages/ckeditor5-table/src/tableproperties/tablepropertiesui.js

@@ -130,7 +130,11 @@ export default class TablePropertiesUI extends Plugin {
 		} );
 
 		this.listenTo( view, 'cancel', () => {
-			editor.execute( 'undo', this._undoStepBatch );
+			// https://github.com/ckeditor/ckeditor5/issues/6180
+			if ( this._undoStepBatch.operations.length ) {
+				editor.execute( 'undo', this._undoStepBatch );
+			}
+
 			this._hideView();
 		} );
 

+ 55 - 36
packages/ckeditor5-table/tests/tablecellproperties/tablecellpropertiesui.js

@@ -130,45 +130,64 @@ describe( 'table cell properties', () => {
 				expect( contextualBalloon.visibleView ).to.be.null;
 			} );
 
-			it( 'should undo the entire batch of changes on #cancel', () => {
-				// Show the view. New batch will be created.
-				tableCellPropertiesButton.fire( 'execute' );
+			describe( '#cancel event', () => {
+				// https://github.com/ckeditor/ckeditor5/issues/6180
+				it( 'should not undo if it there were no changes made to the property fields', () => {
+					const spy = sinon.spy( editor, 'execute' );
 
-				// Do the changes like a user.
-				tableCellPropertiesView.borderStyle = 'dotted';
-				tableCellPropertiesView.backgroundColor = 'red';
-
-				expect( getModelData( editor.model ) ).to.equal(
-					'<table>' +
-						'<tableRow>' +
-							'<tableCell backgroundColor="red" borderStyle="dotted">' +
-								'<paragraph>[]foo</paragraph>' +
-							'</tableCell>' +
-						'</tableRow>' +
-					'</table>' +
-					'<paragraph>bar</paragraph>'
-				);
-
-				tableCellPropertiesView.fire( 'cancel' );
-
-				expect( getModelData( editor.model ) ).to.equal(
-					'<table>' +
-						'<tableRow>' +
-							'<tableCell>' +
-								'<paragraph>[]foo</paragraph>' +
-							'</tableCell>' +
-						'</tableRow>' +
-					'</table>' +
-					'<paragraph>bar</paragraph>'
-				);
-			} );
+					// Show the view. New batch will be created.
+					tableCellPropertiesButton.fire( 'execute' );
 
-			it( 'should hide on #cancel', () => {
-				tableCellPropertiesButton.fire( 'execute' );
-				expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView );
+					// Cancel the view immediately.
+					tableCellPropertiesView.fire( 'cancel' );
 
-				tableCellPropertiesView.fire( 'cancel' );
-				expect( contextualBalloon.visibleView ).to.be.null;
+					sinon.assert.notCalled( spy );
+				} );
+
+				it( 'should undo the entire batch of changes if there were some', () => {
+					const spy = sinon.spy( editor, 'execute' );
+
+					// Show the view. New batch will be created.
+					tableCellPropertiesButton.fire( 'execute' );
+
+					// Do the changes like a user.
+					tableCellPropertiesView.borderStyle = 'dotted';
+					tableCellPropertiesView.backgroundColor = 'red';
+
+					expect( getModelData( editor.model ) ).to.equal(
+						'<table>' +
+							'<tableRow>' +
+								'<tableCell backgroundColor="red" borderStyle="dotted">' +
+									'<paragraph>[]foo</paragraph>' +
+								'</tableCell>' +
+							'</tableRow>' +
+						'</table>' +
+						'<paragraph>bar</paragraph>'
+					);
+
+					tableCellPropertiesView.fire( 'cancel' );
+
+					expect( getModelData( editor.model ) ).to.equal(
+						'<table>' +
+							'<tableRow>' +
+								'<tableCell>' +
+									'<paragraph>[]foo</paragraph>' +
+								'</tableCell>' +
+							'</tableRow>' +
+						'</table>' +
+						'<paragraph>bar</paragraph>'
+					);
+
+					sinon.assert.calledWith( spy, 'undo', tableCellPropertiesUI._undoStepBatch );
+				} );
+
+				it( 'should hide the view', () => {
+					tableCellPropertiesButton.fire( 'execute' );
+					expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView );
+
+					tableCellPropertiesView.fire( 'cancel' );
+					expect( contextualBalloon.visibleView ).to.be.null;
+				} );
 			} );
 
 			it( 'should hide on the Esc key press', () => {

+ 55 - 36
packages/ckeditor5-table/tests/tableproperties/tablepropertiesui.js

@@ -130,45 +130,64 @@ describe( 'table properties', () => {
 				expect( contextualBalloon.visibleView ).to.be.null;
 			} );
 
-			it( 'should undo the entire batch of changes on #cancel', () => {
-				// Show the view. New batch will be created.
-				tablePropertiesButton.fire( 'execute' );
+			describe( '#cancel event', () => {
+				// https://github.com/ckeditor/ckeditor5/issues/6180
+				it( 'should not undo if it there were no changes made to the property fields', () => {
+					const spy = sinon.spy( editor, 'execute' );
 
-				// Do the changes like a user.
-				tablePropertiesView.borderStyle = 'dotted';
-				tablePropertiesView.backgroundColor = 'red';
-
-				expect( getModelData( editor.model ) ).to.equal(
-					'<table backgroundColor="red" borderStyle="dotted">' +
-						'<tableRow>' +
-							'<tableCell>' +
-								'<paragraph>[]foo</paragraph>' +
-							'</tableCell>' +
-						'</tableRow>' +
-					'</table>' +
-					'<paragraph>bar</paragraph>'
-				);
-
-				tablePropertiesView.fire( 'cancel' );
-
-				expect( getModelData( editor.model ) ).to.equal(
-					'<table>' +
-						'<tableRow>' +
-							'<tableCell>' +
-								'<paragraph>[]foo</paragraph>' +
-							'</tableCell>' +
-						'</tableRow>' +
-					'</table>' +
-					'<paragraph>bar</paragraph>'
-				);
-			} );
+					// Show the view. New batch will be created.
+					tablePropertiesButton.fire( 'execute' );
 
-			it( 'should hide on #cancel', () => {
-				tablePropertiesButton.fire( 'execute' );
-				expect( contextualBalloon.visibleView ).to.equal( tablePropertiesView );
+					// Cancel the view immediately.
+					tablePropertiesView.fire( 'cancel' );
 
-				tablePropertiesView.fire( 'cancel' );
-				expect( contextualBalloon.visibleView ).to.be.null;
+					sinon.assert.notCalled( spy );
+				} );
+
+				it( 'should undo the entire batch of changes if there were some', () => {
+					const spy = sinon.spy( editor, 'execute' );
+
+					// Show the view. New batch will be created.
+					tablePropertiesButton.fire( 'execute' );
+
+					// Do the changes like a user.
+					tablePropertiesView.borderStyle = 'dotted';
+					tablePropertiesView.backgroundColor = 'red';
+
+					expect( getModelData( editor.model ) ).to.equal(
+						'<table backgroundColor="red" borderStyle="dotted">' +
+							'<tableRow>' +
+								'<tableCell>' +
+									'<paragraph>[]foo</paragraph>' +
+								'</tableCell>' +
+							'</tableRow>' +
+						'</table>' +
+						'<paragraph>bar</paragraph>'
+					);
+
+					tablePropertiesView.fire( 'cancel' );
+
+					expect( getModelData( editor.model ) ).to.equal(
+						'<table>' +
+							'<tableRow>' +
+								'<tableCell>' +
+									'<paragraph>[]foo</paragraph>' +
+								'</tableCell>' +
+							'</tableRow>' +
+						'</table>' +
+						'<paragraph>bar</paragraph>'
+					);
+
+					sinon.assert.calledWith( spy, 'undo', tablePropertiesUI._undoStepBatch );
+				} );
+
+				it( 'should hide the view', () => {
+					tablePropertiesButton.fire( 'execute' );
+					expect( contextualBalloon.visibleView ).to.equal( tablePropertiesView );
+
+					tablePropertiesView.fire( 'cancel' );
+					expect( contextualBalloon.visibleView ).to.be.null;
+				} );
 			} );
 
 			it( 'should hide on the Esc key press', () => {