From b8d865f6aa2f40ba4dd5d1b3049db5494fc35515 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Tue, 20 Aug 2019 11:23:00 +0200 Subject: [PATCH 01/12] POC of how we can speed up redrawing during resizing. --- src/widgetresize.js | 4 ++-- src/widgetresize/resizer.js | 8 +------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/widgetresize.js b/src/widgetresize.js index 94a1c5bf..593a3b8f 100644 --- a/src/widgetresize.js +++ b/src/widgetresize.js @@ -70,8 +70,8 @@ export default class WidgetResize extends Plugin { } ); const redrawResizers = throttle( () => { - for ( const context of this.resizers ) { - context.redraw(); + if ( this.activeResizer ) { + this.activeResizer.redraw(); } }, THROTTLE_THRESHOLD ); diff --git a/src/widgetresize/resizer.js b/src/widgetresize/resizer.js index 43b24f0a..e4c67a13 100644 --- a/src/widgetresize/resizer.js +++ b/src/widgetresize/resizer.js @@ -119,8 +119,6 @@ export default class Resizer { this._sizeUI.bindToState( this._options, this.state ); this.state.begin( domResizeHandle, this._getHandleHost(), this._getResizeHost() ); - - this.redraw(); } /** @@ -142,17 +140,13 @@ export default class Resizer { newSize.handleHostWidth = Math.round( domHandleHostRect.width ); newSize.handleHostHeight = Math.round( domHandleHostRect.height ); + // Handle max-width limitation. const domResizeHostRect = new Rect( domHandleHost ); newSize.width = Math.round( domResizeHostRect.width ); newSize.height = Math.round( domResizeHostRect.height ); this.state.update( newSize ); - - // Refresh values based on the real image. Real image might be limited by max-width, and thus fetching it - // here will reflect this limitation. - this._domResizerWrapper.style.width = newSize.handleHostWidth + 'px'; - this._domResizerWrapper.style.height = newSize.handleHostHeight + 'px'; } /** From 5ca359edf30703ef96ac62c6ba8ac124a41aecf6 Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Thu, 5 Sep 2019 09:41:55 +0200 Subject: [PATCH 02/12] Internal: Adjusted the order of operations to improve performance. --- src/widgetresize/resizer.js | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/widgetresize/resizer.js b/src/widgetresize/resizer.js index e4c67a13..1895feb8 100644 --- a/src/widgetresize/resizer.js +++ b/src/widgetresize/resizer.js @@ -195,16 +195,23 @@ export default class Resizer { domWrapper.style.width = clientRect.width + 'px'; domWrapper.style.height = clientRect.height + 'px'; + 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 ) ) { - domWrapper.style.left = handleHost.offsetLeft + 'px'; - domWrapper.style.top = handleHost.offsetTop + 'px'; + domWrapper.style.left = offsets.left + 'px'; + domWrapper.style.top = offsets.top + 'px'; - domWrapper.style.height = handleHost.offsetHeight + 'px'; - domWrapper.style.width = handleHost.offsetWidth + 'px'; + domWrapper.style.height = offsets.height + 'px'; + domWrapper.style.width = offsets.width + 'px'; } } From 16596911eb73f09c40127f2de31645805a4877c6 Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Thu, 5 Sep 2019 09:46:13 +0200 Subject: [PATCH 03/12] Internal: Fixed a regression where handles would be mispositioned during the first render. Also improved the performance, editor.ui#update listener is now throttled with a higher interval, and all the necessary redrawing happens in updateSize. --- src/widgetresize.js | 47 +++++++++++++++++++++++++++++++++++-- src/widgetresize/resizer.js | 8 +++++-- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/widgetresize.js b/src/widgetresize.js index 593a3b8f..87152380 100644 --- a/src/widgetresize.js +++ b/src/widgetresize.js @@ -9,6 +9,7 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import Resizer from './widgetresize/resizer'; +import { isWidget } from './utils'; import DomEmitterMixin from '@ckeditor/ckeditor5-utils/src/dom/emittermixin'; import global from '@ckeditor/ckeditor5-utils/src/dom/global'; import { throttle } from 'lodash-es'; @@ -28,8 +29,46 @@ export default class WidgetResize extends Plugin { return 'WidgetResize'; } + _resizeWorkaround() { + this.editor.editing.downcastDispatcher.on( 'selection', ( evt, data, conversionApi ) => { + const viewWriter = conversionApi.writer; + let lastMarked = null; + + for ( const range of viewWriter.document.selection.getRanges() ) { + for ( const value of range ) { + const node = value.item; + + // Do not mark nested widgets in selected one. See: ckeditor/ckeditor5-widget#57. + if ( isWidget( node ) && !isChild( node, lastMarked ) && node.hasClass( 'ck-widget_with-resizer' ) ) { + const resizer = this.resizersByWrapper.get( node ); + + if ( resizer ) { + resizer.redraw(); + } + + lastMarked = node; + } + } + } + }, { priority: 'low' } ); + + // Checks whether the specified `element` is a child of the `parent` element. + // + // @param {module:engine/view/element~Element} element An element to check. + // @param {module:engine/view/element~Element|null} parent A parent for the element. + // @returns {Boolean} + function isChild( element, parent ) { + if ( !parent ) { + return false; + } + + return Array.from( element.getAncestors() ).includes( parent ); + } + } + init() { this.resizers = []; + this.resizersByWrapper = new Map(); this.activeResizer = null; const domDocument = global.window.document; @@ -39,6 +78,8 @@ export default class WidgetResize extends Plugin { isFormatting: true } ); + this._resizeWorkaround(); + this._observer = Object.create( DomEmitterMixin ); this._observer.listenTo( domDocument, 'mousedown', ( event, domEventData ) => { @@ -73,7 +114,7 @@ export default class WidgetResize extends Plugin { if ( this.activeResizer ) { this.activeResizer.redraw(); } - }, THROTTLE_THRESHOLD ); + }, 100 ); // Redrawing on any change of the UI of the editor (including content changes). this.editor.ui.on( 'update', redrawResizers ); @@ -95,10 +136,12 @@ export default class WidgetResize extends Plugin { resizer.attach(); - this.editor.editing.view.once( 'render', () => resizer.redraw() ); + this.editor.editing.view.once( 'render', () => resizer.redraw() ); // no longer needed as we listen for widget selection change this.resizers.push( resizer ); + this.resizersByWrapper.set( options.viewElement, resizer ); + return resizer; } diff --git a/src/widgetresize/resizer.js b/src/widgetresize/resizer.js index 1895feb8..55d6cdef 100644 --- a/src/widgetresize/resizer.js +++ b/src/widgetresize/resizer.js @@ -146,6 +146,8 @@ export default class Resizer { newSize.width = Math.round( domResizeHostRect.width ); newSize.height = Math.round( domResizeHostRect.height ); + this.redraw( domHandleHostRect ); + this.state.update( newSize ); } @@ -181,8 +183,10 @@ export default class Resizer { /** * Redraws the resizer. + * + * @param {module:utils/dom/rect~Rect} handleHostRect */ - redraw() { + redraw( handleHostRect ) { // TODO review this const domWrapper = this._domResizerWrapper; @@ -190,7 +194,7 @@ export default class Resizer { // Refresh only if resizer exists in the DOM. const widgetWrapper = domWrapper.parentElement; const handleHost = this._getHandleHost(); - const clientRect = new Rect( handleHost ); + const clientRect = handleHostRect || new Rect( handleHost ); domWrapper.style.width = clientRect.width + 'px'; domWrapper.style.height = clientRect.height + 'px'; From 0cfe643b70734b1212a7b478556740dac2ac6b2b Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Thu, 5 Sep 2019 11:48:01 +0200 Subject: [PATCH 04/12] Internal: Store currently focused resizer. Looks like current way of storing "active" resizer is now redundant, so very likely I'll replace it soon. --- src/widgetresize.js | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/widgetresize.js b/src/widgetresize.js index 87152380..93f2cb02 100644 --- a/src/widgetresize.js +++ b/src/widgetresize.js @@ -33,6 +33,7 @@ export default class WidgetResize extends Plugin { this.editor.editing.downcastDispatcher.on( 'selection', ( evt, data, conversionApi ) => { const viewWriter = conversionApi.writer; let lastMarked = null; + let activeResizer = null; for ( const range of viewWriter.document.selection.getRanges() ) { for ( const value of range ) { @@ -40,16 +41,18 @@ export default class WidgetResize extends Plugin { // Do not mark nested widgets in selected one. See: ckeditor/ckeditor5-widget#57. if ( isWidget( node ) && !isChild( node, lastMarked ) && node.hasClass( 'ck-widget_with-resizer' ) ) { - const resizer = this.resizersByWrapper.get( node ); - - if ( resizer ) { - resizer.redraw(); - } + activeResizer = this.resizersByWrapper.get( node ) || activeResizer; lastMarked = node; } } } + + if ( activeResizer ) { + activeResizer.redraw(); + } + + this.focusedResizer = activeResizer; }, { priority: 'low' } ); // Checks whether the specified `element` is a child of the `parent` element. @@ -67,12 +70,12 @@ export default class WidgetResize extends Plugin { } init() { + this.focusedResizer = null; this.resizers = []; this.resizersByWrapper = new Map(); this.activeResizer = null; const domDocument = global.window.document; - const THROTTLE_THRESHOLD = 16; // 16ms = ~60fps this.editor.model.schema.setAttributeProperties( 'width', { isFormatting: true @@ -100,7 +103,7 @@ export default class WidgetResize extends Plugin { if ( this.activeResizer ) { this.activeResizer.updateSize( domEventData ); } - }, THROTTLE_THRESHOLD ) ); + }, 16 ) ); // 60 fps this._observer.listenTo( domDocument, 'mouseup', () => { if ( this.activeResizer ) { @@ -110,17 +113,17 @@ export default class WidgetResize extends Plugin { } } ); - const redrawResizers = throttle( () => { - if ( this.activeResizer ) { - this.activeResizer.redraw(); + const redrawFocusedResizer = throttle( () => { + if ( this.focusedResizer ) { + this.focusedResizer.redraw(); } - }, 100 ); + }, 200 ); // 5 fps // Redrawing on any change of the UI of the editor (including content changes). - this.editor.ui.on( 'update', redrawResizers ); + this.editor.ui.on( 'update', redrawFocusedResizer ); // Resizers need to be redrawn upon window resize, because new window might shrink resize host. - this._observer.listenTo( global.window, 'resize', redrawResizers ); + this._observer.listenTo( global.window, 'resize', redrawFocusedResizer ); } destroy() { From efce6009eb109d90384f385e2b3eb6e0d26d0fa4 Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Thu, 5 Sep 2019 13:17:28 +0200 Subject: [PATCH 05/12] Internal: Renamed variable. --- src/widgetresize.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/widgetresize.js b/src/widgetresize.js index 93f2cb02..87268d17 100644 --- a/src/widgetresize.js +++ b/src/widgetresize.js @@ -71,7 +71,6 @@ export default class WidgetResize extends Plugin { init() { this.focusedResizer = null; - this.resizers = []; this.resizersByWrapper = new Map(); this.activeResizer = null; @@ -141,15 +140,13 @@ export default class WidgetResize extends Plugin { this.editor.editing.view.once( 'render', () => resizer.redraw() ); // no longer needed as we listen for widget selection change - this.resizers.push( resizer ); - this.resizersByWrapper.set( options.viewElement, resizer ); return resizer; } _getResizerByHandle( domResizeHandle ) { - for ( const resizer of this.resizers ) { + for ( const resizer of this.resizersByWrapper.values() ) { if ( resizer.containsHandle( domResizeHandle ) ) { return resizer; } From ceede9da07a9849b5e9645662d6c5f2fc6406114 Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Fri, 6 Sep 2019 14:13:15 +0200 Subject: [PATCH 06/12] Intenral: Removed a workaround that is no longer necessary. --- src/widgetresize.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/widgetresize.js b/src/widgetresize.js index 87268d17..ec60798e 100644 --- a/src/widgetresize.js +++ b/src/widgetresize.js @@ -138,8 +138,6 @@ export default class WidgetResize extends Plugin { resizer.attach(); - this.editor.editing.view.once( 'render', () => resizer.redraw() ); // no longer needed as we listen for widget selection change - this.resizersByWrapper.set( options.viewElement, resizer ); return resizer; From 1a0c8639a4610bcc71b7a9e606ca03cb07a328cc Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Tue, 10 Sep 2019 14:21:09 +0200 Subject: [PATCH 07/12] Internal: Added missing docs, reorganized code in WidgetResize plugin, renamed resizersByWrapper to resizers property. --- src/widgetresize.js | 133 +++++++++++++++++++++++++------------------- 1 file changed, 76 insertions(+), 57 deletions(-) diff --git a/src/widgetresize.js b/src/widgetresize.js index ec60798e..10667cc1 100644 --- a/src/widgetresize.js +++ b/src/widgetresize.js @@ -29,50 +29,21 @@ export default class WidgetResize extends Plugin { return 'WidgetResize'; } - _resizeWorkaround() { - this.editor.editing.downcastDispatcher.on( 'selection', ( evt, data, conversionApi ) => { - const viewWriter = conversionApi.writer; - let lastMarked = null; - let activeResizer = null; - - for ( const range of viewWriter.document.selection.getRanges() ) { - for ( const value of range ) { - const node = value.item; - - // Do not mark nested widgets in selected one. See: ckeditor/ckeditor5-widget#57. - if ( isWidget( node ) && !isChild( node, lastMarked ) && node.hasClass( 'ck-widget_with-resizer' ) ) { - activeResizer = this.resizersByWrapper.get( node ) || activeResizer; - - lastMarked = node; - } - } - } - - if ( activeResizer ) { - activeResizer.redraw(); - } - - this.focusedResizer = activeResizer; - }, { priority: 'low' } ); - - // Checks whether the specified `element` is a child of the `parent` element. - // - // @param {module:engine/view/element~Element} element An element to check. - // @param {module:engine/view/element~Element|null} parent A parent for the element. - // @returns {Boolean} - function isChild( element, parent ) { - if ( !parent ) { - return false; - } - - return Array.from( element.getAncestors() ).includes( parent ); - } - } - init() { - this.focusedResizer = null; - this.resizersByWrapper = new Map(); - this.activeResizer = null; + /** + * A map of resizers created using this plugin instance. + * + * @type {Map.} + */ + this.resizers = new Map(); + + /** + * Currently focused widget resizer instance. + * + * @protected + * @type {module:widget/widgetresize/resizer~Resizer|null} + */ + this._focusedResizer = null; const domDocument = global.window.document; @@ -80,10 +51,11 @@ export default class WidgetResize extends Plugin { isFormatting: true } ); - this._resizeWorkaround(); - this._observer = Object.create( DomEmitterMixin ); + // A resizer that is currently used. + let activeResizer; + this._observer.listenTo( domDocument, 'mousedown', ( event, domEventData ) => { if ( !Resizer.isResizeHandle( domEventData.target ) ) { return; @@ -91,30 +63,32 @@ export default class WidgetResize extends Plugin { const resizeHandle = domEventData.target; - this.activeResizer = this._getResizerByHandle( resizeHandle ); + activeResizer = this._getResizerByHandle( resizeHandle ); - if ( this.activeResizer ) { - this.activeResizer.begin( resizeHandle ); + if ( activeResizer ) { + activeResizer.begin( resizeHandle ); } } ); this._observer.listenTo( domDocument, 'mousemove', throttle( ( event, domEventData ) => { - if ( this.activeResizer ) { - this.activeResizer.updateSize( domEventData ); + if ( activeResizer ) { + activeResizer.updateSize( domEventData ); } }, 16 ) ); // 60 fps this._observer.listenTo( domDocument, 'mouseup', () => { - if ( this.activeResizer ) { - this.activeResizer.commit(); + if ( activeResizer ) { + activeResizer.commit(); - this.activeResizer = null; + activeResizer = null; } } ); + this._attachFocusChangeListener(); + const redrawFocusedResizer = throttle( () => { - if ( this.focusedResizer ) { - this.focusedResizer.redraw(); + if ( this._focusedResizer ) { + this._focusedResizer.redraw(); } }, 200 ); // 5 fps @@ -138,13 +112,58 @@ export default class WidgetResize extends Plugin { resizer.attach(); - this.resizersByWrapper.set( options.viewElement, resizer ); + this.resizers.set( options.viewElement, resizer ); return resizer; } + /** + * Adds a listener that keep the track of currently focused resizer. + * + * @private + */ + _attachFocusChangeListener() { + this.editor.editing.downcastDispatcher.on( 'selection', ( evt, data, conversionApi ) => { + const viewWriter = conversionApi.writer; + let lastMarked = null; + let focusedResizer = null; + + for ( const range of viewWriter.document.selection.getRanges() ) { + for ( const value of range ) { + const node = value.item; + + // Do not mark nested widgets in selected one. See: ckeditor/ckeditor5-widget#57. + if ( isWidget( node ) && !isChild( node, lastMarked ) && node.hasClass( 'ck-widget_with-resizer' ) ) { + focusedResizer = this.resizers.get( node ) || focusedResizer; + + lastMarked = node; + } + } + } + + if ( focusedResizer ) { + focusedResizer.redraw(); + } + + this._focusedResizer = focusedResizer; + }, { priority: 'low' } ); + + // Checks whether the specified `element` is a child of the `parent` element. + // + // @param {module:engine/view/element~Element} element An element to check. + // @param {module:engine/view/element~Element|null} parent A parent for the element. + // @returns {Boolean} + function isChild( element, parent ) { + if ( !parent ) { + return false; + } + + return Array.from( element.getAncestors() ).includes( parent ); + } + } + _getResizerByHandle( domResizeHandle ) { - for ( const resizer of this.resizersByWrapper.values() ) { + for ( const resizer of this.resizers.values() ) { if ( resizer.containsHandle( domResizeHandle ) ) { return resizer; } From 2724f314e07dac8a718b93c7b59922c443cf9d41 Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Thu, 10 Oct 2019 11:19:26 +0200 Subject: [PATCH 08/12] Internal: Active resizer is kept now as an observable member. --- src/widgetresize.js | 56 ++++++++++++++++++++++--------------- src/widgetresize/resizer.js | 2 +- 2 files changed, 34 insertions(+), 24 deletions(-) diff --git a/src/widgetresize.js b/src/widgetresize.js index a4ccc5c9..3b855edb 100644 --- a/src/widgetresize.js +++ b/src/widgetresize.js @@ -12,6 +12,8 @@ import Resizer from './widgetresize/resizer'; import { isWidget } from './utils'; import DomEmitterMixin from '@ckeditor/ckeditor5-utils/src/dom/emittermixin'; import global from '@ckeditor/ckeditor5-utils/src/dom/global'; +import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin'; +import mix from '@ckeditor/ckeditor5-utils/src/mix'; import { throttle } from 'lodash-es'; /** @@ -20,6 +22,7 @@ import { throttle } from 'lodash-es'; * Use the {@link module:widget/widgetresize~WidgetResize#attachTo} method to create a resizer for the specified widget. * * @extends module:core/plugin~Plugin + * @mixes module:utils/observablemixin~ObservableMixin */ export default class WidgetResize extends Plugin { /** @@ -38,12 +41,22 @@ export default class WidgetResize extends Plugin { this.resizers = new Map(); /** - * Currently focused widget resizer instance. + * @protected + * @observable + * @type {module:widget/widgetresize/resizer~Resizer|null} Currently visible resizer. + */ + this.set( '_visibleResizer', null ); + + /** + * References an active resizer. + * + * Active resizer means a resizer which handle is actively used by the end user. * * @protected + * @observable * @type {module:widget/widgetresize/resizer~Resizer|null} */ - this._focusedResizer = null; + this.set( '_activeResizer', null ); const domDocument = global.window.document; @@ -53,9 +66,6 @@ export default class WidgetResize extends Plugin { this._observer = Object.create( DomEmitterMixin ); - // A resizer that is currently used. - let activeResizer; - this._observer.listenTo( domDocument, 'mousedown', ( event, domEventData ) => { if ( !Resizer.isResizeHandle( domEventData.target ) ) { return; @@ -63,35 +73,37 @@ export default class WidgetResize extends Plugin { const resizeHandle = domEventData.target; - activeResizer = this._getResizerByHandle( resizeHandle ); + this._activeResizer = this._getResizerByHandle( resizeHandle ); - if ( activeResizer ) { - activeResizer.begin( resizeHandle ); + if ( this._activeResizer ) { + this._activeResizer.begin( resizeHandle ); } } ); this._observer.listenTo( domDocument, 'mousemove', throttle( ( event, domEventData ) => { - if ( activeResizer ) { - activeResizer.updateSize( domEventData ); + if ( this._activeResizer ) { + this._activeResizer.updateSize( domEventData ); } }, 16 ) ); // 60 fps this._observer.listenTo( domDocument, 'mouseup', () => { - if ( activeResizer ) { - activeResizer.commit(); + if ( this._activeResizer ) { + this._activeResizer.commit(); - activeResizer = null; + this._activeResizer = null; } } ); this._attachFocusChangeListener(); const redrawFocusedResizer = throttle( () => { - if ( this._focusedResizer ) { - this._focusedResizer.redraw(); + if ( this._visibleResizer ) { + this._visibleResizer.redraw(); } }, 200 ); // 5 fps + this.on( 'change:_visibleResizer', redrawFocusedResizer ); + // Redrawing on any change of the UI of the editor (including content changes). this.editor.ui.on( 'update', redrawFocusedResizer ); @@ -118,7 +130,7 @@ export default class WidgetResize extends Plugin { } /** - * Adds a listener that keep the track of currently focused resizer. + * Listens for selection change and sets the visible resizer accordingly. * * @private */ @@ -126,7 +138,7 @@ export default class WidgetResize extends Plugin { this.editor.editing.downcastDispatcher.on( 'selection', ( evt, data, conversionApi ) => { const viewWriter = conversionApi.writer; let lastMarked = null; - let focusedResizer = null; + let matchedResizer = null; for ( const range of viewWriter.document.selection.getRanges() ) { for ( const value of range ) { @@ -134,18 +146,14 @@ export default class WidgetResize extends Plugin { // Do not mark nested widgets in selected one. See: ckeditor/ckeditor5-widget#57. if ( isWidget( node ) && !isChild( node, lastMarked ) && node.hasClass( 'ck-widget_with-resizer' ) ) { - focusedResizer = this.resizers.get( node ) || focusedResizer; + matchedResizer = this.resizers.get( node ) || matchedResizer; lastMarked = node; } } } - if ( focusedResizer ) { - focusedResizer.redraw(); - } - - this._focusedResizer = focusedResizer; + this._visibleResizer = matchedResizer; }, { priority: 'low' } ); // Checks whether the specified `element` is a child of the `parent` element. @@ -171,6 +179,8 @@ export default class WidgetResize extends Plugin { } } +mix( WidgetResize, ObservableMixin ); + /** * Interface describing a resizer. It allows to specify the resizing host, custom logic for calculating aspect ratio, etc. * diff --git a/src/widgetresize/resizer.js b/src/widgetresize/resizer.js index b9cef5e8..91169731 100644 --- a/src/widgetresize/resizer.js +++ b/src/widgetresize/resizer.js @@ -184,7 +184,7 @@ export default class Resizer { /** * Redraws the resizer. * - * @param {module:utils/dom/rect~Rect} handleHostRect + * @param {module:utils/dom/rect~Rect} [handleHostRect] Handle host rectangle might be given to improve performance. */ redraw( handleHostRect ) { // TODO review this From 18059c817aef1e0059e6780f543e4baa2aafa567 Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Thu, 10 Oct 2019 14:30:20 +0200 Subject: [PATCH 09/12] Internal: WidgetResize._resizers is now a private member. Simplified the code. --- src/widgetresize.js | 83 ++++++++++++++++++--------------------------- 1 file changed, 33 insertions(+), 50 deletions(-) diff --git a/src/widgetresize.js b/src/widgetresize.js index 3b855edb..0131395f 100644 --- a/src/widgetresize.js +++ b/src/widgetresize.js @@ -9,7 +9,6 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import Resizer from './widgetresize/resizer'; -import { isWidget } from './utils'; import DomEmitterMixin from '@ckeditor/ckeditor5-utils/src/dom/emittermixin'; import global from '@ckeditor/ckeditor5-utils/src/dom/global'; import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin'; @@ -33,13 +32,6 @@ export default class WidgetResize extends Plugin { } init() { - /** - * A map of resizers created using this plugin instance. - * - * @type {Map.} - */ - this.resizers = new Map(); - /** * @protected * @observable @@ -58,6 +50,14 @@ export default class WidgetResize extends Plugin { */ this.set( '_activeResizer', null ); + /** + * A map of resizers created using this plugin instance. + * + * @private + * @type {Map.} + */ + this._resizers = new Map(); + const domDocument = global.window.document; this.editor.model.schema.setAttributeProperties( 'width', { @@ -94,8 +94,6 @@ export default class WidgetResize extends Plugin { } } ); - this._attachFocusChangeListener(); - const redrawFocusedResizer = throttle( () => { if ( this._visibleResizer ) { this._visibleResizer.redraw(); @@ -109,6 +107,14 @@ export default class WidgetResize extends Plugin { // Resizers need to be redrawn upon window resize, because new window might shrink resize host. this._observer.listenTo( global.window, 'resize', redrawFocusedResizer ); + + const viewSelection = this.editor.editing.view.document.selection; + + viewSelection.on( 'change', () => { + const selectedElement = viewSelection.getSelectedElement(); + + this._visibleResizer = this._getResizerByViewElement( selectedElement ) || null; + } ); } destroy() { @@ -124,59 +130,36 @@ export default class WidgetResize extends Plugin { resizer.attach(); - this.resizers.set( options.viewElement, resizer ); + this._resizers.set( options.viewElement, resizer ); return resizer; } /** - * Listens for selection change and sets the visible resizer accordingly. + * Returns a resizer that contains a given resize handle. * - * @private + * @protected + * @param {HTMLElement} domResizeHandle + * @returns {module:widget/widgetresize/resizer~Resizer} */ - _attachFocusChangeListener() { - this.editor.editing.downcastDispatcher.on( 'selection', ( evt, data, conversionApi ) => { - const viewWriter = conversionApi.writer; - let lastMarked = null; - let matchedResizer = null; - - for ( const range of viewWriter.document.selection.getRanges() ) { - for ( const value of range ) { - const node = value.item; - - // Do not mark nested widgets in selected one. See: ckeditor/ckeditor5-widget#57. - if ( isWidget( node ) && !isChild( node, lastMarked ) && node.hasClass( 'ck-widget_with-resizer' ) ) { - matchedResizer = this.resizers.get( node ) || matchedResizer; - - lastMarked = node; - } - } - } - - this._visibleResizer = matchedResizer; - }, { priority: 'low' } ); - - // Checks whether the specified `element` is a child of the `parent` element. - // - // @param {module:engine/view/element~Element} element An element to check. - // @param {module:engine/view/element~Element|null} parent A parent for the element. - // @returns {Boolean} - function isChild( element, parent ) { - if ( !parent ) { - return false; - } - - return Array.from( element.getAncestors() ).includes( parent ); - } - } - _getResizerByHandle( domResizeHandle ) { - for ( const resizer of this.resizers.values() ) { + for ( const resizer of this._resizers.values() ) { if ( resizer.containsHandle( domResizeHandle ) ) { return resizer; } } } + + /** + * Returns a resizer created for a given view element (widget element). + * + * @protected + * @param {module:engine/view/containerelement~ContainerElement} viewElement + * @returns {module:widget/widgetresize/resizer~Resizer} + */ + _getResizerByViewElement( viewElement ) { + return this._resizers.get( viewElement ); + } } mix( WidgetResize, ObservableMixin ); From 572672715075c3d768ba27f8cd010c7b2c6f0a89 Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Mon, 14 Oct 2019 10:02:57 +0200 Subject: [PATCH 10/12] Internal: Visible resizer change should not be throttled. --- src/widgetresize.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/widgetresize.js b/src/widgetresize.js index 0131395f..f9f019f8 100644 --- a/src/widgetresize.js +++ b/src/widgetresize.js @@ -94,19 +94,23 @@ export default class WidgetResize extends Plugin { } } ); - const redrawFocusedResizer = throttle( () => { + const redrawFocusedResizer = () => { if ( this._visibleResizer ) { this._visibleResizer.redraw(); } - }, 200 ); // 5 fps + }; // 5 fps + const redrawFocusedResizerThrottled = throttle( redrawFocusedResizer, 16 ); // ~60fps + + // Redraws occurring upon change of visible resizer must not be throttled, as it is crucial for the initial + // render. Without it the resizer frame would be misaligned with resizing host for a fraction of second. this.on( 'change:_visibleResizer', redrawFocusedResizer ); // Redrawing on any change of the UI of the editor (including content changes). - this.editor.ui.on( 'update', redrawFocusedResizer ); + this.editor.ui.on( 'update', redrawFocusedResizerThrottled ); // Resizers need to be redrawn upon window resize, because new window might shrink resize host. - this._observer.listenTo( global.window, 'resize', redrawFocusedResizer ); + this._observer.listenTo( global.window, 'resize', redrawFocusedResizerThrottled ); const viewSelection = this.editor.editing.view.document.selection; From ace08b2b7550025d89b822e6d865d12e25ba01d2 Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Mon, 14 Oct 2019 10:18:29 +0200 Subject: [PATCH 11/12] Internal: Removed throttling for mousemove event. --- src/widgetresize.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/widgetresize.js b/src/widgetresize.js index f9f019f8..5493cf50 100644 --- a/src/widgetresize.js +++ b/src/widgetresize.js @@ -80,11 +80,11 @@ export default class WidgetResize extends Plugin { } } ); - this._observer.listenTo( domDocument, 'mousemove', throttle( ( event, domEventData ) => { + this._observer.listenTo( domDocument, 'mousemove', ( event, domEventData ) => { if ( this._activeResizer ) { this._activeResizer.updateSize( domEventData ); } - }, 16 ) ); // 60 fps + } ); this._observer.listenTo( domDocument, 'mouseup', () => { if ( this._activeResizer ) { From f7de63de5210f258e89a0787664bc54f612beff8 Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Mon, 14 Oct 2019 16:16:47 +0200 Subject: [PATCH 12/12] Internal: Minor cleanup. --- src/widgetresize.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/widgetresize.js b/src/widgetresize.js index 5493cf50..066ea063 100644 --- a/src/widgetresize.js +++ b/src/widgetresize.js @@ -33,9 +33,11 @@ export default class WidgetResize extends Plugin { init() { /** + * The currently visible resizer. + * * @protected * @observable - * @type {module:widget/widgetresize/resizer~Resizer|null} Currently visible resizer. + * @member {module:widget/widgetresize/resizer~Resizer|null} #_visibleResizer */ this.set( '_visibleResizer', null ); @@ -46,7 +48,7 @@ export default class WidgetResize extends Plugin { * * @protected * @observable - * @type {module:widget/widgetresize/resizer~Resizer|null} + * @member {module:widget/widgetresize/resizer~Resizer|null} #_activeResizer */ this.set( '_activeResizer', null ); @@ -98,11 +100,11 @@ export default class WidgetResize extends Plugin { if ( this._visibleResizer ) { this._visibleResizer.redraw(); } - }; // 5 fps + }; - const redrawFocusedResizerThrottled = throttle( redrawFocusedResizer, 16 ); // ~60fps + const redrawFocusedResizerThrottled = throttle( redrawFocusedResizer, 200 ); // 5fps - // Redraws occurring upon change of visible resizer must not be throttled, as it is crucial for the initial + // Redraws occurring upon a change of visible resizer must not be throttled, as it is crucial for the initial // render. Without it the resizer frame would be misaligned with resizing host for a fraction of second. this.on( 'change:_visibleResizer', redrawFocusedResizer );