Преглед изворни кода

Improved delete and backspace handling around blocks widgets when empty elements precede or follow a widget.

Aleksander Nowodzinski пре 5 година
родитељ
комит
f265208334

+ 85 - 17
packages/ckeditor5-widget/src/widgettypearound/widgettypearound.js

@@ -580,30 +580,69 @@ export default class WidgetTypeAround extends Plugin {
 			const direction = domEventData.direction;
 			const selectedModelWidget = model.document.selection.getSelectedElement();
 
-			if ( typeAroundSelectionAttributeValue === 'before' ) {
-				if ( direction === 'backward' ) {
-					const range = schema.getNearestSelectionRange( model.createPositionBefore( selectedModelWidget ), direction );
+			const isFakeCaretBefore = typeAroundSelectionAttributeValue === 'before';
+			const isForwardDelete = direction == 'forward';
 
-					if ( range ) {
+			const shouldDeleteEntireWidget = ( isFakeCaretBefore && isForwardDelete ) || ( !isFakeCaretBefore && !isForwardDelete );
+			const shouldTryDeleteContentBeforeWidget = isFakeCaretBefore && !isForwardDelete;
+			const shouldTryDeleteContentAfterWidget = !isFakeCaretBefore && isForwardDelete;
+
+			if ( shouldDeleteEntireWidget ) {
+				editor.execute( 'delete', {
+					selection: model.createSelection( selectedModelWidget, 'on' )
+				} );
+			}
+
+			if ( shouldTryDeleteContentBeforeWidget ) {
+				const range = schema.getNearestSelectionRange( model.createPositionBefore( selectedModelWidget ), direction );
+
+				if ( range ) {
+					const deepestEmptyRangeAncestor = getDeepestEmptyPositionAncestor( range.start );
+
+					// Handle a case when there's an empty document tree branch before the widget that should be deleted.
+					//
+					//		<foo><bar></bar></foo>[<widget></widget>]
+					//
+					// Note: Range is collapsed, so it does not matter if this is start or end.
+					if ( deepestEmptyRangeAncestor ) {
+						model.deleteContent( model.createSelection( deepestEmptyRangeAncestor, 'on' ), {
+							doNotAutoparagraph: true
+						} );
+					}
+					// Handle a case when there's a non-empty document tree branch before the widget.
+					//
+					//		<foo>bar</foo>[<widget></widget>] -> <foo>ba[]</foo><widget></widget>
+					//
+					else {
 						model.change( writer => {
 							writer.setSelection( range );
 							editor.execute( 'delete' );
 						} );
 					}
-				} else {
-					editor.execute( 'delete', {
-						selection: model.createSelection( selectedModelWidget, 'on' )
-					} );
 				}
-			} else {
-				if ( direction === 'backward' ) {
-					editor.execute( 'delete', {
-						selection: model.createSelection( selectedModelWidget, 'on' )
-					} );
-				} else {
-					const range = schema.getNearestSelectionRange( model.createPositionAfter( selectedModelWidget ), direction );
-
-					if ( range ) {
+			}
+
+			if ( shouldTryDeleteContentAfterWidget ) {
+				const range = schema.getNearestSelectionRange( model.createPositionAfter( selectedModelWidget ), direction );
+
+				if ( range ) {
+					const deepestEmptyRangeAncestor = getDeepestEmptyPositionAncestor( range.start );
+
+					// Handle a case when there's an empty document tree branch after the widget that should be deleted.
+					//
+					//		[<widget></widget>]<foo><bar></bar></foo>
+					//
+					// Note: Range is collapsed, so it does not matter if this is start or end.
+					if ( deepestEmptyRangeAncestor ) {
+						model.deleteContent( model.createSelection( deepestEmptyRangeAncestor, 'on' ), {
+							doNotAutoparagraph: true
+						} );
+					}
+					// Handle a case when there's a non-empty document tree branch after the widget.
+					//
+					//		[<widget></widget>]<foo>bar</foo> -> <widget></widget><foo>[]ar</foo>
+					//
+					else {
 						model.change( writer => {
 							writer.setSelection( range );
 							editor.execute( 'forwardDelete' );
@@ -682,3 +721,32 @@ function injectFakeCaret( wrapperDomElement ) {
 
 	wrapperDomElement.appendChild( caretTemplate.render() );
 }
+
+// Returns the ancestor of a position closest to the root which is empty. For instance,
+// for a position in `<baz>`:
+//
+//		<foo>abc<bar><baz>[]</baz></bar></foo>
+//
+// it returns `<bar>`.
+//
+// @param {module:engine/model/position~Position} position
+// @returns {module:engine/model/element~Element|null}
+function getDeepestEmptyPositionAncestor( position ) {
+	const firstPositionParent = position.parent;
+
+	if ( !firstPositionParent.isEmpty ) {
+		return null;
+	}
+
+	let deepestEmptyAncestor = firstPositionParent;
+
+	for ( const ancestor of firstPositionParent.getAncestors().reverse() ) {
+		if ( ancestor.childCount > 1 || ancestor.is( 'rootElement' ) ) {
+			break;
+		}
+
+		deepestEmptyAncestor = ancestor;
+	}
+
+	return deepestEmptyAncestor;
+}

+ 138 - 0
packages/ckeditor5-widget/tests/widgettypearound/widgettypearound.js

@@ -894,6 +894,76 @@ describe( 'WidgetTypeAround', () => {
 					sinon.assert.calledOnce( domEventDataStub.domEvent.preventDefault );
 				} );
 
+				it( 'should delete an empty paragraph before a widget if the "fake caret" is also before the widget', () => {
+					setModelData( editor.model, '<paragraph></paragraph>[<blockWidget></blockWidget>]' );
+
+					fireKeyboardEvent( 'arrowleft' );
+
+					expect( getModelData( model ) ).to.equal( '<paragraph></paragraph>[<blockWidget></blockWidget>]' );
+					expect( modelSelection.getAttribute( 'widget-type-around' ) ).to.equal( 'before' );
+
+					fireDeleteEvent();
+					expect( getModelData( model ) ).to.equal( '[<blockWidget></blockWidget>]' );
+					expect( modelSelection.getAttribute( 'widget-type-around' ) ).to.equal( 'before' );
+
+					sinon.assert.calledOnce( eventInfoStub.stop );
+					sinon.assert.calledOnce( domEventDataStub.domEvent.preventDefault );
+				} );
+
+				it( 'should delete an empty document tree branch before a widget if the "fake caret" is also before the widget', () => {
+					setModelData( editor.model, '<blockQuote><paragraph></paragraph></blockQuote>[<blockWidget></blockWidget>]' );
+
+					fireKeyboardEvent( 'arrowleft' );
+
+					expect( getModelData( model ) ).to.equal(
+						'<blockQuote>' +
+							'<paragraph></paragraph>' +
+						'</blockQuote>' +
+						'[<blockWidget></blockWidget>]'
+					);
+					expect( modelSelection.getAttribute( 'widget-type-around' ) ).to.equal( 'before' );
+
+					fireDeleteEvent();
+					expect( getModelData( model ) ).to.equal( '[<blockWidget></blockWidget>]' );
+					expect( modelSelection.getAttribute( 'widget-type-around' ) ).to.equal( 'before' );
+
+					sinon.assert.calledOnce( eventInfoStub.stop );
+					sinon.assert.calledOnce( domEventDataStub.domEvent.preventDefault );
+				} );
+
+				it( 'should delete an empty document tree sub-branch before a widget if the "fake caret" is also before the widget', () => {
+					setModelData( editor.model,
+						'<blockQuote>' +
+							'<paragraph>foo</paragraph>' +
+							'<paragraph></paragraph>' +
+						'</blockQuote>' +
+						'[<blockWidget></blockWidget>]'
+					);
+
+					fireKeyboardEvent( 'arrowleft' );
+
+					expect( getModelData( model ) ).to.equal(
+						'<blockQuote>' +
+							'<paragraph>foo</paragraph>' +
+							'<paragraph></paragraph>' +
+						'</blockQuote>' +
+						'[<blockWidget></blockWidget>]'
+					);
+					expect( modelSelection.getAttribute( 'widget-type-around' ) ).to.equal( 'before' );
+
+					fireDeleteEvent();
+					expect( getModelData( model ) ).to.equal(
+						'<blockQuote>' +
+							'<paragraph>foo</paragraph>' +
+						'</blockQuote>' +
+						'[<blockWidget></blockWidget>]'
+					);
+					expect( modelSelection.getAttribute( 'widget-type-around' ) ).to.equal( 'before' );
+
+					sinon.assert.calledOnce( eventInfoStub.stop );
+					sinon.assert.calledOnce( domEventDataStub.domEvent.preventDefault );
+				} );
+
 				it( 'should do nothing if the "fake caret" is before the widget but there is nothing to delete there', () => {
 					setModelData( editor.model, '[<blockWidget></blockWidget>]' );
 
@@ -979,6 +1049,74 @@ describe( 'WidgetTypeAround', () => {
 					sinon.assert.calledOnce( domEventDataStub.domEvent.preventDefault );
 				} );
 
+				it( 'should delete an empty paragraph after a widget if the "fake caret" is also after the widget', () => {
+					setModelData( editor.model, '[<blockWidget></blockWidget>]<paragraph></paragraph>' );
+
+					fireKeyboardEvent( 'arrowright' );
+
+					expect( getModelData( model ) ).to.equal( '[<blockWidget></blockWidget>]<paragraph></paragraph>' );
+					expect( modelSelection.getAttribute( 'widget-type-around' ) ).to.equal( 'after' );
+
+					fireDeleteEvent( true );
+					expect( getModelData( model ) ).to.equal( '[<blockWidget></blockWidget>]' );
+					expect( modelSelection.getAttribute( 'widget-type-around' ) ).to.equal( 'after' );
+
+					sinon.assert.calledOnce( eventInfoStub.stop );
+					sinon.assert.calledOnce( domEventDataStub.domEvent.preventDefault );
+				} );
+
+				it( 'should delete an empty document tree branch after a widget if the "fake caret" is also after the widget', () => {
+					setModelData( editor.model, '[<blockWidget></blockWidget>]<blockQuote><paragraph></paragraph></blockQuote>' );
+
+					fireKeyboardEvent( 'arrowright' );
+
+					expect( getModelData( model ) ).to.equal(
+						'[<blockWidget></blockWidget>]' +
+						'<blockQuote><paragraph></paragraph></blockQuote>'
+					);
+					expect( modelSelection.getAttribute( 'widget-type-around' ) ).to.equal( 'after' );
+
+					fireDeleteEvent( true );
+					expect( getModelData( model ) ).to.equal( '[<blockWidget></blockWidget>]' );
+					expect( modelSelection.getAttribute( 'widget-type-around' ) ).to.equal( 'after' );
+
+					sinon.assert.calledOnce( eventInfoStub.stop );
+					sinon.assert.calledOnce( domEventDataStub.domEvent.preventDefault );
+				} );
+
+				it( 'should delete an empty document tree sub-branch after a widget if the "fake caret" is also after the widget', () => {
+					setModelData( editor.model,
+						'[<blockWidget></blockWidget>]' +
+						'<blockQuote>' +
+							'<paragraph></paragraph>' +
+							'<paragraph>foo</paragraph>' +
+						'</blockQuote>'
+					);
+
+					fireKeyboardEvent( 'arrowright' );
+
+					expect( getModelData( model ) ).to.equal(
+						'[<blockWidget></blockWidget>]' +
+						'<blockQuote>' +
+							'<paragraph></paragraph>' +
+							'<paragraph>foo</paragraph>' +
+						'</blockQuote>'
+					);
+					expect( modelSelection.getAttribute( 'widget-type-around' ) ).to.equal( 'after' );
+
+					fireDeleteEvent( true );
+					expect( getModelData( model ) ).to.equal(
+						'[<blockWidget></blockWidget>]' +
+						'<blockQuote>' +
+							'<paragraph>foo</paragraph>' +
+						'</blockQuote>'
+					);
+					expect( modelSelection.getAttribute( 'widget-type-around' ) ).to.equal( 'after' );
+
+					sinon.assert.calledOnce( eventInfoStub.stop );
+					sinon.assert.calledOnce( domEventDataStub.domEvent.preventDefault );
+				} );
+
 				it( 'should do nothing if the "fake caret" is after the widget but there is nothing to delete there', () => {
 					setModelData( editor.model, '[<blockWidget></blockWidget>]' );