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

Commit

Permalink
Fix: The Mention UI will use ContextualBalloon plugin to display to…
Browse files Browse the repository at this point in the history
… prevent balloon collisions with other features. Closes #27.

BREAKING CHANGE: The `MentionUI#panelView` property is removed. The Mention feature now uses the `ContextualBalloon` plugin.
  • Loading branch information
mlewand authored May 7, 2019
2 parents 5f199de + 178182b commit 9ae7f30
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 75 deletions.
119 changes: 68 additions & 51 deletions src/mentionui.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@
import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview';
import Collection from '@ckeditor/ckeditor5-utils/src/collection';
import BalloonPanelView from '@ckeditor/ckeditor5-ui/src/panel/balloon/balloonpanelview';
import clickOutsideHandler from '@ckeditor/ckeditor5-ui/src/bindings/clickoutsidehandler';
import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect';
import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
import ContextualBalloon from '@ckeditor/ckeditor5-ui/src/panel/balloon/contextualballoon';

import TextWatcher from './textwatcher';

Expand Down Expand Up @@ -43,13 +43,6 @@ export default class MentionUI extends Plugin {
constructor( editor ) {
super( editor );

/**
* The balloon panel view containing the mention view.
*
* @type {module:ui/panel/balloon/balloonpanelview~BalloonPanelView}
*/
this.panelView = this._creatPanelView();

/**
* The mention view.
*
Expand All @@ -73,9 +66,19 @@ export default class MentionUI extends Plugin {
* @inheritDoc
*/
init() {
const editor = this.editor;

/**
* The contextual balloon plugin instance.
*
* @private
* @member {module:ui/panel/balloon/contextualballoon~ContextualBalloon}
*/
this._balloon = editor.plugins.get( ContextualBalloon );

// Key listener that handles navigation in mention view.
this.editor.editing.view.document.on( 'keydown', ( evt, data ) => {
if ( isHandledKey( data.keyCode ) && this.panelView.isVisible ) {
editor.editing.view.document.on( 'keydown', ( evt, data ) => {
if ( isHandledKey( data.keyCode ) && this._isUIVisible ) {
data.preventDefault();
evt.stop(); // Required for Enter key overriding.

Expand All @@ -92,20 +95,20 @@ export default class MentionUI extends Plugin {
}

if ( data.keyCode == keyCodes.esc ) {
this._hidePanelAndRemoveMarker();
this._hideUIAndRemoveMarker();
}
}
}, { priority: 'highest' } ); // Required to override the Enter key.

// Close the #panelView upon clicking outside of the plugin UI.
// Close the dropdown upon clicking outside of the plugin UI.
clickOutsideHandler( {
emitter: this.panelView,
contextElements: [ this.panelView.element ],
activator: () => this.panelView.isVisible,
callback: () => this._hidePanelAndRemoveMarker()
emitter: this._mentionsView,
activator: () => this._isUIVisible,
contextElements: [ this._balloon.view.element ],
callback: () => this._hideUIAndRemoveMarker()
} );

const feeds = this.editor.config.get( 'mention.feeds' );
const feeds = editor.config.get( 'mention.feeds' );

for ( const mentionDescription of feeds ) {
const feed = mentionDescription.feed;
Expand Down Expand Up @@ -145,24 +148,26 @@ export default class MentionUI extends Plugin {
super.destroy();

// Destroy created UI components as they are not automatically destroyed (see ckeditor5#1341).
this.panelView.destroy();
this._mentionsView.destroy();
}

/**
* Creates the {@link #panelView}.
*
* @private
* @returns {module:ui/panel/balloon/balloonpanelview~BalloonPanelView}
* @inheritDoc
*/
_creatPanelView() {
const panelView = new BalloonPanelView( this.editor.locale );

panelView.withArrow = false;
panelView.render();

this.editor.ui.view.body.add( panelView );
static get requires() {
return [ ContextualBalloon ];
}

return panelView;
/**
* Returns true when {@link #_mentionsView} is in the {@link module:ui/panel/balloon/contextualballoon~ContextualBalloon} and it is
* currently visible.
*
* @readonly
* @protected
* @type {Boolean}
*/
get _isUIVisible() {
return this._balloon.visibleView === this._mentionsView;
}

/**
Expand All @@ -178,8 +183,6 @@ export default class MentionUI extends Plugin {

this._items = new Collection();

this.panelView.content.add( mentionsView );

mentionsView.items.bindTo( this._items ).using( data => {
const { item, marker } = data;

Expand Down Expand Up @@ -222,7 +225,7 @@ export default class MentionUI extends Plugin {
const start = end.getShiftedBy( -matchedTextLength );
const range = model.createRange( start, end );

this._hidePanelAndRemoveMarker();
this._hideUIAndRemoveMarker();

editor.execute( 'mention', {
mention: item,
Expand Down Expand Up @@ -328,15 +331,15 @@ export default class MentionUI extends Plugin {
}

if ( this._items.length ) {
this._showPanel( mentionMarker );
this._showUI( mentionMarker );
} else {
this._hidePanelAndRemoveMarker();
this._hideUIAndRemoveMarker();
}
} );
} );

watcher.on( 'unmatched', () => {
this._hidePanelAndRemoveMarker();
this._hideUIAndRemoveMarker();
} );

return watcher;
Expand All @@ -356,31 +359,44 @@ export default class MentionUI extends Plugin {
}

/**
* Shows the {@link #panelView}. If the panel is already visible, it will reposition it.
* Shows the mentions balloon. If the panel is already visible, it will reposition it.
*
* @private
*/
_showPanel( markerMarker ) {
this.panelView.pin( this._getBalloonPanelPositionData( markerMarker, this.panelView.position ) );
this.panelView.show();
_showUI( markerMarker ) {
if ( this._isUIVisible ) {
// Update balloon position as the mention list view may change its size.
this._balloon.updatePosition( this._getBalloonPanelPositionData( markerMarker, this._mentionsView.position ) );
} else {
this._balloon.add( {
view: this._mentionsView,
position: this._getBalloonPanelPositionData( markerMarker, this._mentionsView.position ),
withArrow: false
} );
}

this._mentionsView.position = this._balloon.view.position;

this._mentionsView.selectFirst();
}

/**
* Hides the {@link #panelView} and removes the 'mention' marker from the markers collection.
* Hides the mentions balloon and removes the 'mention' marker from the markers collection.
*
* @private
*/
_hidePanelAndRemoveMarker() {
_hideUIAndRemoveMarker() {
if ( this.editor.model.markers.has( 'mention' ) ) {
this.editor.model.change( writer => writer.removeMarker( 'mention' ) );
}

this.panelView.unpin();
if ( this._isUIVisible ) {
this._balloon.remove( this._mentionsView );
}

// Make the last matched position on panel view undefined so the #_getBalloonPanelPositionData() method will return all positions
// on the next call.
this.panelView.position = undefined;
this.panelView.hide();
this._mentionsView.position = undefined;
}

/**
Expand Down Expand Up @@ -425,11 +441,11 @@ export default class MentionUI extends Plugin {
* Creates a position options object used to position the balloon panel.
*
* @param {module:engine/model/markercollection~Marker} mentionMarker
* @param {String|undefined} positionName The name of the last matched position name.
* @param {String|undefined} preferredPosition The name of the last matched position name.
* @returns {module:utils/dom/position~Options}
* @private
*/
_getBalloonPanelPositionData( mentionMarker, positionName ) {
_getBalloonPanelPositionData( mentionMarker, preferredPosition ) {
const editing = this.editor.editing;
const domConverter = editing.view.domConverter;
const mapper = editing.mapper;
Expand All @@ -453,15 +469,16 @@ export default class MentionUI extends Plugin {

return null;
},
positions: getBalloonPanelPositions( positionName )
positions: getBalloonPanelPositions( preferredPosition )
};
}
}

// Returns the balloon positions data callbacks.
//
// @param {String} preferredPosition
// @returns {Array.<module:utils/dom/position~Position>}
function getBalloonPanelPositions( positionName ) {
function getBalloonPanelPositions( preferredPosition ) {
const positions = {
// Positions the panel to the southeast of the caret rectangle.
'caret_se': targetRect => {
Expand Down Expand Up @@ -501,9 +518,9 @@ function getBalloonPanelPositions( positionName ) {
};

// Returns only the last position if it was matched to prevent the panel from jumping after the first match.
if ( positions.hasOwnProperty( positionName ) ) {
if ( positions.hasOwnProperty( preferredPosition ) ) {
return [
positions[ positionName ]
positions[ preferredPosition ]
];
}

Expand Down
55 changes: 31 additions & 24 deletions tests/mentionui.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@
import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';
import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import BalloonPanelView from '@ckeditor/ckeditor5-ui/src/panel/balloon/balloonpanelview';
import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
import global from '@ckeditor/ckeditor5-utils/src/dom/global';
import { setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
import DomEventData from '@ckeditor/ckeditor5-engine/src/view/observer/domeventdata';
import EventInfo from '@ckeditor/ckeditor5-utils/src/eventinfo';
import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
import ContextualBalloon from '@ckeditor/ckeditor5-ui/src/panel/balloon/contextualballoon';
import env from '@ckeditor/ckeditor5-utils/src/env';

import MentionUI from '../src/mentionui';
Expand Down Expand Up @@ -57,6 +57,12 @@ describe( 'MentionUI', () => {
} );
} );

it( 'should load ContextualBalloon plugin', () => {
return createClassicTestEditor().then( () => {
expect( editor.plugins.get( ContextualBalloon ) ).to.be.instanceOf( ContextualBalloon );
} );
} );

describe( 'init()', () => {
it( 'should throw if marker was not provided for feed', () => {
return createClassicTestEditor( { feeds: [ { feed: [ 'a' ] } ] } ).catch( error => {
Expand Down Expand Up @@ -88,25 +94,26 @@ describe( 'MentionUI', () => {
} );
} );

describe( 'child views', () => {
beforeEach( () => createClassicTestEditor() );

describe( 'panelView', () => {
it( 'should create a view instance', () => {
expect( panelView ).to.instanceof( BalloonPanelView );
} );
describe( 'contextual balloon', () => {
beforeEach( () => {
return createClassicTestEditor( staticConfig )
.then( () => {
setData( model, '<paragraph>foo []</paragraph>' );

it( 'should be added to the ui.view.body collection', () => {
expect( Array.from( editor.ui.view.body ) ).to.include( panelView );
} );
model.change( writer => {
writer.insertText( '@', doc.selection.getFirstPosition() );
} );
} )
.then( waitForDebounce );
} );

it( 'should have disabled arrow', () => {
expect( panelView.withArrow ).to.be.false;
} );
it( 'should disable arrow', () => {
expect( panelView.isVisible ).to.be.true;
expect( panelView.withArrow ).to.be.false;
} );

it( 'should have added MentionView as a child', () => {
expect( panelView.content.get( 0 ) ).to.be.instanceof( MentionsView );
} );
it( 'should add MentionView to a panel', () => {
expect( editor.plugins.get( ContextualBalloon ).visibleView ).to.be.instanceof( MentionsView );
} );
} );

Expand Down Expand Up @@ -152,12 +159,13 @@ describe( 'MentionUI', () => {
return waitForDebounce()
.then( () => {
const pinArgument = pinSpy.firstCall.args[ 0 ];
const { target, positions, limiter } = pinArgument;
const { target, positions, limiter, fitInViewport } = pinArgument;

expect( positions ).to.have.length( 4 );

// Mention UI should set limiter to the editable area.
expect( limiter() ).to.equal( editingView.domConverter.mapViewToDom( editableElement ) );
expect( fitInViewport ).to.be.undefined;

expect( editor.model.markers.has( 'mention' ) ).to.be.true;
const mentionMarker = editor.model.markers.get( 'mention' );
Expand Down Expand Up @@ -226,7 +234,7 @@ describe( 'MentionUI', () => {

expect( positions ).to.have.length( 4 );

positionAfterFirstShow = panelView.position;
positionAfterFirstShow = mentionsView.position;

model.change( writer => {
writer.insertText( 't', doc.selection.getFirstPosition() );
Expand Down Expand Up @@ -356,15 +364,14 @@ describe( 'MentionUI', () => {
stopPropagation: sinon.spy()
};

const mentionElementSpy = testUtils.sinon.spy( mentionsView.element, 'scrollTop', [ 'set' ] );

return waitForDebounce()
.then( () => {
// The scroll test highly depends on browser styles.
// Some CI test environments might not load theme which will result that tests will not render on CI as locally.
// To make this test repeatable across different environments it enforces mentions view size to 100px...
const reset = 'padding:0px;margin:0px;border:0 none;line-height: 1em;';

const mentionElementSpy = testUtils.sinon.spy( mentionsView.element, 'scrollTop', [ 'set' ] );
mentionsView.element.style = `min-height:100px;height:100px;max-height:100px;${ reset };`;

// ...and each list view item size to 25px...
Expand Down Expand Up @@ -865,7 +872,7 @@ describe( 'MentionUI', () => {
} );

expect( panelView.isVisible ).to.be.false;
expect( panelView.position ).to.be.undefined;
expect( mentionsView.position ).to.be.undefined;
expect( editor.model.markers.has( 'mention' ) ).to.be.false;
} );
} );
Expand Down Expand Up @@ -896,7 +903,7 @@ describe( 'MentionUI', () => {
} );

expect( panelView.isVisible ).to.be.false;
expect( panelView.position ).to.be.undefined;
expect( mentionsView.position ).to.be.undefined;
expect( editor.model.markers.has( 'mention' ) ).to.be.false;
} );
} );
Expand Down Expand Up @@ -1557,7 +1564,7 @@ describe( 'MentionUI', () => {
doc = model.document;
editingView = editor.editing.view;
mentionUI = editor.plugins.get( MentionUI );
panelView = mentionUI.panelView;
panelView = editor.plugins.get( ContextualBalloon ).view;
mentionsView = mentionUI._mentionsView;
} );
}
Expand Down

0 comments on commit 9ae7f30

Please sign in to comment.