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 #33 from ckeditor/t/28
Browse files Browse the repository at this point in the history
Fix: The `BalloonToolbar` should be enabled for a table content. Link editing should be possible in the table. Closes #28.
  • Loading branch information
jodator authored Jun 8, 2018
2 parents b8b9799 + 8c5384e commit 037df98
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 14 deletions.
6 changes: 4 additions & 2 deletions src/tabletoolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
import ToolbarView from '@ckeditor/ckeditor5-ui/src/toolbar/toolbarview';
import ContextualBalloon from '@ckeditor/ckeditor5-ui/src/panel/balloon/contextualballoon';
import { isTableWidgetSelected } from './utils';
import { isTableWidgetSelected, isTableContentSelected } from './utils';
import { repositionContextualBalloon, getBalloonPositionData } from './ui/utils';

const balloonClassName = 'ck-toolbar-container';
Expand Down Expand Up @@ -112,7 +112,9 @@ export default class TableToolbar extends Plugin {
if ( !editor.ui.focusTracker.isFocused ) {
this._hideToolbar();
} else {
if ( isTableWidgetSelected( editor.editing.view.document.selection ) ) {
const viewSeleciton = editor.editing.view.document.selection;

if ( isTableWidgetSelected( viewSeleciton ) || isTableContentSelected( viewSeleciton ) ) {
this._showToolbar();
} else {
this._hideToolbar();
Expand Down
9 changes: 8 additions & 1 deletion src/ui/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import BalloonPanelView from '@ckeditor/ckeditor5-ui/src/panel/balloon/balloonpanelview';
import { getParentTable } from '../commands/utils';
import { isTableWidgetSelected } from '../utils';

/**
* A helper utility that positions the
Expand All @@ -34,8 +35,14 @@ export function repositionContextualBalloon( editor ) {
export function getBalloonPositionData( editor ) {
const editingView = editor.editing.view;
const defaultPositions = BalloonPanelView.defaultPositions;
const viewSelection = editingView.document.selection;
let parentTable;

const parentTable = getParentTable( editingView.document.selection.getFirstPosition() );
if ( isTableWidgetSelected( viewSelection ) ) {
parentTable = viewSelection.getSelectedElement();
} else {
parentTable = getParentTable( viewSelection.getFirstPosition() );
}

return {
target: editingView.domConverter.viewToDom( parentTable ),
Expand Down
14 changes: 13 additions & 1 deletion src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import { toWidget, isWidget } from '@ckeditor/ckeditor5-widget/src/utils';
import { getParentTable } from './commands/utils';

const tableSymbol = Symbol( 'isImage' );
const tableSymbol = Symbol( 'isTable' );

/**
* Converts a given {@link module:engine/view/element~Element} to a table widget:
Expand Down Expand Up @@ -45,6 +45,18 @@ export function isTableWidget( viewElement ) {
* @returns {Boolean}
*/
export function isTableWidgetSelected( selection ) {
const viewElement = selection.getSelectedElement();

return !!( viewElement && isTableWidget( viewElement ) );
}

/**
* Checks if a table widget content is selected.
*
* @param {module:engine/view/selection~Selection|module:engine/view/documentselection~DocumentSelection} selection
* @returns {Boolean}
*/
export function isTableContentSelected( selection ) {
const parentTable = getParentTable( selection.getFirstPosition() );

return !!( parentTable && isTableWidget( parentTable ) );
Expand Down
20 changes: 13 additions & 7 deletions tests/integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,21 +46,27 @@ describe( 'TableToolbar integration', () => {
return newEditor.destroy();
} );

it( 'should prevent the BalloonToolbar from being displayed when an table is selected', () => {
// When table is selected along with text.
it( 'should allow the BalloonToolbar to be displayed when a table is selected with surrounding text', () => {
setModelData( newEditor.model, '<paragraph>fo[o</paragraph><table><tableRow><tableCell></tableCell></tableRow></table>]' );

balloonToolbar.show();

// BalloonToolbar should be visible.
expect( balloon.visibleView ).to.equal( balloonToolbar.toolbarView );
} );

it( 'should allow the BalloonToolbar to be displayed when a table content is selected', () => {
setModelData( newEditor.model, '<paragraph>foo</paragraph><table><tableRow><tableCell>x[y]z</tableCell></tableRow></table>' );

balloonToolbar.show();

expect( balloon.visibleView ).to.equal( balloonToolbar.toolbarView );
} );

// When only table is selected.
setModelData( newEditor.model, '<paragraph>foo</paragraph><table><tableRow><tableCell>[]</tableCell></tableRow></table>' );
it( 'should prevent the BalloonToolbar from being displayed when a table is selected as whole', () => {
setModelData( newEditor.model, '<paragraph>foo</paragraph>[<table><tableRow><tableCell>foo</tableCell></tableRow></table>]' );

balloonToolbar.show();

// BalloonToolbar should not be visible.
expect( balloon.visibleView ).to.be.null;
} );

Expand All @@ -70,7 +76,7 @@ describe( 'TableToolbar integration', () => {
const normalPrioritySpy = sinon.spy();

// Select an table
setModelData( newEditor.model, '<paragraph>foo</paragraph><table><tableRow><tableCell>[]</tableCell></tableRow></table>' );
setModelData( newEditor.model, '<paragraph>foo</paragraph>[<table><tableRow><tableCell>x</tableCell></tableRow></table>]' );

newEditor.listenTo( balloonToolbar, 'show', highestPrioritySpy, { priority: 'highest' } );
newEditor.listenTo( balloonToolbar, 'show', highPrioritySpy, { priority: 'high' } );
Expand Down
19 changes: 16 additions & 3 deletions tests/tabletoolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,12 @@ describe( 'TableToolbar', () => {
} );

it( 'should show the toolbar on render when the table is selected', () => {
setData( model, '<paragraph>foo</paragraph>[<table><tableRow><tableCell></tableCell></tableRow></table>]' );

expect( balloon.visibleView ).to.equal( toolbar );
} );

it( 'should show the toolbar on render when the table content is selected', () => {
setData( model, '<paragraph>[foo]</paragraph><table><tableRow><tableCell></tableCell></tableRow></table>' );

expect( balloon.visibleView ).to.be.null;
Expand All @@ -126,7 +132,7 @@ describe( 'TableToolbar', () => {
expect( balloon.visibleView ).to.be.null;

model.change( writer => {
// Select the [<table></table>]
// Select the [<tableCell></tableCell>]
writer.setSelection(
Range.createOn( doc.getRoot().getChild( 1 ).getChild( 0 ).getChild( 0 ) )
);
Expand All @@ -141,13 +147,20 @@ describe( 'TableToolbar', () => {
} );

it( 'should not engage when the toolbar is in the balloon yet invisible', () => {
setData( model, '<table><tableRow><tableCell>[]</tableCell></tableRow></table>' );
setData( model, '<table><tableRow><tableCell>x[y]z</tableCell></tableRow></table>' );
expect( balloon.visibleView ).to.equal( toolbar );

// Put anything on top of the ContextualBalloon stack above the table toolbar.
const lastView = new View();
lastView.element = document.createElement( 'div' );

balloon.add( { view: lastView } );
balloon.add( {
view: lastView,
position: {
target: document.body
}
} );

expect( balloon.visibleView ).to.equal( lastView );

editingView.change( () => {} );
Expand Down

0 comments on commit 037df98

Please sign in to comment.