Parcourir la source

Improved toolbar to work with collaborative editing.

Oskar Wróbel il y a 8 ans
Parent
commit
6208e76d34

+ 68 - 29
packages/ckeditor5-ui/src/toolbar/contextualtoolbar.js

@@ -47,6 +47,19 @@ export default class ContextualToolbar extends Plugin {
 		 */
 		this._balloon = this.editor.plugins.get( ContextualBalloon );
 
+		/**
+		 * This is internal plugin event which is fired 200 ms after selection last change (lodash#debounce).
+		 * This is to makes easy test debounced action without need to use `setTimeout`. Lodash keeps time related
+		 * stuff in a closure and it's not possible to override it by sinon fake timers.
+		 *
+		 * This debounced function is stored as a plugin property to make possible to cancel
+		 * trailing debounced invocation on destroy.
+		 *
+		 * @private
+		 * @member {Function}
+		 */
+		this._fireChangeDoneDebounced = debounce( () => this.fire( '_selectionChangeDone' ), 200 );
+
 		// Attach lifecycle actions.
 		this._handleSelectionChange();
 		this._handleFocusChange();
@@ -93,21 +106,19 @@ export default class ContextualToolbar extends Plugin {
 	_handleSelectionChange() {
 		const selection = this.editor.document.selection;
 
-		// This is internal plugin event which is fired 200 ms after selection last change (lodash#debounce).
-		// This is to makes easy test debounced action without need to use `setTimeout`.
-		// Because lodash keeps time related stuff in a closure it's not possible to override it
-		// by sinon fake timers.
-		const fireChangeDoneDebounced = debounce( () => this.fire( '_selectionChangeDone' ), 200 );
-
-		this.listenTo( selection, 'change:range', () => {
-			// Hide the toolbar when the selection starts changing.
-			this._hidePanel();
+		this.listenTo( selection, 'change:range', ( evt, data ) => {
+			// When the selection is not changed by a collaboration and when is not collapsed.
+			if ( data.directChange || selection.isCollapsed ) {
+				// Hide the toolbar when the selection starts changing.
+				this._hidePanel();
+			}
 
-			// Show the toolbar attached to the selection when the selection stops changing.
-			fireChangeDoneDebounced();
+			// Fire internal `_selectionChangeDone` when the selection stops changing.
+			this._fireChangeDoneDebounced();
 		} );
 
-		this.on( '_selectionChangeDone', () => this._showPanel() );
+		// Hide the toolbar when the selection stops changing.
+		this.listenTo( this, '_selectionChangeDone', () => this._showPanel() );
 	}
 
 	/**
@@ -128,26 +139,15 @@ export default class ContextualToolbar extends Plugin {
 			return;
 		}
 
-		// Get direction of the selection.
-		const isBackward = editingView.selection.isBackward;
-
-		// getBoundingClientRect() makes no sense when the selection spans across number
-		// of lines of text. Using getClientRects() allows us to browse micro–ranges
-		// that would normally make up the bounding client rect.
-		const rangeRects = editingView.domConverter.viewRangeToDom( editingView.selection.getFirstRange() ).getClientRects();
-
-		// Select the proper range rect depending on the direction of the selection.
-		const rangeRect = isBackward ? rangeRects.item( 0 ) : rangeRects.item( rangeRects.length - 1 );
-
 		// Add panel to the common editor contextual balloon.
 		this._balloon.add( {
 			view: this.toolbarView,
-			position: {
-				target: rangeRect,
-				positions: isBackward ?
-					[ positions.backwardSelection, positions.backwardSelectionAlternative ] :
-					[ positions.forwardSelection, positions.forwardSelectionAlternative ],
-			}
+			position: this._getBalloonPositionData()
+		} );
+
+		// Update panel position when selection changes while balloon will be opened (by a collaboration).
+		this.listenTo( this.editor.editing.view, 'render', () => {
+			this._balloon.updatePosition( this._getBalloonPositionData() );
 		} );
 	}
 
@@ -158,9 +158,48 @@ export default class ContextualToolbar extends Plugin {
 	 */
 	_hidePanel() {
 		if ( this._balloon.hasView( this.toolbarView ) ) {
+			this.stopListening( this.editor.editing.view, 'render' );
 			this._balloon.remove( this.toolbarView );
 		}
 	}
+
+	/**
+	 * Returns positioning options for the {@link #_balloon}. They control the way balloon is attached
+	 * to the selection.
+	 *
+	 * @private
+	 * @returns {module:utils/dom/position~Options}
+	 */
+	_getBalloonPositionData() {
+		const editingView = this.editor.editing.view;
+
+		// Get direction of the selection.
+		const isBackward = editingView.selection.isBackward;
+
+		// getBoundingClientRect() makes no sense when the selection spans across number
+		// of lines of text. Using getClientRects() allows us to browse micro–ranges
+		// that would normally make up the bounding client rect.
+		const rangeRects = editingView.domConverter.viewRangeToDom( editingView.selection.getFirstRange() ).getClientRects();
+
+		// Select the proper range rect depending on the direction of the selection.
+		const rangeRect = isBackward ? rangeRects.item( 0 ) : rangeRects.item( rangeRects.length - 1 );
+
+		return {
+			target: rangeRect,
+			positions: isBackward ?
+				[ positions.backwardSelection, positions.backwardSelectionAlternative ] :
+				[ positions.forwardSelection, positions.forwardSelectionAlternative ],
+		};
+	}
+
+	/**
+	 * @inheritDoc
+	 */
+	destroy() {
+		this._fireChangeDoneDebounced.cancel();
+		this.stopListening();
+		super.destroy();
+	}
 }
 
 // List of available toolbar positions.

+ 98 - 8
packages/ckeditor5-ui/tests/toolbar/contextualtoolbar.js

@@ -44,7 +44,8 @@ describe( 'ContextualToolbar', () => {
 
 	afterEach( () => {
 		sandbox.restore();
-		editor.destroy();
+
+		return editor.destroy();
 	} );
 
 	it( 'should be loaded', () => {
@@ -63,7 +64,7 @@ describe( 'ContextualToolbar', () => {
 		expect( contextualToolbar.toolbarView.items ).to.length( 2 );
 	} );
 
-	it( 'should fire internal `_selectionChangeDone` event 200 ms after last selection change', () => {
+	it( 'should fire internal `_selectionChangeDone` event 200 ms after last selection change', ( done ) => {
 		// This test uses setTimeout to test lodash#debounce because sinon fake timers
 		// doesn't work with lodash. Lodash keeps time related stuff in a closure
 		// and sinon is not able to override it.
@@ -72,7 +73,7 @@ describe( 'ContextualToolbar', () => {
 		setData( editor.document, '<paragraph>[bar]</paragraph>' );
 		contextualToolbar.on( '_selectionChangeDone', spy );
 
-		editor.document.selection.fire( 'change:range' );
+		editor.document.selection.fire( 'change:range', {} );
 
 		// Not yet.
 		sinon.assert.notCalled( spy );
@@ -83,7 +84,7 @@ describe( 'ContextualToolbar', () => {
 			sinon.assert.notCalled( spy );
 
 			// Fire event one more time.
-			editor.document.selection.fire( 'change:range' );
+			editor.document.selection.fire( 'change:range', {} );
 
 			// Another 100 ms waiting.
 			setTimeout( () => {
@@ -94,6 +95,7 @@ describe( 'ContextualToolbar', () => {
 				setTimeout( () => {
 					// And here it is.
 					sinon.assert.calledOnce( spy );
+					done();
 				}, 100 );
 			}, 100 );
 		}, 100 );
@@ -109,14 +111,40 @@ describe( 'ContextualToolbar', () => {
 		expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView );
 	} );
 
-	it( 'should close when selection starts changing', () => {
+	it( 'should close when selection starts changing by a directChange', () => {
 		setData( editor.document, '<paragraph>[bar]</paragraph>' );
 
 		contextualToolbar.fire( '_selectionChangeDone' );
 
 		expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView );
 
-		editor.document.selection.fire( 'change:range' );
+		editor.document.selection.fire( 'change:range', { directChange: true } );
+
+		expect( balloon.visibleView ).to.null;
+	} );
+
+	it( 'should not close when selection starts changing by not a directChange', () => {
+		setData( editor.document, '<paragraph>[bar]</paragraph>' );
+
+		contextualToolbar.fire( '_selectionChangeDone' );
+
+		expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView );
+
+		editor.document.selection.fire( 'change:range', { directChange: false } );
+
+		expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView );
+	} );
+
+	it( 'should close when selection starts changing by not a directChange but will become collapsed', () => {
+		setData( editor.document, '<paragraph>[bar]</paragraph>' );
+
+		contextualToolbar.fire( '_selectionChangeDone' );
+
+		// Collapse range silently (without firing `change:range` { directChange: true } event).
+		const range = editor.document.selection._ranges[ 0 ];
+		range.end = range.start;
+
+		editor.document.selection.fire( 'change:range', { directChange: false } );
 
 		expect( balloon.visibleView ).to.null;
 	} );
@@ -124,6 +152,14 @@ describe( 'ContextualToolbar', () => {
 	it( 'should open below if the selection is forward', () => {
 		setData( editor.document, '<paragraph>[bar]</paragraph>' );
 
+		// Mock limiter.
+		mockBoundingBox( document.body, {
+			left: 0,
+			width: 1000,
+			top: 0,
+			height: 1000
+		} );
+
 		contextualToolbar.fire( '_selectionChangeDone' );
 
 		expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView );
@@ -150,6 +186,14 @@ describe( 'ContextualToolbar', () => {
 	it( 'should open above if the selection is backward', () => {
 		setData( editor.document, '<paragraph>[bar]</paragraph>', { lastRangeBackward: true } );
 
+		// Mock limiter.
+		mockBoundingBox( document.body, {
+			left: 0,
+			width: 1000,
+			top: 0,
+			height: 1000
+		} );
+
 		contextualToolbar.fire( '_selectionChangeDone' );
 
 		expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView );
@@ -176,12 +220,12 @@ describe( 'ContextualToolbar', () => {
 	it( 'should not open if the collapsed selection is moving', () => {
 		setData( editor.document, '<paragraph>ba[]r</paragraph>' );
 
-		editor.document.selection.fire( 'change:range' );
+		editor.document.selection.fire( 'change:range', {} );
 		contextualToolbar.fire( '_selectionChangeDone' );
 
 		setData( editor.document, '<paragraph>b[]ar</paragraph>' );
 
-		editor.document.selection.fire( 'change:range' );
+		editor.document.selection.fire( 'change:range', {} );
 		contextualToolbar.fire( '_selectionChangeDone' );
 
 		expect( balloon.visibleView ).to.null;
@@ -212,6 +256,52 @@ describe( 'ContextualToolbar', () => {
 		} ).to.not.throw();
 	} );
 
+	it( 'should update balloon position when toolbar is opened and editor content has changed', () => {
+		const spy = sandbox.spy( balloon, 'updatePosition' );
+
+		setData( editor.document, '<paragraph>[bar]</paragraph>' );
+
+		contextualToolbar.fire( '_selectionChangeDone' );
+
+		sinon.assert.notCalled( spy );
+
+		editor.editing.view.fire( 'render' );
+
+		sinon.assert.calledOnce( spy );
+	} );
+
+	it( 'should update balloon position when toolbar is closed', () => {
+		const spy = sandbox.spy( balloon, 'updatePosition' );
+
+		setData( editor.document, '<paragraph>[bar]</paragraph>' );
+
+		contextualToolbar.fire( '_selectionChangeDone' );
+
+		// Hide toolbar.
+		editor.document.selection.fire( 'change:range', { directChange: true } );
+
+		editor.editing.view.fire( 'render' );
+
+		sinon.assert.notCalled( spy );
+	} );
+
+	describe( 'destroy()', () => {
+		it( 'should not fire `_selectionChangeDone` after plugin destroy', ( done ) => {
+			const spy = sandbox.spy();
+
+			contextualToolbar.on( '_selectionChangeDone', spy );
+
+			editor.document.selection.fire( 'change:range', { directChange: true } );
+
+			contextualToolbar.destroy();
+
+			setTimeout( () => {
+				sinon.assert.notCalled( spy );
+				done();
+			}, 200 );
+		} );
+	} );
+
 	function stubClientRects() {
 		const editingView = editor.editing.view;
 		const originalViewRangeToDom = editingView.domConverter.viewRangeToDom;