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

Commit

Permalink
Merge pull request #122 from ckeditor/i/6060-cleanup
Browse files Browse the repository at this point in the history
Fix: Image resize now cleans up temporary view `width` style changes. Closes ckeditor/ckeditor5#6060.
  • Loading branch information
Reinmar authored Apr 22, 2020
2 parents 6273527 + de2922f commit 92226f9
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 3 deletions.
8 changes: 8 additions & 0 deletions src/widgetresize.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ export default class WidgetResize extends Plugin {
return 'WidgetResize';
}

/**
* @inheritDoc
*/
init() {
/**
* The currently visible resizer.
Expand Down Expand Up @@ -103,6 +106,9 @@ export default class WidgetResize extends Plugin {
} );
}

/**
* @inheritDoc
*/
destroy() {
this._observer.stopListening();

Expand Down Expand Up @@ -227,6 +233,8 @@ mix( WidgetResize, ObservableMixin );
*/

/**
* A view of an element to be resized. Typically it's the main widget's view instance.
*
* @member {module:engine/view/containerelement~ContainerElement} module:widget/widgetresize~ResizerOptions#viewElement
*/

Expand Down
23 changes: 20 additions & 3 deletions src/widgetresize/resizer.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,13 @@ export default class Resizer {
*/
this._viewResizerWrapper = null;

/**
* The width of the resized {@link module:widget/widgetresize~ResizerOptions#viewElement viewElement} before the resizing started.
*
* @private
* @member {Number|String|undefined} _initialViewWidth
*/

/**
* @observable
*/
Expand Down Expand Up @@ -139,6 +146,8 @@ export default class Resizer {

this._sizeUI.bindToState( this._options, this.state );

this._initialViewWidth = this._options.viewElement.getStyle( 'width' );

this.state.begin( domResizeHandle, this._getHandleHost(), this._getResizeHost() );
}

Expand Down Expand Up @@ -188,9 +197,11 @@ export default class Resizer {
const unit = this._options.unit || '%';
const newValue = ( unit === '%' ? this.state.proposedWidthPercents : this.state.proposedWidth ) + unit;

this._options.onCommit( newValue );

this._cleanup();
// Both cleanup and onCommit callback are very likely to make view changes. Ensure that it is made in a single step.
this._options.editor.editing.view.change( () => {
this._cleanup();
this._options.onCommit( newValue );
} );
}

/**
Expand Down Expand Up @@ -269,6 +280,12 @@ export default class Resizer {
_cleanup() {
this._sizeUI.dismiss();
this._sizeUI.isVisible = false;

const editingView = this._options.editor.editing.view;

editingView.change( writer => {
writer.setStyle( 'width', this._initialViewWidth, this._options.viewElement );
} );
}

/**
Expand Down
41 changes: 41 additions & 0 deletions tests/widgetresize.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { resizerMouseSimulator, focusEditor, getHandleCenterPoint, getWidgetDomP

describe( 'WidgetResize', () => {
let editor, editorElement, widget, mouseListenerSpies, commitStub;
const INITIAL_WIDGET_WIDTH = '25%';

beforeEach( async () => {
editorElement = createEditorElement();
Expand Down Expand Up @@ -377,6 +378,24 @@ describe( 'WidgetResize', () => {
expect( commitStub.callCount, 'call count' ).to.be.eql( 0 );
} );

it( 'restores the original view width after resize is done', () => {
// Note that ultimately width should be changed, but through a model converter, not with direct view changes (#6060).
createResizer();
const renderListener = sinon.stub();

const usedHandle = 'bottom-right';
const domParts = getWidgetDomParts( editor, widget, usedHandle );
const finalPointerPosition = getHandleCenterPoint( domParts.widget, usedHandle ).moveBy( 40, 40 );

editor.editing.view.on( 'render', renderListener );

resizerMouseSimulator.dragTo( editor, domParts.resizeHandle, finalPointerPosition );

expect( widget.getStyle( 'width' ) ).to.equal( INITIAL_WIDGET_WIDTH );
// Verify render count https://github.com/ckeditor/ckeditor5-widget/pull/122#issuecomment-617012777.
expect( renderListener.callCount ).to.be.equal( 3 );
} );

it( 'returns proper value when resize host is different from widget wrapper', () => {
createResizer( {
unit: undefined,
Expand Down Expand Up @@ -466,6 +485,28 @@ describe( 'WidgetResize', () => {
} );
} );

describe( 'cancel()', () => {
it( 'restores original view width', () => {
const resizer = createResizer();

const usedHandle = 'bottom-right';
const domParts = getWidgetDomParts( editor, widget, usedHandle );
const startingPosition = getHandleCenterPoint( domParts.widget, usedHandle ).moveBy( 100, 0 );
const domTarget = domParts.resizeHandle;

resizerMouseSimulator.down( editor, domTarget );
resizerMouseSimulator.move( editor, domTarget, {
pageX: startingPosition.x,
pageY: startingPosition.y
} );

resizer.cancel();

// Value should be restored to the initial value (#6060).
expect( widget.getStyle( 'width' ) ).to.be.equal( INITIAL_WIDGET_WIDTH );
} );
} );

describe( '_getResizerByHandle()', () => {
it( 'returns properly in case of invalid handle element', () => {
const randomElement = document.createElement( 'span' );
Expand Down

0 comments on commit 92226f9

Please sign in to comment.