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

Introduced ContextualBalloon class for managing contextual balloons. #177

Merged
merged 17 commits into from
Apr 5, 2017
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
170 changes: 170 additions & 0 deletions src/contextualballoon.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
/**
* @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
* For licensing, see LICENSE.md.
*/

/**
* @module ui/contextualballoon
*/

import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';

/**
* Common contextual balloon of the Editor.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How much work would it take to make it a generic utility? I suppose eventually we may end up with other UI components which should manage their children in a similar way. E.g. a dialog, where you edit some fields, then you go deeper to edit some details or internals, then you go back again, etc., without leaving the dialog. Each editing level would be a different child view of the same dialog. If this is a lot of thinking, designing and coding we may think about it as a follow–up, because it's not necessary to MVP.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created follow-up #186

*
* This class reuses the same {module:ui/view~View} for each contextual balloon panel in the editor UI, makes
* possible to add multiple panels to the same balloon (stored in the stack, last one in the stack is visible)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why calling it "panels"? They're simply views, aren't they? It's a tool to use a single BalloonPanelView with multiple child views. The word "panel" repeats many times in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed.

* and prevents of displaying more than one contextual balloon panel at the same time.
*/
export default class ContextualBalloon {
/**
* Creates ContextualBalloon instance.
*
* @constructor
*/
constructor() {
/**
* Stack of panels injected to the balloon. Last one in the stack is displayed
* as content of {@link module:ui/contextualballoon~ContextualBalloon#view}.
*
* @private
* @member {Map} #_stack
*/
this._stack = new Map();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@readonly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not @private?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good point :P


/**
* Balloon panel view.
*
* @member {module:ui:view~View} #view
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@readonly?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway, any practical reason why not creating the BalloonPanelView instance directly at this point but assigning it like ctxBalloon.view = new BalloonPanelView() later on instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved it to the constructor.

*/
}

/**
* Returns configuration of currently visible panel or `null` when there is no panel in the stack.
*
* @returns {module:ui/contextualballoon~Panel|null}
*/
get visible() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Boolean properties and variables are always prefixed by an auxiliary verb:

https://github.com/ckeditor/ckeditor5-design/wiki/Code-Style-Naming-Guidelines

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This getter returns visible view. It's not checking if given view is visible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So get visibleView() maybe? I'm trying to make it less ambiguous.

Copy link
Contributor Author

@oskarwrobel oskarwrobel Apr 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... this returns not only the view but the whole configuration { view, position } https://github.com/ckeditor/ckeditor5-ui/pull/177/files#diff-93d63500d90d723217fc16438b6b32a4R193

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But maybe only the view should be returned.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... looks like there is no need to return the balloon position configuration because this position is used internally by ContextualBalloon.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

return this._stack.get( this.view.content.get( 0 ) ) || null;
}

/**
* Returns `true` when panel of given view is in the stack otherwise returns `false`.
*
* @param {module:ui:view~View} view
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Invalid link.

* @returns {Boolean}
*/
isPanelInStack( view ) {
return this._stack.has( view );
}

/**
* Adds panel to the stack and makes this panel visible.
*
* @param {module:ui/contextualballoon~Panel} panelData Configuration of the panel.
*/
add( panelData ) {
if ( this._stack.get( panelData.view ) ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this.isPanelInStack( panelData.view ) be cleaner?

Copy link
Contributor Author

@oskarwrobel oskarwrobel Apr 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I added isPanelInstack() after I wrote this code.

/**
* Trying to add configuration of the same panel more than once.
*
* @error contextualballoon-add-item-exist
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error id in docs does not match error id in throw new CKEditorError().

*/
throw new CKEditorError( 'contextualballoon-add-panel-exist: Cannot add the same panel twice.' );
}

// When adding panel to the not empty balloon.
if ( this.visible ) {
// Remove displayed content from the view.
this.view.content.remove( this.visible.view );
}

// Add new panel to the stack.
this._stack.set( panelData.view, panelData );
// And display it.
this._showPanel( panelData );
}

/**
* Removes panel of given {@link: module:ui/view~View} from the stack of panels.
* If removed panel was visible then the panel before in the stack will be visible instead.
* When there is no panel in the stack then balloon will hide.
*
* @param {module:ui/view~View} view View of panel which will be removed from the balloon.
*/
remove( view ) {
if ( !this._stack.get( view ) ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this.isPanelInStack( view )

/**
* Trying to remove configuration of the panel not defined in the stack.
*
* @error contextualballoon-remove-panel-not-exist
*/
throw new CKEditorError( 'contextualballoon-remove-panel-not-exist: Cannot remove configuration of not existing panel.' );
}

// When visible panel is being removed.
if ( this.visible.view === view ) {
// We need to remove it from the view content.
this.view.content.remove( view );

// And then remove from the stack.
this._stack.delete( view );

// Next we need to check if there is other panel in stack to show.
const lastPanel = Array.from( this._stack ).pop();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using this._stack.values() would let us avoid lastPanel[ 1 ] a couple of lines later.


// If it is.
if ( lastPanel ) {
// Just show it.
this._showPanel( lastPanel[ 1 ] );
// Otherwise.
} else {
// Hide balloon panel.
this.view.hide();
}
// Otherwise.
} else {
// Just remove given panel from the stack.
this._stack.delete( view );
}
}

/**
* Updates position of balloon panel according to position data
* of the first panel in the {#_stack}.
*/
updatePosition() {
this.view.attachTo( this._getBalloonPosition() );
}

/**
* Sets panel as a content of the balloon and attaches balloon using position options of the first panel.
*
* @private
* @param {module:ui/contextualballoon~Panel} panelData Configuration of the panel.
*/
_showPanel( panelData ) {
this.view.content.add( panelData.view );
this.view.attachTo( this._getBalloonPosition() );
}

/**
* Returns position options of the first panel in the stack.
* This helps to keep balloon in the same position when panels are changed.
*
* @private
* @returns {module:utils/dom/position~Options}
*/
_getBalloonPosition() {
return Array.from( this._stack )[ 0 ][ 1 ].position;
}
}

/**
* An object describing configuration of single panel added to the balloon stack.
*
* @typedef {Object} module:ui/contextualballoon~Panel
*
* @property {module:ui/view~View} view Panel content view.
* @property {module:utils/dom/position~Options} position Positioning options.
*/
Loading