Merge pull request #7664 from ckeditor/i/7633-resizer-glitch

Fix (widget): `Resizer#redraw()` should not change the editing view unless a different size should be set. Closes #7633.
ckeditor · Jul 22, 2020 · 978dd71 · 978dd71
2 parents 02f91dd + 0bac0ea
commit 978dd71
Show file tree

Hide file tree

Showing 2 changed files with 86 additions and 31 deletions.
diff --git a/packages/ckeditor5-widget/src/widgetresize/resizer.js b/packages/ckeditor5-widget/src/widgetresize/resizer.js
@@ -10,6 +10,7 @@
 import View from '@ckeditor/ckeditor5-ui/src/view';
 import Template from '@ckeditor/ckeditor5-ui/src/template';
 import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect';
+import compareArrays from '@ckeditor/ckeditor5-utils/src/comparearrays';
 
 import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin';
 import mix from '@ckeditor/ckeditor5-utils/src/mix';
@@ -228,39 +229,59 @@ export default class Resizer {
 	redraw( handleHostRect ) {
 		const domWrapper = this._domResizerWrapper;
 
-		if ( existsInDom( domWrapper ) ) {
-			this._options.editor.editing.view.change( writer => {
-				// Refresh only if resizer exists in the DOM.
-				const widgetWrapper = domWrapper.parentElement;
-				const handleHost = this._getHandleHost();
-				const clientRect = handleHostRect || new Rect( handleHost );
-
-				writer.setStyle( 'width', clientRect.width + 'px', this._viewResizerWrapper );
-				writer.setStyle( 'height', clientRect.height + 'px', this._viewResizerWrapper );
-
-				const offsets = {
-					left: handleHost.offsetLeft,
-					top: handleHost.offsetTop,
-					height: handleHost.offsetHeight,
-					width: handleHost.offsetWidth
-				};
-
-				// In case a resizing host is not a widget wrapper, we need to compensate
-				// for any additional offsets the resize host might have. E.g. wrapper padding
-				// or simply another editable. By doing that the border and resizers are shown
-				// only around the resize host.
-				if ( !widgetWrapper.isSameNode( handleHost ) ) {
-					writer.setStyle( 'left', offsets.left + 'px', this._viewResizerWrapper );
-					writer.setStyle( 'top', offsets.top + 'px', this._viewResizerWrapper );
-
-					writer.setStyle( 'height', offsets.height + 'px', this._viewResizerWrapper );
-					writer.setStyle( 'width', offsets.width + 'px', this._viewResizerWrapper );
-				}
-			} );
+		// Refresh only if resizer exists in the DOM.
+		if ( !existsInDom( domWrapper ) ) {
+			return;
+		}
+
+		const widgetWrapper = domWrapper.parentElement;
+		const handleHost = this._getHandleHost();
+		const resizerWrapper = this._viewResizerWrapper;
+		const currentDimensions = [
+			resizerWrapper.getStyle( 'width' ),
+			resizerWrapper.getStyle( 'height' ),
+			resizerWrapper.getStyle( 'left' ),
+			resizerWrapper.getStyle( 'top' )
+		];
+		let newDimensions;
+
+		if ( widgetWrapper.isSameNode( handleHost ) ) {
+			const clientRect = handleHostRect || new Rect( handleHost );
+
+			newDimensions = [
+				clientRect.width + 'px',
+				clientRect.height + 'px',
+				undefined,
+				undefined
+			];
+		}
+		// In case a resizing host is not a widget wrapper, we need to compensate
+		// for any additional offsets the resize host might have. E.g. wrapper padding
+		// or simply another editable. By doing that the border and resizers are shown
+		// only around the resize host.
+		else {
+			newDimensions = [
+				handleHost.offsetWidth + 'px',
+				handleHost.offsetHeight + 'px',
+				handleHost.offsetLeft + 'px',
+				handleHost.offsetTop + 'px'
+			];
 		}
 
-		function existsInDom( element ) {
-			return element && element.ownerDocument && element.ownerDocument.contains( element );
+		// Make changes to the view only if the resizer should actually get new dimensions.
+		// Otherwise, if View#change() was always called, this would cause EditorUI#update
+		// loops because the WidgetResize plugin listens to EditorUI#update and updates
+		// the resizer.
+		// https://github.com/ckeditor/ckeditor5/issues/7633
+		if ( compareArrays( currentDimensions, newDimensions ) !== 'same' ) {
+			this._options.editor.editing.view.change( writer => {
+				writer.setStyle( {
+					width: newDimensions[ 0 ],
+					height: newDimensions[ 1 ],
+					left: newDimensions[ 2 ],
+					top: newDimensions[ 3 ]
+				}, resizerWrapper );
+			} );
 		}
 	}
 
@@ -514,3 +535,7 @@ function extractCoordinates( event ) {
 		y: event.pageY
 	};
 }
+
+function existsInDom( element ) {
+	return element && element.ownerDocument && element.ownerDocument.contains( element );
+}
diff --git a/packages/ckeditor5-widget/tests/widgetresize/resizer.js b/packages/ckeditor5-widget/tests/widgetresize/resizer.js
@@ -131,6 +131,36 @@ describe( 'Resizer', () => {
 			// Cleanup.
 			renderedElement.remove();
 		} );
+
+		// https://github.com/ckeditor/ckeditor5/issues/7633
+		it( 'should not cause changes in the view unless the host size actually changed', () => {
+			const resizerInstance = createResizer( {
+				getHandleHost: widgetWrapper => widgetWrapper
+			} );
+
+			resizerInstance.attach();
+			const renderedElement = resizerInstance._viewResizerWrapper.render( document );
+
+			document.body.appendChild( renderedElement );
+
+			const viewChangeSpy = sinon.spy( editor.editing.view, 'change' );
+
+			resizerInstance.redraw();
+			sinon.assert.calledOnce( viewChangeSpy );
+
+			resizerInstance.redraw();
+			sinon.assert.calledOnce( viewChangeSpy );
+
+			const host = resizerInstance._getHandleHost();
+
+			host.style.width = '123px';
+
+			resizerInstance.redraw();
+			sinon.assert.calledTwice( viewChangeSpy );
+
+			// Cleanup.
+			renderedElement.remove();
+		} );
 	} );
 
 	describe( '_proposeNewSize()', () => {